netdev - Re: [PATCH net] rds: Fix endian annotations across various assignments

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250810210058.GP222315@ZenIV>
Date: Sun, 10 Aug 2025 22:00:58 +0100
From: Al Viro <viro@...iv.linux.org.uk>
To: Ujwal Kundur <ujwal.kundur@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, allison.henderson@...cle.com,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, horms@...nel.org, netdev@...r.kernel.org,
	linux-rdma@...r.kernel.org, rds-devel@....oracle.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] rds: Fix endian annotations across various
 assignments

On Mon, Aug 11, 2025 at 01:01:01AM +0530, Ujwal Kundur wrote:

> > It took me about 60 seconds to prove the POLLERR change was wrong, and
> > i know nothing about this code base. So it is in fact not a lot of
> > effort.
> I looked up the definition of POLLERR on Elixir [1] and it seemed like
> a valid Sparse report to me. I wasn't aware of EPOLLERR, and now
> realize all the other operations are prefixed with EPOLL* in af_rds.c.
> I look forward to reviews/critiques to learn from them but being
> accused of using LLMs is kinda disheartening.

As for the POLLERR part of that, the thing about POLL* constants is that
beyond the first 6 (IN/PRI/OUT/ERR/HUP/NVAL) they are arch-dependent,
and not just in a sense of bit assignments.

generic:
IN  PRI  OUT  ERR  HUP  NVAL  RDNORM  RDBAND WRNORM  WRBAND  MSG  REMOVE  RDHUP
0   1    2    3    4    5     6       7      8       9       10   11      12
sparc:
0   1    2    3    4    5     6       7      =OUT    8       9    10      11
mips,m68k:
0   1    2    3    4    5     6       7      =OUT    8       10   11      12
xtensa:
0   1    2    3    4    5     6       7      =OUT    8       10   13      12

So these get mapped from/to by poll(2) (mangle_poll() and demangle_poll()
resp.) and __poll_t serves as a tool for catching the places that might
be confused.  The internal values (also used by eventpoll(2)) are

0   1    2    3    4    5     6       7      8       9       10   ---     12

POLLREMOVE is Solaris-only thing and we do not even attempt to implement
it.  So's POLLMSG, but there's a bit of a twist - nobody ever returns
that shit from ->poll(), but SIGIO from dnotify and from lease breaking
comes with ->si_band in siginfo set to POLLIN|POLLRDNORM|POLLMSG;

Warning about __poll_t is usually "mixing POLL... and EPOLL... is a bad idea".
Here it's not a bug (note that ERR gets the same bit in all of the above),
but it's trivial to annotate properly...