NetBSD Problem Report #59056

From www@netbsd.org  Fri Feb  7 19:36:44 2025
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id A52081A923A
	for <gnats-bugs@gnats.NetBSD.org>; Fri,  7 Feb 2025 19:36:44 +0000 (UTC)
Message-Id: <20250207193643.8A55D1A923C@mollari.NetBSD.org>
Date: Fri,  7 Feb 2025 19:36:43 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: poll POLLHUP bugs
X-Send-Pr-Version: www-1.0

>Number:         59056
>Category:       kern
>Synopsis:       poll POLLHUP bugs
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Feb 07 19:40:01 +0000 2025
>Last-Modified:  Mon Feb 10 02:45:01 +0000 2025
>Originator:     Taylor R Campbell
>Release:        
>Organization:
The NotPolleD Foundation
>Environment:
>Description:
If you open a {pipe, socketpair} and close the read side (either side for socket pair), polling the write side for POLLOUT returns:

- POLLHUP|POLLOUT, for pipe;
- POLLOUT, for socketpair.

The pipe case is wrong because POLLHUP and POLLOUT are mutually exclusive in POSIX:

POLLHUP
    A device has been disconnected, or a pipe or FIFO has been closed by the last process that had it open for writing. [...] This event and POLLOUT are mutually-exclusive; a stream can never be writable if a hangup has occurred.

https://pubs.opengroup.org/onlinepubs/9699919799/functions/poll.html

The socketpair is also wrong because any attempt to write to the socket triggers SIGPIPE/EPIPE, so surely POLLHUP must be set for it.
>How-To-Repeat:
#include <sys/socket.h>

#include <err.h>
#include <poll.h>
#include <signal.h>
#include <stdio.h>
#include <unistd.h>

static void
ignoresig(int signo)
{
}

int
main(void)
{
	int s[2];
	struct pollfd pfd;
	int i, nfds;

	if (signal(SIGPIPE, ignoresig) == SIG_ERR)
		err(1, "signal(SIGPIPE)");

	if (socketpair(AF_LOCAL, SOCK_STREAM, 0, s) == -1)
		err(1, "socketpair");
//	if (pipe(s) == -1)
//		err(1, "pipe");
	pfd = (struct pollfd){ .fd = s[1], .events = POLLOUT };
	nfds = poll(&pfd, 1, 0);
	if (nfds == -1)
		err(1, "poll");
	printf("nfds=%d\n", nfds);
	printf("[%d] .fd=%d .revents=0x%x\n", 0, pfd.fd, pfd.revents);
	if (close(s[0]) == -1)
		err(1, "close");
	pfd = (struct pollfd){ .fd = s[1], .events = POLLOUT };
	nfds = poll(&pfd, 1, 0);
	if (nfds == -1)
		err(1, "poll");
	printf("nfds=%d\n", nfds);
	printf("[%d] .fd=%d .revents=0x%x\n", 0, pfd.fd, pfd.revents);

	fflush(stdout);

	char ch = 0;
	if (write(s[1], &ch, 1) == -1)
		err(1, "write");

	return ferror(stdout);
}

>Fix:
Yes, please!

>Audit-Trail:
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59056 CVS commit: src/tests/lib/libc/sys
Date: Sun, 9 Feb 2025 17:09:51 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Sun Feb  9 17:09:51 UTC 2025

 Modified Files:
 	src/tests/lib/libc/sys: t_poll.c

 Log Message:
 t_poll: Make sure POLLOUT is clear when testing POLLHUP.

 POLLOUT and POLLHUP are mutually exclusive in POSIX.  Also, we're
 only polling for POLLIN anyway.

 PR kern/59056: poll POLLHUP bugs


 To generate a diff of this commit:
 cvs rdiff -u -r1.8 -r1.9 src/tests/lib/libc/sys/t_poll.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59056 CVS commit: src/tests/lib/libc/sys
Date: Sun, 9 Feb 2025 17:10:23 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Sun Feb  9 17:10:23 UTC 2025

 Modified Files:
 	src/tests/lib/libc/sys: t_poll.c

 Log Message:
 t_poll: Tweak to improve diagnostics.

 PR kern/59056: poll POLLHUP bugs


 To generate a diff of this commit:
 cvs rdiff -u -r1.9 -r1.10 src/tests/lib/libc/sys/t_poll.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59056 CVS commit: src/tests/lib/libc/sys
