Re: [PATCH net v2 2/2] selftests: net: netlink-dumps: start a dump with a notification queued

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Whether dump_with_queued_ntf
  (tools/testing/selftests/net/netlink-dumps.c) can catch the bug…

--

selftests: net: netlink-dumps: start a dump with a notification queued

This adds a dump_with_queued_ntf test to netlink-dumps.c. The test starts
an RTM_GETLINK dump on a socket with SO_RCVBUF 2048 while an lo link
notification is still queued. It reads like libnl does, peeking before
each read, and expects the dump to finish with a zero status.

> diff --git a/tools/testing/selftests/net/netlink-dumps.c b/tools/testing/selftests/net/netlink-dumps.c
> index 51129c564d0a6..4a94a25167e25 100644
> --- a/tools/testing/selftests/net/netlink-dumps.c
> +++ b/tools/testing/selftests/net/netlink-dumps.c

[ ... ]

> @@ -260,4 +263,103 @@ TEST(close_with_ref)

[ ... ]

> +TEST(dump_with_queued_ntf)
> +{
> +	struct sockaddr_nl addr = {
> +		.nl_family	= AF_NETLINK,
> +		.nl_groups	= RTMGRP_LINK,
> +	};
> +	int netlink_sock, inet_sock, i, ret, links = 0;
> +	int rcvbuf = 2048;

[ ... ]

> +	// Leave one notification queued, then start the dump
> +	ASSERT_EQ(lo_set_up(inet_sock, false), 0);
> +	n = send(netlink_sock, &dump_links, sizeof(dump_links), 0);
> +	ASSERT_EQ(n, sizeof(dump_links));

[Severity: Low]
Does this test catch the bug on every kernel config? The commit message
says "The test fails without the previous patch". That seems to depend on
how much memory the kernel charges for the queued notification, and the
test never checks that.

With SO_RCVBUF 2048, sk_rcvbuf ends up as 4096. Call the truesize of the
queued lo RTM_NEWLINK notification T. Call the truesize of the -ENOBUFS
ack that netlink_ack() sends A. The old check in netlink_dump():

    rmem != skb->truesize && rmem >= rcvbuf

rejects the first large dump skb whenever T > 0. What happens next is
decided in netlink_attachskb():

    if ((rmem == skb->truesize || rmem <= READ_ONCE(sk->sk_rcvbuf)) &&
        !test_bit(NETLINK_S_CONGESTED, &nlk->state)) {

If T + A <= 4096, the ack is queued. The unfixed kernel keeps failing the
dump from netlink_recvmsg(), the peek/read loop never sees NLMSG_DONE,
and EXPECT_TRUE(done) fails as intended.

If T + A > 4096, the ack is dropped and netlink_overrun() is called. Once
the test reads the notification, the socket is empty. The old
rmem == skb->truesize exception then lets the dump go through, so the
test passes on an unfixed kernel. When T >= 4096, the fixed check:

    if (rmem - skb->truesize >= rcvbuf || rmem > INT_MAX) {

also rejects the first skb, so in that case the two kernels behave the
same.

T looks config dependent. netlink_trim() shrinks the notification with
pskb_expand_head() into a kmalloc bucket that has to hold the payload
plus skb_shared_info.

With the default CONFIG_MAX_SKB_FRAGS=17, the lo notification seems to
fit the 2 KiB bucket (T around 2304). With CONFIG_MAX_SKB_FRAGS=45, the
larger skb_shared_info appears to push it into the 4 KiB bucket (T around
4352). These byte sizes are estimates, not runtime measurements.

The same thing could happen in two other cases. One is if the
pskb_expand_head() call in netlink_trim() fails, since it runs without
direct reclaim and with __GFP_NORETRY. The other is if the lo
notification grows in a future kernel.

Would it help to read SO_MEMINFO after lo_set_up(inet_sock, false) and
before send()? The test could then skip, or fail, unless
SK_MEMINFO_RMEM_ALLOC is non-zero and still leaves room for the ack, for
example RMEM_ALLOC <= SK_MEMINFO_RCVBUF / 2. Another option is to pick
SO_RCVBUF based on the measured notification size.

> +
> +	for (i = 0; i < 100 && !done; i++) {

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261027-netlink-dump-livelock-v2-0-c4bb05352a33%40gmail.com