On Wed, Oct 20, 2021 at 05:14:08PM +0200, Martin Pieuchot wrote:
> On 15/10/21(Fri) 16:33, Anton Lindqvist wrote:
> > On Fri, Oct 15, 2021 at 03:30:40PM +0200, Alexander Bluhm wrote:
> > > On Thu, Oct 14, 2021 at 11:32:49PM -0600, Theo de Raadt wrote:
> > > > Obviously.
> > > >
> > > > Back it out, ASAP, then try to repair.
> > > >
> > > > It is quite surprising there aren't enough regression tests to catch
> > > > something like this.
> > >
> > > I do not see any problem with this diff on my regress machines.
> > > My latest amd64 snap is this one, does it contain the commit?
> > >
> > > OpenBSD 7.0-current (GENERIC.MP) #37: Thu Oct 14 12:29:37 MDT 2021
> > > dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> > >
> > > Anton, is there something special with your setup?
> >
> > I don't think so, other than my machine being slower than yours.
> >
> > > Have you tried to backout this diff to confirm that it is related?
> >
> > Backing out the diff made all tests pass again.
>
> I wonder if you aren't hitting a corner case in the pipe's kqueue
> handler which is returning EPIPE. This might be relevant to timing.
>
> Ironically the poll(2) part of my diff already handles EPIPE...
>
> >
> > mpi@, let me know if I can help in anyway.
>
> Could you try the diff below? It's the same as before plus a special
> check for EPIPE.
>
> If you can still reproduce the issue with it, please enter ddb(4) and set
> `kpoll_debug' to 1 to get more debug informations.
You're pointing in the right direction but the pipe semantics for kqueue
and select are still different. With your diff, the t-scp3 test case
hangs in kqread when the end of the pipe is gone.
pipe_poll() returns successfully (i.e. POLLHUP) when the other end is
gone causing select to flag the fd as ready. Interacting with the pipe
fd will fail with EPIPE at this point.
Instead of masking EPIPE in pselregister(), here's an attempt to
preserve the same semantics. I've put together a deterministic
reproducer, coming up in a separate reply.
Index: kern/sys_pipe.c
===================================================================
RCS file: /cvs/src/sys/kern/sys_pipe.c,v
retrieving revision 1.126
diff -u -p -r1.126 sys_pipe.c
--- kern/sys_pipe.c 30 Dec 2020 17:02:32 -0000 1.126
+++ kern/sys_pipe.c 21 Oct 2021 18:47:16 -0000
@@ -890,16 +890,27 @@ pipe_kqfilter(struct file *fp, struct kn
kn->kn_hook = rpipe;
klist_insert_locked(&rpipe->pipe_sel.si_note, kn);
break;
- case EVFILT_WRITE:
+ case EVFILT_WRITE: {
+ struct pipe *kpipe = wpipe;
+
if (wpipe == NULL) {
- /* other end of pipe has been closed */
- error = EPIPE;
- break;
+ if (kn->kn_flags & __EV_POLL) {
+ /*
+ * select(2) semantics requires the pipe to
+ * become ready only to deliver EPIPE.
+ */
+ kpipe = rpipe;
+ } else {
+ /* other end of pipe has been closed */
+ error = EPIPE;
+ break;
+ }
}
kn->kn_fop = &pipe_wfiltops;
- kn->kn_hook = wpipe;
- klist_insert_locked(&wpipe->pipe_sel.si_note, kn);
+ kn->kn_hook = kpipe;
+ klist_insert_locked(&kpipe->pipe_sel.si_note, kn);
break;
+ }
default:
error = EINVAL;
}
