On 12/10/2010 12:36 PM, Laine Stump wrote: > On 12/03/2010 05:03 PM, Eric Blake wrote: >> * src/util/util.c (__virExec): Don't use FD_ISSET on out-of-bounds fd. >> --- >> >> Noticed this one by inspection, while investigating >> https://bugzilla.redhat.com/show_bug.cgi?id=659855 >> >> Don't know if it's the root cause of the crash in that bug, though. Actually, on re-reading, I think that bug report was not an actual crash, so much as someone trying to understand the original code and providing a gdb backtrace of a working setup based on a misreading of the code base. >> - (!keepfd || >> - !FD_ISSET(i, keepfd))) { >> + (!keepfd || (i< FD_SETSIZE&& !FD_ISSET(i, keepfd)))) { >> tmpfd = i; >> VIR_FORCE_CLOSE(tmpfd); >> } > > ACK. Definitely this could be bad news if OPEN_MAX > FD_SETSIZE, and > even if that's not possible, it doesn't hurt to check anyway. On Linux, FD_SETSIZE is 1024, and getconf(_SC_OPEN_MAX) starts at 1024 (I don't know if it can dynamically grow larger, but if we have an fd that large, we probably have bigger problems of an fd leak on our hands before this bug would have bitten us); but unless getconf can return a dynamic value for _SC_OPEN_MAX, the undefined behavior would never occur. On the other hand, cygwin is a platform where FD_SETSIZE is 64, but getconf(_SC_OPEN_MAX) is dynamic, starting at 32, but growing as large as 3200. Thus, even having as low as fd 65 open, and having the stack overflow of reading beyond keepfd accidentally see that fd as one to be kept, can leak fd 65 into the child (and only depending on what was at the out-of-bounds memory). Still a bit high to have that many fds open at once, but much more feasible than the Linux limit, and definitely a platform where OPEN_MAX can be larger than FD_SETSIZE. I've pushed this now. -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list