ryu@kwmux.UUCP (07/14/86)
4.2BSD, 4.3BSD, vaxen and non-vaxen fixes in +FIX sys/sys/kern_descrip.c sys/sys/sys_inode.c sys/sys/sys_socket.c by Ryu Koriyama, Waseda Univ. Sci. & Eng. It is possible to close a file descriptor more than once, or otherwise use it after it has been closed, and possibly after another process has reallocated it or reallocated the in-core inode it points to. This has been first reported by Jeff Mogul <mogul@navajo> 13 Dec 83 and temporaly fix made by Brian Thomson decvax!uthub!tompson 13 Feb 84 REPEAT BY: First install Jeff Mogul's firewall panic in closef() [ref. unix-wizards <14552@sri-arpa.UUCP> Dec 12 83] you may have already seen this. If not, then put it in first Like so: kern_descrip.c, closef(), before (*fp->f_ops->fo_close)(fp); insert if(fp->f_count < 1) panic("closef: f_count < 1"); If you run the system heavly, you will soon meet a panic. FIX: What is happening is, 1. Usr calls close() via syscall 2. In close() it call closef() 3. Closef() calls (*fp->f_ops->fo_close)() which may be ether ino_close() or soo_close() 4.1. In ino_close(), after iput(), ino_close() sets fp->f_count to 0, which means (struct file *)fp is no longer used and is now an open slot. This is because we no longer need fp. Why we can say this? We are still passing fp to [cb]devsw[major(dev)].d_close(), BUT in ino_close(), a little after fp->f_count is set to 0, we are using fp as a variable to check that another inode for the same device isn't active. So what we are passing to [cb]devsw[].d_close() is garbage which will have fileNFILE in it (see the code). 4.1.2. Now suppose we block in [cb]devsw[].d_close(). 4.1.3. Fp is now a free slot. Someone, via falloc, gets this file structure. This is because falloc is told that a file structure which f_count field is 0 is a free slot. So now someone has this file structure. 4.1.4. Now the blocking [cb]devsw[].d_close() in 4.1.2 above has ended. We will now return to closef(). 4.1.5. In closef(), as soon as we have returned, closef() will set the f_count field to 0 since he thinks that the file structure is now longer needed. BOGUS!!! Someone is using that file structure! Sooner or later someone comes around and gets that file structure via falloc(). HAVOC!!! Now we have a file structure which is shared by 2 people. We will get a mixed file. DOOMED!!! 4.2. Now then, in soo_close() we only use the f_data field in the file structure. So we can't give fp away. Soo_close() calles soclose() to do the works. 4.2.1. Soon after soclose() returns, soo_close() will set fp->f_data to 0. WHY? Why does he have to set fp->f_data to 0? What happened to fp->f_close? Why doesn't he free the file structure which we no longer need? 4.2.2. Well as soon as we return to closef(), closef() will set the f_count to 0 and free the file structure slot. Here we are happly ever after. Now we know the problem. The problem is WHO is responsible to free the file structure slot. If closef() is responsible, ino_close() should not free it. If fileops.fo_close() is responsible, soo_close() should do free it. Looking at the code, in ino_close(), there is the next line fp->f_count = 0; /* XXX Should catch */ So, the author is asking fileops.fo_close() to be responsible in freeing the file structure slot. Here comes the bug. Now when we moved to 4.2BSD, we have got the socket. We got the soo_close() routine. But the soo_close() routine does not free the file structure slot. So we had to do it someware else. So the code went into closef(). THIS IS THE BUG!!! The FIX at last! Since the fileops.fo_close() routine should be responsible for freeing the file structure slot, 1. closef() should not set the f_count field to 0 2. all of the fileops.fo_close() routines should set the f_count field to 0 as soon as they have no need of the file structure slot. So the fix will be, 1. take out the fp->f_count=0; code out of closef(); 2. put in the fp->f_count=0; code into the last line of soo_close(9 routine. This way we will be freeing the file structure slot as soon as we are finished with it, and we can have less chance not able to be getting a file structure slot. FIX: file: sys/sys/kern_descrip.c routine: closef() modify: } (*fp->f_ops->fo_close)(fp); fp->f_count = 0; } to: } (*fp->f_ops->fo_close)(fp); } file: sys/sys_socket.c routine: soo_close() modify: error = soclose((struct socket *0fp->f_data); fp->f_data = 0; return (error); } to: error = soclose((struct socket *0fp->f_data); fp->f_data = 0; /* XXX Just in case... */ fp->f_count = 0; return (error); } END OF FIX Ryu Koriyama Waseda Univ. Sci. & Eng. To contact: Foretune Co. R&D Div. Towa Building Room 55 3-5-1 Suido Bunkyo-ku, Tokyo, Japan ZIP 112 Phone 03-811-7861 (day and night) Or on the net: via JUNET ryu@psycho.juice !titcca!kwmux!kiwi!psycho!ryu P.S. This fix was posted via this channel since the vax I am using is not connected to any net. You can contact me via this address. P.P.S. This fix is still needed in 4.3BSD and other codes than vaxen ones. We have this code running on heavly loaded vaxes but have no problem. At least this is NOT an ENBUG. P.P.P.S. Special thanks to Mr. Yoshihiro Magara yoshih-m@ascii.junet Mr. Masahiko Kiso masahi-k@ascii.junet for the support and testing the debug.