Re: accept4 and SOCK_NONBLOCK

[Eric Blake]

On 8/20/19 10:27 AM, Richard W.M. Jones wrote:
> First of all I'm using Linux 5.0.17 & glibc-2.29-15 so as far as I'm
> aware accept4 exists and fully works and gnulib shouldn't be replacing
> it at all.  I don't understand why it's being replaced.
> 

I think I'm reproducing your setup (Fedora 30), and running:

./gnulib-tool --test accept4

shows:
...
/usr/bin/mkdir -p sys
rm -f sys/socket.h-t sys/socket.h && \
{ echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */'; \
  sed -e 's|@''GUARD_PREFIX''@|GL|g' \
...
      -e 's/@''GNULIB_ACCEPT4''@/1/g' \
...
      -e 's|@''HAVE_ACCEPT4''@|1|g' \
...
make[4]: Entering directory '/home/eblake/gnulib/testdir935/build/gllib'
gcc -DHAVE_CONFIG_H -I. -I../../gllib -I..  -DGNULIB_STRICT_CHECKING=1
-g -O2 -MT accept4.o -MD -MP -MF .deps/accept4.Tpo -c -o accept4.o
../../gllib/accept4.c

so it is indeed locating the system accept4(), but also compiling the
gnulib file.  In turn, the gnulib file starts out:

#if HAVE_ACCEPT4

Aha - there's the bug.

Commit e597d4b8 changed from AC_CHECK_FUNCS_ONCE (defines HAVE_ACCEPT4)
to AC_CHECK_DECLS (defines HAVE_DECL_ACCEPT4), but failed to update the
.c file.  I'll push the obvious fix.



> But given that, in libguestfs we call:
> 
>   r = accept4 (console_sock, NULL, NULL, SOCK_NONBLOCK|SOCK_CLOEXEC);
> 
>   
> https://github.com/libguestfs/libguestfs/blob/master/lib/launch-libvirt.c#L639
> 
> This is a valid set of flags according to the Linux man page for
> accept4(2).  But it fails with EINVAL because of the following test in
> gnulib:
> 
>   if ((flags & ~(SOCK_CLOEXEC | O_TEXT | O_BINARY)) != 0)
>     {
>       errno = EINVAL;
>       return -1;
>     }
> 
>   https://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/accept4.c#n61

Yes, that code is unnecessarily strict; gnulib should be taught to
handle SOCK_NONBLOCK, but doing it everywhere correctly is a bigger
task, as it also affects the socket() function.

> 
> So I think the set of flags should be broadened to include
> SOCK_NONBLOCK + add a call to set_nonblocking_flag.
> 
> As for why accept4 is being replaced at all, the only reference to it
> in config.log is:
> 
>   configure:25630: checking whether accept4 is declared
>   configure:25630: gcc -c -O2 -g -pipe -Wall -Werror=format-security 
> -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions 
> -fstack-protector-strong -grecord-gcc-switches 
> -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 
> -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic 
> -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection  
> conftest.c >&5
>   configure:25630: $? = 0
>   configure:25630: result: yes

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature