inetd bug

Cathey, Jim jcathey at ciena.com
Wed Nov 5 23:23:59 UTC 2008


The one-liner appears to work in my testing.
Perhaps I was worrying too much about the
zero FD's case.  (Habit!)

-- Jim




-----Original Message-----
From: Denys Vlasenko [mailto:vda.linux at googlemail.com] 
Sent: Wednesday, November 05, 2008 2:35 PM
To: Cathey, Jim
Cc: wharms at bfs.de; busybox at busybox.net
Subject: Re: inetd bug

Hi Jim,

On Wednesday 05 November 2008 18:47, Cathey, Jim wrote:
> On the face of it I think this fix is inadequate,
> given that "maxsock + 1" is used in the select().
> With no services, should select not have a 0 as the
> first argument, thus requiring maxsock to be -1 and
> not 0?  All the problems fell out of that value.

This is safe. fds 0, 1 and 2 are not included in
fd bitmask for select. Look closer:

        if (!(opt & 2))
                bb_daemonize_or_rexec(0, argv - optind);
        else
                bb_sanitize_stdio();

This code makes sure that even if user somehow managed
to start inetd with these descriptors CLOSED (!),
we reopen them to /dev/null. We never use them
for network i/o.

This the lowest descriptor in select() which can ever
be used is 3. Specifying no_of_fds=1 in select() will make it
wait on empty set of fds.


I want to have minimalistic fix because your patch has bugs:

-               if (maxsock >= 0 && fd > maxsock) {
+               if (fd > maxsock) {
                        prev_maxsock = maxsock = fd;

Scenario: we need to stop waiting on a fd - we call
remove_fd_from_set(). It sets maxsock to -1 ("unknown").

Then we got SIGCHLD and need to start waiting on another fd
(another wait-type service). We call add_fd_to_set().
Due to your change fd > maxsock (because maxsock == -1)
and it will erroneously set prev_maxsock = maxsock = fd,
which is NOT the maximal open fd!

On next iteration select() will lose some number of fds
(all fds which are > fd).

So, does one-line fix I posted work, or not? In my testing,
it works.
--
vda




More information about the busybox mailing list