On Mon, Dec 06, 2021 at 05:17:21PM +0000, Visa Hankala wrote:
> This patch adjusts the EVFILT_EXCEPT code of sockets and FIFOs so that
> it would raise the HUP condition only when the channel has been closed
> from both sides. This should match better with the POLLHUP case of
> soo_poll() and fifo_poll().
>
> The "poll index ... unclaimed" error seen by some was related to this.
> With pfd[i].events = 0, the EVFILT_EXCEPT filter triggered prematurely
> when the socket was only half-closed.
>
> When comparing the code, note that POLL_NOHUP was set by the old
> select(2) code with writefds. The kqueue-based code achieves the same
> effect with FIFOs by not raising HUP in filt_fifowrite().
>
> The resulting code is possibly overly contrived. The SS_CANTRCVMORE
> condition alone is irrelevant for poll and select. It could be removed
> to simplify the code, but then socket and FIFO's EVFILT_EXCEPT would
> behave inconsistently relative to EV_EOF. However, only NOTE_OOB has
> been documented and it skips SS_CANTRCVMORE...
Here is a revised patch for EVFILT_EXCEPT handlers. Changes:
* Disallow use of EVFILT_EXCEPT through kevent(2) with FIFOs and pipes.
These file types do not implement out-of-band data that userspace
could query.
* Do not return EOF condition to userspace from EVFILT_EXCEPT code.
The relevant code blocks are now conditional to __EV_POLL and only
set __EV_HUP.
These items prevent unintended "leakage" of functionality to userspace.
Also note that ptys and sockets can now return both NOTE_OOB and __EV_HUP
at the same time. This was present in the initial revision.
OK?
Index: kern/sys_pipe.c
===================================================================
RCS file: src/sys/kern/sys_pipe.c,v
retrieving revision 1.131
diff -u -p -r1.131 sys_pipe.c
--- kern/sys_pipe.c 8 Dec 2021 13:03:52 -0000 1.131
+++ kern/sys_pipe.c 11 Dec 2021 12:36:06 -0000
@@ -927,6 +927,11 @@ pipe_kqfilter(struct file *fp, struct kn
error = EPERM;
break;
}
+ if ((kn->kn_flags & __EV_POLL) == 0) {
+ /* Disallow usage through kevent(2). */
+ error = EINVAL;
+ break;
+ }
kn->kn_fop = &pipe_efiltops;
kn->kn_hook = rpipe;
klist_insert_locked(&rpipe->pipe_sel.si_note, kn);
@@ -1074,19 +1079,20 @@ int
filt_pipeexcept_common(struct knote *kn, struct pipe *rpipe)
{
struct pipe *wpipe;
+ int active = 0;
rw_assert_wrlock(rpipe->pipe_lock);
wpipe = pipe_peer(rpipe);
- if ((rpipe->pipe_state & PIPE_EOF) || wpipe == NULL) {
- kn->kn_flags |= EV_EOF;
- if (kn->kn_flags & __EV_POLL)
+ if (kn->kn_flags & __EV_POLL) {
+ if ((rpipe->pipe_state & PIPE_EOF) || wpipe == NULL) {
kn->kn_flags |= __EV_HUP;
- return (1);
+ active = 1;
+ }
}
- return (0);
+ return (active);
}
int
Index: kern/tty_pty.c
===================================================================
RCS file: src/sys/kern/tty_pty.c,v
retrieving revision 1.110
diff -u -p -r1.110 tty_pty.c
--- kern/tty_pty.c 24 Oct 2021 00:02:25 -0000 1.110
+++ kern/tty_pty.c 11 Dec 2021 12:36:06 -0000
@@ -727,6 +734,7 @@ filt_ptcexcept(struct knote *kn, long hi
{
struct pt_softc *pti = (struct pt_softc *)kn->kn_hook;
struct tty *tp;
+ int active = 0;
tp = pti->pt_tty;
@@ -736,18 +744,18 @@ filt_ptcexcept(struct knote *kn, long hi
((pti->pt_flags & PF_UCNTL) && pti->pt_ucntl)) {
kn->kn_fflags |= NOTE_OOB;
kn->kn_data = 1;
- return (1);
+ active = 1;
}
- return (0);
}
- if (!ISSET(tp->t_state, TS_CARR_ON)) {
- kn->kn_flags |= EV_EOF;
- if (kn->kn_flags & __EV_POLL)
+
+ if (kn->kn_flags & __EV_POLL) {
+ if (!ISSET(tp->t_state, TS_CARR_ON)) {
kn->kn_flags |= __EV_HUP;
- return (1);
+ active = 1;
+ }
}
- return (0);
+ return (active);
}
const struct filterops ptcread_filtops = {
Index: kern/uipc_socket.c
===================================================================
RCS file: src/sys/kern/uipc_socket.c,v
retrieving revision 1.269
diff -u -p -r1.269 uipc_socket.c
--- kern/uipc_socket.c 11 Nov 2021 16:35:09 -0000 1.269
+++ kern/uipc_socket.c 11 Dec 2021 12:36:06 -0000
@@ -2263,14 +2263,13 @@ filt_soexcept_common(struct knote *kn, s
kn->kn_data -= so->so_oobmark;
rv = 1;
}
- } else if (so->so_state & SS_CANTRCVMORE) {
- kn->kn_flags |= EV_EOF;
- if (kn->kn_flags & __EV_POLL) {
- if (so->so_state & SS_ISDISCONNECTED)
- kn->kn_flags |= __EV_HUP;
+ }
+
+ if (kn->kn_flags & __EV_POLL) {
+ if (so->so_state & SS_ISDISCONNECTED) {
+ kn->kn_flags |= __EV_HUP;
+ rv = 1;
}
- kn->kn_fflags = so->so_error;
- rv = 1;
}
return rv;
Index: miscfs/fifofs/fifo_vnops.c
===================================================================
RCS file: src/sys/miscfs/fifofs/fifo_vnops.c,v
retrieving revision 1.87
diff -u -p -r1.87 fifo_vnops.c
--- miscfs/fifofs/fifo_vnops.c 11 Dec 2021 09:28:26 -0000 1.87
+++ miscfs/fifofs/fifo_vnops.c 11 Dec 2021 12:36:06 -0000
@@ -544,6 +544,10 @@ fifo_kqfilter(void *v)
/* Prevent triggering exceptfds. */
return (EPERM);
}
+ if ((ap->a_kn->kn_flags & __EV_POLL) == 0) {
+ /* Disallow usage through kevent(2). */
+ return (EINVAL);
+ }
ap->a_kn->kn_fop = &fifoexcept_filtops;
so = fip->fi_readsock;
sb = &so->so_rcv;
@@ -704,13 +708,11 @@ filt_fifoexcept_common(struct knote *kn,
soassertlocked(so);
- if (so->so_state & SS_CANTRCVMORE) {
- kn->kn_flags |= EV_EOF;
- if (kn->kn_flags & __EV_POLL) {
- if (so->so_state & SS_ISDISCONNECTED)
- kn->kn_flags |= __EV_HUP;
+ if (kn->kn_flags & __EV_POLL) {
+ if (so->so_state & SS_ISDISCONNECTED) {
+ kn->kn_flags |= __EV_HUP;
+ rv = 1;
}
- rv = 1;
}
return (rv);
