Valgrind discovered invalid file-handle closing

[GoogleCodeExporter]

On running the program with valgrind:

sudo valgrind -v -v --log-file=log.out ./shellinaboxd -q -c 
/var/lib/shellinabox -p 4200 -u shellinabox -g shellinabox --no-beep

There are the following attempts to close file handles near the start of the 
program.  I checked the presence of these files with access() and found that 
they are bonafide paths in /proc/self/fd during the closeAllFds() call, but 
system ("ls ...") cannot see them.  They seem to be regular files and FIFOs.

I could find no easy way to distinguish these handles from the kinds we need to 
close.  So, either we use some other criteria or we allow these attempts to 
close filehandles that the kernel appears to want us to *not* close.

Warning: invalid file descriptor 1014 in syscall close()
==30873==    at 0x510B8C0: __close_nocancel (syscall-template.S:82)
==30873==    by 0x40811F: closeAllFds (launcher.c:726)
==30873==    by 0x40A170: forkLauncher (launcher.c:1718)
==30873==    by 0x404B74: main (shellinaboxd.c:1240)
==30873== Warning: invalid file descriptor 1015 in syscall close()
==30873==    at 0x510B8C0: __close_nocancel (syscall-template.S:82)
==30873==    by 0x40811F: closeAllFds (launcher.c:726)
==30873==    by 0x40A170: forkLauncher (launcher.c:1718)
==30873==    by 0x404B74: main (shellinaboxd.c:1240)
==30873== Warning: invalid file descriptor 1016 in syscall close()
==30873==    Use --log-fd=<number> to select an alternative log fd.
==30873==    at 0x510B8C0: __close_nocancel (syscall-template.S:82)
==30873==    by 0x40811F: closeAllFds (launcher.c:726)
==30873==    by 0x40A170: forkLauncher (launcher.c:1718)
==30873==    by 0x404B74: main (shellinaboxd.c:1240)
==30873== Warning: invalid file descriptor 1017 in syscall close()
==30873==    at 0x510B8C0: __close_nocancel (syscall-template.S:82)
==30873==    by 0x40811F: closeAllFds (launcher.c:726)
==30873==    by 0x40A170: forkLauncher (launcher.c:1718)
==30873==    by 0x404B74: main (shellinaboxd.c:1240)
==30873== Warning: invalid file descriptor 1018 in syscall close()
==30873==    at 0x510B8C0: __close_nocancel (syscall-template.S:82)
==30873==    by 0x40811F: closeAllFds (launcher.c:726)
==30873==    by 0x40A170: forkLauncher (launcher.c:1718)
==30873==    by 0x404B74: main (shellinaboxd.c:1240)
==30873== Warning: invalid file descriptor 1019 in syscall close()
==30873==    at 0x510B8C0: __close_nocancel (syscall-template.S:82)
==30873==    by 0x40811F: closeAllFds (launcher.c:726)
==30873==    by 0x40A170: forkLauncher (launcher.c:1718)
==30873==    by 0x404B74: main (shellinaboxd.c:1240)

Original issue reported on code.google.com by beewoo...@gmail.com on 6 Apr 2012 at 3:25

[GoogleCodeExporter]

I don't think anything needs to be done here.

Upon calling exec(), we have to close all file handles except for the ones that 
we intentionally want to pass to the newly launched child process. In Advanced 
Programming in the UNIX Environment, Stevens suggests closing all possible file 
handles. This can be done by looping from 3 to sysconf(OPEN_MAX).

Historically, this was good advice as a) there was no way to determine the list 
of open file handles, and b) OPEN_MAX was typically pretty small (on the order 
of a few hundred). So, the cost of making these system calls was cheap.

These days, OPEN_MAX is on the order of thousands or tens of thousands. Making 
this many calls to close() results in noticeable delays. So, if at all 
possible, it is more efficient to only close the file handles that are actually 
open. On Linux, this can be achieved by enumerating the entries in 
/proc/self/fd.

In either case, we'll trigger a false positive in valgrind. Valgrind opens a 
few file handles for internal use, and since we have no way of telling that we 
should be skipping these file handles, we'll attempt to close them.

I believe, Valgrind notices and actually prevents us from doing so. So, no 
actual harm done. But we still get a warning message.

The correct thing to do is to ignore the warning, if it triggers as part of 
closeAllFds(). If it triggers anywhere else, we of course want to investigate.

If you feel particularly bothered by this, you should try to come up with a 
valgrind suppression file. That's actually the right thing to do here.

Original comment by zod...@gmail.com on 6 Apr 2012 at 4:23