From: Rich Felker <dalias@libc.org>
To: Torvald Riegel <triegel@redhat.com>
Cc: "Ondřej Bílka" <neleai@seznam.cz>,
"GLIBC Devel" <libc-alpha@sourceware.org>
Subject: Re: [PATCH 1/3] Use reliable sem_wait interruption in nptl/tst-sem6.
Date: Tue, 09 Dec 2014 20:19:00 -0000 [thread overview]
Message-ID: <20141209201907.GP4574@brightrain.aerifal.cx> (raw)
In-Reply-To: <1418151428.25868.238.camel@triegel.csb>
On Tue, Dec 09, 2014 at 07:57:08PM +0100, Torvald Riegel wrote:
> On Tue, 2014-12-09 at 13:36 -0500, Rich Felker wrote:
> > On Tue, Dec 09, 2014 at 07:24:49PM +0100, Torvald Riegel wrote:
> > > > Which does not answer my objection. What extra bugs could this test catch,
> > > > compared to say tst-sem2? If there is no such bug you could just delete
> > > > that file.
> > >
> > > tst-sem2 tests that spurious wake-ups and such don't return anything but
> > > -1 and errno==EINTR, in particular that 0 isn't returned.
> > >
> > > After the patch, tst-sem6 tests that a signal handler that posts a token
> > > will make sem_wait return. It *also* allows for sem_wait to return -1
> > > and errno==EINTR in that case.
> > >
> > > Thus, one possible error that the patched tst-sem6 will catch is if the
> > > sem_wait itself just retries the futex_wait after the futex_wait
> > > returned EINTR, instead of looking for whether there is an available
> > > token.
> >
> > This would not be a bug. Simply retrying the futex_wait would result
> > in EAGAIN, since the futex value would no longer match.
>
> Right. So it would catch a bug that did a futex_wait after loading the
> new value.
I don't follow. If I understand what type of bug you're talking about,
there's no way such a bug would arise accidentally and only affect
EINTR. It would be a break in the whole usage pattern for futex waits
and would affect EAGAIN and non-spurious wakes too unless someone
intentionally special-cased EINTR to do the wrong thing.
> > > Let me try to summarize the background behind this change again:
> > >
> > > 1) Linux documents futex_wait to return EINTR on signals *or* on
> > > spurious wake-ups.
> >
> > No, the man pages document this, and they're wrong. I have not seen
> > any other "Linux documentation" claiming it.
>
> But is there other documentation than the man pages? The sources don't
> really count because that's not a guarantee nor a specification, that's
> the current implementation.
>
> Also, at least one kernel person seems to have confirmed that the
> current manpage is correct: https://lkml.org/lkml/2014/5/15/356
The linked mailing list message does not contain the text EINTR at
all, so I don't see where your claim that it supports the current man
page text about EINTR comes from.
> > > 2) If we treat 1) as true -- which we should to unless getting
> > > confirmation otherwise -- sem_wait must not return EINTR to the caller
> > > anymore if futex_wait returned EINTR.
> > > 3) Because of 2), the behavior that is tested in tst-sem6 before my
> > > patch cannot be implemented anymore.
> >
> > These (2) and (3) are based on false assumptions.
>
> I don't have any evidence to rely on something else. Don't get me
> wrong, if we get confirmation from the kernel that 1) is not true, then
> I'm open to doing something else. But until then, what should we do?
>
> Also, the change is within what's allowed by POSIX IMO, so we're not
> inventing new behavior here.
It's allowed by POSIX, yes, and as I've said before, I agree it's
better behavior -- programming with interrupting signal handlers is a
backwards, bogus practice, and from a hardening standpoint it seems
preferable not to have sem_wait fail at all. I just don't think the
"spurious EINTR is documented" argument should be used to justify such
a change, because accepting spurious EINTR is going to come back to
bite us if there are ever other interfaces (I believe aio_suspend
already is one?) that need to be implemented with futex and need to
report EINTR.
Rich
next prev parent reply other threads:[~2014-12-09 20:19 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-05 18:37 [PATCH 0/3] Fix semaphore destruction (BZ #12674) Torvald Riegel
2014-12-05 18:53 ` [PATCH 1/3] Use reliable sem_wait interruption in nptl/tst-sem6 Torvald Riegel
2014-12-06 13:50 ` Ondřej Bílka
2014-12-08 11:43 ` Torvald Riegel
2014-12-08 22:29 ` Ondřej Bílka
2014-12-09 10:16 ` Torvald Riegel
2014-12-09 16:50 ` Ondřej Bílka
2014-12-09 18:25 ` Torvald Riegel
2014-12-09 18:37 ` Rich Felker
2014-12-09 19:47 ` Torvald Riegel
2014-12-09 20:19 ` Rich Felker [this message]
2014-12-10 9:34 ` Torvald Riegel
2014-12-05 19:03 ` [PATCH 2/3] Fix nptl/tst-sem4: always start with a fresh semaphore Torvald Riegel
2014-12-06 13:55 ` Ondřej Bílka
2014-12-05 19:24 ` [PATCH 3/3] Update to new generic semaphore algorithm Torvald Riegel
2014-12-15 21:59 ` Torvald Riegel
2018-05-08 14:00 ` Andreas Schwab
2018-05-08 14:22 ` Torvald Riegel
2014-12-05 20:30 ` [PATCH 0/3] Fix semaphore destruction (BZ #12674) Rich Felker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141209201907.GP4574@brightrain.aerifal.cx \
--to=dalias@libc.org \
--cc=libc-alpha@sourceware.org \
--cc=neleai@seznam.cz \
--cc=triegel@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).