Re: kern/54435 (reading TCP urgent data with MSG_OOB doesn't clear poll(2) status)

[Chuck Silvers <chuq%chuq.com@localhost>]

The following reply was made to PR kern/54435; it has been noted by GNATS.

From: Chuck Silvers <chuq%chuq.com@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: kern-bug-people%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost,
	Valery Ushakov <uwe%stderr.spb.ru@localhost>,
	Taylor R Campbell <riastradh%NetBSD.org@localhost>,
	Michael van Elst <mlelstv%serpens.de@localhost>,
	Robert Elz <kre%munnari.OZ.AU@localhost>,
	Christos Zoulas <christos%zoulas.com@localhost>,
	Rin Okuyama <rokuyama.rk%gmail.com@localhost>,
	matthew green <mrg%eterna.com.au@localhost>
Subject: Re: kern/54435 (reading TCP urgent data with MSG_OOB doesn't clear
 poll(2) status)
Date: Thu, 19 Nov 2020 10:40:44 -0800

 There's still a problem with the change that was committed for this PR.
 With the current code, when recv(MSG_OOB) is used to retrieve the
 OOB data, we clear SS_RCVATMARK immediately, which is fine for poll()
 deciding whether to report POLLRDBAND, but that also means that
 the application can no longer use SIOCATMARK tell where in the
 stream the OOB data was, which is a critical part of using this API.
 If SIOCATMARK always reports "not at the mark", the application will
 think that entire rest of the stream came before the OOB byte that
 was read, which breaks any use of this interface.  This is exactly
 the problem that this change causes for rlogin.
 
 The real underlying problem is that the current code assumes that the
 conditions under which SIOCATMARK should report "at the mark" and the
 conditions under which poll() should report POLLRDBAND are the same,
 but they are actually not the same, since SIOCATMARK must report "at
 the mark" at some point after poll() stops reporting POLLRDBAND for
 this interface to work at all.  We need to add some extra state so
 that these two conditions can each be reported at the correct time.
 I have a patch below that does that.
 
 To demonstrate this, I adjusted uwe's oobrecv program to report the
 results of SIOCATMARK before every recv() call, not just the calls
 that are done when poll() returns POLLRDBAND, and I also have it
 print "not at mark" explicitly just to make the status super-clear.
 Here are the results of "./oobserv nun" and "./oobrecv oob":
 
 
 
 netbsd HEAD
 
 # ./oobrecv oob
 reading urgent data with MSG_OOB
 poll: revents = 0x83: IN PRI RDBAND
 <not at mark>
 recv(MSG_OOB) = 1
 B
 poll: revents = 0x1: IN
 <not at mark>
 recv() = 2
 ac
 poll: revents = 0x1: IN
 <not at mark>
 recv() = 0
 
 
 
 netbsd HEAD + my patch
 
 # ./oobrecv 
 reading urgent data with MSG_OOB
 poll: revents = 0x83: IN PRI RDBAND
 <not at mark>
 recv(MSG_OOB) = 1
 B
 poll: revents = 0x1: IN
 <not at mark>
 recv() = 1
 a
 poll: revents = 0x1: IN
 <at mark>
 recv() = 1
 c
 poll: revents = 0x1: IN
 <not at mark>
 recv() = 0
 
 
 
 linux (5.7.17-200.fc32.x86_64)
 
 # ./oobrecv 
 reading urgent data with MSG_OOB
 poll: revents = 0x3: IN PRI
 <not at mark>
 recv(MSG_OOB) = 1
 B
 poll: revents = 0x1: IN
 <not at mark>
 recv() = 1
 a
 poll: revents = 0x1: IN
 <at mark>
 recv() = 1
 c
 poll: revents = 0x1: IN
 <not at mark>
 recv() = 0
 
 
 
 The netbsd HEAD result is clearly wrong, since we never see "at mark",
 and the mark should actually occur in between the two bytes returned by
 the second recv(), so returning those two bytes together is also wrong.
 The result with my patch is correct, and matches linux.
 
 With the patch, an rlogin binary from netbsd 9.0 also works again.
 
 The patch and modified test program are at:
 https://ftp.netbsd.org/pub/NetBSD/misc/chs/diff.oob.1
 https://ftp.netbsd.org/pub/NetBSD/misc/chs/oobrecv.c
 
 I'll commit the patch this weekend.
 
 -Chuck