Re: musl signalfd (was: selfpipe + musl libc info)

Re: musl signalfd (was: selfpipe + musl libc info)

[Rich Felker]

On Mon, Apr 08, 2013 at 07:52:31AM +0200, Laurent Bercot wrote:
>  Thanks David.
>  And Rich, thanks for patching musl. However, I think your workaround for
> pre-2.6.27 kernels is unneeded, if not downright incorrect.
> 
>  The signalfd(2) manpage says: "In Linux up to version 2.6.26, the flags
> argument is unused, and must be specified as zero."
>  And also:
>  "ERRORS 
>   EINVAL  flags is invalid; or, in Linux 2.6.26 or earlier, flags is nonzero."
> 
>  The current musl patch actually honors the flags with a race condition.
> Instead, returning -1 EINVAL if the signalfd4 syscall does not exist but
> flags is nonzero is the safer thing to do - and is what the manpage
> specifies.
> 
>  The skalibs signalfd autodetection test actually relies on this. It
> tests the signalfd() libc call with a SFD_NONBLOCK flags argument; if
> the call fails for any reason, skalibs will not use signalfd at all
> (and implement selfpipe via a real self-pipe).

I don't know if you got my last email, because somehow when I hit
reply-to-all it went to your list (where it was held as moderated) but
not your personal address. So I'm repeating it here:

----------------------------------------------------------------------
This makes no sense. For SFD_NONBLOCK, setting it atomically does not
matter; the application could just fallback to setting it with fcntl
if opening it with the nonblocking flag failed. Falling back to using
a pipe in that situation makes no sense. If the issue were
SFD_CLOEXEC, non-atomicity is a problem, but there is no way to
fallback anyway. Using a pipe would still have the same non-atomicity
issue.

If you think there are situations where the application really could
work around the lack of these kernel features, please explain. I'm
open to changing things but I want a good justification.
----------------------------------------------------------------------

I did think of one additional thing I want to ask about though: does
setting non-blocking mode even via fcntl after the fd is obtained fail
to work properly on old kernels? If so, then I agree we should fail
the call if SFD_NONBLOCK was specified and signalfd4 returns -ENOSYS.

Can you let me know what the situation is and whether this issue, or
any other issue I might not be aware of, applies?

Rich

Re: musl signalfd (was: selfpipe + musl libc info)

[Laurent Bercot]

 Thank you for sending the e-mail again; I hadn't received it indeed.

> This makes no sense. For SFD_NONBLOCK, setting it atomically does not
> matter; the application could just fallback to setting it with fcntl
> if opening it with the nonblocking flag failed. Falling back to using
> a pipe in that situation makes no sense. If the issue were
> SFD_CLOEXEC, non-atomicity is a problem, but there is no way to
> fallback anyway. Using a pipe would still have the same non-atomicity
> issue.

 Sorry if I wasn't clear. Talking about my library cluttered things up.
My point is that there are 3 use cases in signalfd:
 1. the signalfd4 syscall exists. (Recent kernels.)
 2. the signalfd syscall exists, but the signalfd4 syscall does not.
(Kernel 2.6.22 to 2.6.26.) 
 3. the signalfd call does not even exist.

 Case 1 is very clear, as is case 3.
 Case 2 is the one I'm addressing.
 The signalfd(2) man page specifies that the library call must not
support a nonzero flags argument if the signalfd4 syscall is not
available, and must return -1 EINVAL.
 You chose to not follow this specification, and honor the flags
argument *even in case 2*. I am arguing that following the specification
is safer: just fail the call when signalfd4 returns -ENOSYS and flags
is nonzero. It is safer because the application might expect an
atomic library call, which your workaround is not. If people really
*need* signalfd with flags, they'll move to a more recent kernel.

 My library performs a compile-time test to check whether it can
safely use the signalfd() libc call, with nonzero flags. This test
obviously succeeds in case 1, and obviously fails in case 3. But
it should also *fail* in case 2, and enable my own workaround instead.
Your current implementation makes the test succeed in case 2, so my
library *will* use signalfd(). This is not a real problem to me, because
it does not rely on signalfd() atomicity, so I'll be happy with your
current version; however, there are a few cases where the safer option
might be preferable. (I'm thinking cross-compilation, where you do not
autodetect sysdeps but feed them by hand. If you say signalfd is OK
because you tested it on a target running musl and it worked, but
your final target is running another libc that does not perform the
same workaround, you're screwed. Pretty rare case, I'll agree.)

-- 
 Laurent

Re: musl signalfd (was: selfpipe + musl libc info)

[Rich Felker]

On Mon, Apr 08, 2013 at 07:52:31AM +0200, Laurent Bercot wrote:
>  Thanks David.
>  And Rich, thanks for patching musl. However, I think your workaround for
> pre-2.6.27 kernels is unneeded, if not downright incorrect.
> 
>  The signalfd(2) manpage says: "In Linux up to version 2.6.26, the flags
> argument is unused, and must be specified as zero."
>  And also:
>  "ERRORS 
>   EINVAL  flags is invalid; or, in Linux 2.6.26 or earlier, flags is nonzero."
> 
>  The current musl patch actually honors the flags with a race condition.
> Instead, returning -1 EINVAL if the signalfd4 syscall does not exist but
> flags is nonzero is the safer thing to do - and is what the manpage
> specifies.
> 
>  The skalibs signalfd autodetection test actually relies on this. It
> tests the signalfd() libc call with a SFD_NONBLOCK flags argument; if
> the call fails for any reason, skalibs will not use signalfd at all
> (and implement selfpipe via a real self-pipe).

This makes no sense. For SFD_NONBLOCK, setting it atomically does not
matter; the application could just fallback to setting it with fcntl
if opening it with the nonblocking flag failed. Falling back to using
a pipe in that situation makes no sense. If the issue were
SFD_CLOEXEC, non-atomicity is a problem, but there is no way to
fallback anyway. Using a pipe would still have the same non-atomicity
issue.

If you think there are situations where the application really could
work around the lack of these kernel features, please explain. I'm
open to changing things but I want a good justification.

Rich