Re: [PATCH net 1/2] netlink: retry a dump skb that does not fit with the minimum size

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete: the ENOBUFS livelock it targets is still
  reachable whenever a minimum-size dump skb also does not fit.

--

netlink: retry a dump skb that does not fit with the minimum size

When the skb sized after max_recvmsg_len is rejected by the sk_rcvbuf
admission check, netlink_dump() now frees it and retries once with an
alloc_min_size skb. The goal is to fix a livelock added by commit
ae8f160e7eb2, where a reader that peeks before reading, such as libnl,
spins on ENOBUFS.

> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index a904dc3bcad6b..b750e79fed2ed 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -2318,10 +2318,21 @@ static int netlink_dump(struct sock *sk, bool lock_taken)
>  	if (!skb)
>  		goto errout_skb;
>  
> +retry:
>  	rcvbuf = READ_ONCE(sk->sk_rcvbuf);
>  	rmem = atomic_add_return(skb->truesize, &sk->sk_rmem_alloc);
>  	if (rmem != skb->truesize && rmem >= rcvbuf) {
>  		atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
> +		/* A skb sized after max_recvmsg_len may not fit next to data
> +		 * still charged to the socket, while a minimum size one does.
> +		 */
> +		if (alloc_size > alloc_min_size) {
> +			kfree_skb(skb);
> +			alloc_size = alloc_min_size;
> +			skb = alloc_skb(alloc_size, GFP_KERNEL);
> +			if (skb)
> +				goto retry;
> +		}
>  		goto errout_skb;
>  	}

[Severity: High]
Does this fully close the livelock described in the commit message?

The admission rule is unchanged. The minimum size skb is still rejected
when rmem_before != 0 and rmem_before + T_min >= sk_rcvbuf, where T_min
is the truesize of an alloc_min_size skb. The comment says a minimum
size skb "does" fit, but nothing here guarantees that.

On the recvmsg side, the dump is only re-run once rmem drops to half of
sk_rcvbuf:

net/netlink/af_netlink.c:netlink_recvmsg() {
    ...
	if (READ_ONCE(nlk->cb_running) &&
	    atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf / 2) {
		ret = netlink_dump(sk, false);
		if (ret) {
			WRITE_ONCE(sk->sk_err, -ret);
			sk_error_report(sk);
		}
	}
    ...
}

When sk_rcvbuf < 2 * T_min, that check does not guarantee
rmem + T_min < sk_rcvbuf. A few configurations appear to land in that
window:

- On 4K pages T_min is roughly 4352 bytes. An unprivileged
  SO_RCVBUF=4096 gives sk_rcvbuf=8192 via __sock_set_rcvbuf(). Any
  rmem_before in roughly [3840, 4096] then fails both the large attempt
  and the retry.

- On PAGE_SIZE >= 8K, NLMSG_GOODSIZE is SKB_WITH_OVERHEAD(8192), so
  T_min is roughly 8448. With iw's 16KiB sk_rcvbuf, rmem in roughly
  [7936, 8192] fails both attempts. A few notifications that are still
  charged could reach that range, since do_one_broadcast() holds them
  via skb_get() until netlink_broadcast() returns.

- When cb->min_dump_alloc is at least the capped max_recvmsg_len,
  alloc_size > alloc_min_size is never true and the retry never runs.
  smc_diag_handler_dump() is one example: it uses
  SKB_WITH_OVERHEAD(32768).

In those states the original sequence still looks reachable:

__netlink_dump_start()
  netlink_dump()            -> -ENOBUFS, cb_running stays true
netlink_recvmsg(MSG_PEEK)
  netlink_dump()            -> -ENOBUFS, sk_err = ENOBUFS
netlink_recvmsg()
  __skb_try_recv_datagram()
    sock_error()            -> ENOBUFS returned, nothing dequeued

rmem never changes, so the peek/read loop keeps spinning at 100% CPU.

Before ae8f160e7eb2, the dump skb was admitted whenever
rmem_before < rcvbuf, so a dump triggered from recvmsg could not fail
this way.

Would it be better to relax the admission rule for dump skbs, instead of
adding a second, smaller skb size?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261023-netlink-dump-livelock-v1-0-478214a635d2%40gmail.com