tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: short _file in stdio -> fd leak




On Mar 13, 2008, at 12:18 AM, Jan Schaumann wrote:

Hey,

So at work we ran into a situation where a process had to fdopen more
than 32K files, which lead to a file descriptor leak.  The reason for
this leak was that while regular fds are ints, _file is a short, so if
fdopen got an fd larger than SHRT_MAX, it would get sign-extended and
thus become invalid, causing the subsequent fclose to fail.

This being FreeBSD, the fix was found and contributed back into
FreeBSD's repository by John Baldwin in
http://www.freebsd.org/cgi/cvsweb.cgi/src/lib/libc/stdio/fdopen.c.diff?r1=1.8;r2=1.9
(with surrounding discussion on
http://docs.freebsd.org/mail/archive/2008/freebsd-arch/20080302.freebsd-arch.html) .

I believe that we are affected by the same problem and would like to
commit the same (simple short-term) fix.  Any objections?


It only addresses fdopen (at least the diff you posted). Need to handle fopen as well.

Also, this is only testing against SHRT_MAX so you really only get 16k of files. In reality the code claims the only sentinel value there is -1 so in theory we could store 0 -> (USHRT_MAX - 1) in there.

Wow...this brings back memories. Solaris is also completely busted in this regard. Except they store the fd as a char! They've never worked around it except "it's fixed in 64bit mode". Otherwise in 32bit you have to be very careful to always have fd's < 256 available if using stdio. We have some very nasty hacks at work to deal with this.

I stared at the FILE struct for a bit and there's no buffer space anywhere in this. If there had been a spare couple bytes somewhere we could have split the int potentially and dealt with it that way.

Then again, could just use the cookie as a look aside to deal with this. Have the cookie point to something that holds both the FILE* and the real fd associated with this (and just don't use the one inside the FILE anymore). Should still function with funopen since you're assumed to provide your own FILE * equiv there via the cookie you pass in.

That would break code which looks inside a FILE* and thinks it knows about everything but things which do that likely deserve to lose here. Otherwise this "kind of supports an fd" leads to bizarre errors for folks which are hard to track down.

James



Home | Main Index | Thread Index | Old Index