Date: Sun, 9 Feb 2025 17:10:37 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Sun Feb  9 17:10:37 UTC 2025

 Modified Files:
 	src/tests/lib/libc/sys: Makefile t_poll.c

 Log Message:
 t_poll: Expand tests of polling when other endpoint is closed.

 This covers fifos, pipes, and sockets; reads and writes; and polling
 when the endpoint is already closed as well as sleeping until the
 endpoint is closed after a delay.

 Unfortunately, there is disagreement right now between:

 (a) POSIX,
 (b) our poll(2) man page, and
 (c) our implementation,

 for many of these cases.  And we recently changed the behaviour of
 poll(2) on fifos between 9 and 10 (PR kern/56429: poll(2) should
 yield POLLHUP when last writer to a fifo close()'s it) but didn't get
 it quite right.

 So I've annotated these test cases with xfails for all the cases I
 think we're currently doing wrong -- under a justification from some
 combination of POSIX, our own documentation, and/or a reasonable
 interpretation of what the semantics _should_ be based on how read(2)
 and write(2) will behave.  These are mostly edge cases:

 1. [pollclosed_pipe_*_write*] When the read side of a pipe is closed,
    poll(2) on the write side currently returns POLLHUP|POLLOUT, which
    POSIX forbids because it requires POLLHUP to be mutually exclusive
    with POLLOUT.  And our man page concurs with POSIX on this rule,
    and has for a long time (going as far back as NetBSD 1.3 or
    earlier, judging by https://man.NetBSD.org/NetBSD-1.3/poll.2).

    => Fixing this won't change whether anything wakes up earlier or
       later -- it will only change which bits are set when the
       wakeups happen.

 2. [pollclosed_fifo*_immediate_readsome] When the write side of a
    fifo was previously open with the read side, and has since been
    closed before calling poll(2), poll(2) on the read side returns 0
    instead of POLLHUP|POLLIN as it is expected to, even though
    read(2) will return EOF without blocking.

    => Fixing this may lead to more wakeups than before, but only in
       cases where read(2) would actually return immediately anyway.

 3. [pollclosed_fifo*_delayed_*_read] When the write side of a fifo is
    closed, poll(2) on the read side returns POLLHUP|POLLIN, as it is
    expected to.  But this state currently isn't persistent, even
    though the behaviour of read(2) in returning EOF without blocking
    is persistent.

    => Fixing this may lead to more wakeups than before, but only in
       cases where read(2) would actually return immediately anyway.

 That said, it turns out that we are correct, according to POSIX, in
 not setting POLLHUP on a socket whose peer has been closed: POLLHUP
 is only for devices, pipes, and FIFOs.  So one of the issues I
 reported in PR 59056 turns out to be bogus.  (Also POLLHUP is only
 for the reader side anyway, not for the writer side.)

 https://pubs.opengroup.org/onlinepubs/9799919799/functions/poll.html

 PR kern/59056: poll POLLHUP bugs


 To generate a diff of this commit:
 cvs rdiff -u -r1.77 -r1.78 src/tests/lib/libc/sys/Makefile
 cvs rdiff -u -r1.10 -r1.11 src/tests/lib/libc/sys/t_poll.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59056 CVS commit: src/tests/lib/libc/sys
Date: Mon, 10 Feb 2025 02:41:34 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Mon Feb 10 02:41:34 UTC 2025

 Modified Files:
 	src/tests/lib/libc/sys: t_poll.c

 Log Message:
 t_poll: Extend the hangup tests to ptys too.

 Of course, ptys work slightly differently from pipes and fifos and
 sockets, presumably for hysterical raisins...

 While here: add a more detailed comment to each xfail for the reason
 why I think it is broken.

 PR kern/59056: poll POLLHUP bugs


 To generate a diff of this commit:
 cvs rdiff -u -r1.11 -r1.12 src/tests/lib/libc/sys/t_poll.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Home	PR Database Search