ryu@kiwi.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 temporally 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 heavily, 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. This is a bit strange since 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 is has 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 assumes 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 because he thinks that the file structure is now longer needed and he should free the slot. But, still, someone may be using that file structure! Sooner or later someone comes around and gets that file structure via falloc(). Now we have a file structure which is shared by 2 processes. We will soon get a mixed file. 4.2. Now then, in soo_close() we will use the f_data field in the file structure. So we can't give fp away. Soo_close() calls soclose() to do the works. 4.2.1. Soon after soclose() returns, soo_close() will set fp->f_data to 0. This is strange too. P.S. I have to take a look at the code once more, but I am sure this is not needed, and is where the problem starts. Anyway soo_close() is not going to free the file structure slot. 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 happily ever after. Now we know the scene. 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 free it. Looking at the code, in ino_close(), there is the next line fp->f_count = 0; /* XXX Should catch */ So, I think, 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. And we have got the ino_close() and soo_close() routine. But the soo_close() routine does not free the file structure slot. So we had to do it somewhere else. So the code went into closef(). The BUG is born. 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() routine. This way we will be freeing the file structure slot as soon as we are finished with it, and we can have a free file structure as soon as possible. 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 *)fp->f_data); fp->f_data = 0; return (error); } to: error = soclose((struct socket *)fp->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.kiwi.juice {somewhere}!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. But, you can still 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 heavily loaded VAXs but have no problem. At least I am sure 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 of the debug.