strace seg with select and -ve nfds

[Dmitry V. Levin]

On Tue, Nov 05, 2013 at 04:19:31PM +0100, Denys Vlasenko wrote:
> On 11/05/2013 01:33 PM, Dr. David Alan Gilbert wrote:
> > * Denys Vlasenko (dvlasenk at redhat.com) wrote:
> >> I propose to do simply this:
> >>
> >> +       nfds = fdsize;
> >>         fdsize = (((fdsize + 7) / 8) + sizeof(long)-1) & -sizeof(long);
> >> +       /* We had bugs a-la "while (j < args[0])" and "umoven(args[0])" below.
> >> +        * Instead of args[0], use nfds for fd count, fdsize for array lengths.
> >> +        */
> >>
> >> and use nfds in those two places where we incorrectly use arg[0] now.
> > 
> >>> Thoughts?
> >>
> >> I applied a slightly simplified version of your fix to strace git, please try it.
> > 
> > That still fails (this is FORTIFY detecting the fail).
> 
> Please elaborate. You get a warning about access to fd_set->[__]fds_bits
> array past its declared bounds? How it looks? Does strace abort or what?

sizeof(fd_set) is part of libc ABI, so glibc in _FORTIFY_SOURCE mode
aborts the process using __chk_fail() when descriptor is larger than
allowed value (FD_SETSIZE at the time of compiling glibc).

> There can be legitimate programs which use select() in excess of glibc-imposed
> artificial limit on bit array sizes.

Yes, the syscall itself imposes no such limitations.
Such programs are more expected to use poll() instead of select(), though.

> > (I guess you could argue that's a false positive from fortify, but there
> > again I think it is an illegal use of FD_ISSET).
> 
> Do you see a reasonably portable way to check FD_ISSET?

Looks like all FD_ISSET implementations just test the n-th bit in the
array of long ints.  I've pushed a commit with yet another FD_ISSET
implementation that hopefully does the same.

> On the related note, how are we doing in "stracing 32-bit app
> with 64-bit strace on a big-endian machine" case?
> In that case, sizeof(long) is important...
> I dread to think about that:(

We cannot make things worse than they were since the beginning. :)


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20131106/1a382c17/attachment.bin>