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
| ||
|
Message-ID: <2229fd4a-a65f-28f8-333f-26a6a1236d52@mojatatu.com> Date: Mon, 29 May 2023 11:37:12 -0300 From: Pedro Tammela <pctammela@...atatu.com> To: Jakub Kicinski <kuba@...nel.org> Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com, kuniyu@...zon.com, dh.herrmann@...il.com, jhs@...atatu.com Subject: Re: [PATCH net] net/netlink: fix NETLINK_LIST_MEMBERSHIPS group array length check On 29/05/2023 03:40, Jakub Kicinski wrote: > On Sat, 27 May 2023 12:01:25 -0300 Pedro Tammela wrote: >> On 27/05/2023 00:33, Jakub Kicinski wrote: >>> On Thu, 25 May 2023 11:46:09 -0300 Pedro Tammela wrote: >>>> For the socket option 'NETLINK_LIST_MEMBERSHIPS' the length is defined >>>> as the number of u32 required to represent the whole bitset. >>> >>> I don't think it is, it's a getsockopt() len is in bytes. >> >> Unfortunately the man page seems to be ambiguous (Emphasis added): >> >> NETLINK_LIST_MEMBERSHIPS (since Linux 4.2) >> Retrieve all groups a socket is a member of. optval is a >> pointer to __u32 and *optlen is the size of the array*. The >> array is filled with the full membership set of the >> socket, and the required array size is returned in optlen. >> >> Size of the array in bytes? in __u32? > > Indeed ambiguous, in C "size of array" could as well refer to sizeof() > or ARRAY_SIZE().. > >> SystemD seems to be expecting the size in __u32 chunks: >> https://github.com/systemd/systemd/blob/9c9b9b89151c3e29f3665e306733957ee3979853/src/libsystemd/sd-netlink/netlink-socket.c#L37 >> >> But then looking into the getsockopt manpage we see (Ubuntu 23.04): >> >> int getsockopt(int sockfd, int level, int optname, >> void optval[restrict *.optlen], >> socklen_t *restrict optlen); >> >> >> So it seems like getsockopt() asks for optlen to be, in this case, __u32 >> chunks? > > Why so? It's a far fetched interpretation of the function signature in the man page but someone could argue that it's trying to emulate a VLA style function prototype over a generic optval. But let's not waste precious time in this discussion. > >> [...] > > I don't know of any other case where socklen_t would refer to something > else than bytes, I'm leaning towards addressing the truncation (and if > systemd thinks the value is in u32s potentially also fixing system, not > that over-allocating will hurt its correctness). OK! Will re-spin to net-next so people have plenty of time to adjust
Powered by blists - more mailing lists