On Wed, May 6, 2015 at 2:23 AM, Vitaliy Makkoveev <[email protected]> wrote: > Struct file has reference count "f_count" field. When f_count > 0 file > instance > exists, when f_count becomes 0 file instance should be removed. It's simple > but some strange magic exists. When file was created by falloc() it has > f_count == 2. One additional FREF() call was done inside falloc(). After > file was setupped, FILE_SET_MATURE() call do FRELE() and f_count becomes == 1. > Also closef() function exists. It wants f_count be >= 2 otherwise it panics. > closef() caller bumps f_count before closef() call just for prevents this > panic. I found this behaviour strange, and I described this before as "Strange > magic with f_count...". Ok, this explanation sucks :) so I dug into cvs > history. ... > Within FREF() and FRELE() "f_usecount" was just replaced by > "f_count" and existed old fp->f_count increments and decrements haven't be > removed. So after sys/sys/file.h rev 1.30 there are some places where f_count > is bumped twice and decremented twice and other related code has hacks for > this > condition. > > Now, the legacy of "f_usecount" coexists with "f_count" and I think it > should be fixed. I did the diff for this. I did system rebuild on diskless > NFS machine without panics, and kern.nfiles before rebuild is equal to > kern.nfiles after rebuild. So looks like there is no f_count corruption > and file instance leaks. In this diff falloc() returns file instance with > f_count == 1 so FRELE() call removed from FILE_SET_MATURE(). "struct proc *" > arg removed from FILE_SET_MATURE() because it is unnecessary. closef() wants > f_count >= 1 and closef() callers don't do additional bump on f_count.
No, after falloc() returns there *are* two references to the file: one in the process's file descriptor table and the other *in use* by the thread that called falloc(). The second reference must be counted for the same reason that other fd-using syscalls use FREF/FRELE: if another thread calls close() on the fd (or, more complicated, targets it with dup2()) then the file descriptor table entry may be removed while the thread that called falloc() is still using the file. By failing to hold that extra reference from falloc() through FILE_SET_MATURE(), your proposed change introduces a use-after-free. Philip Guenther
