[PATCH net] netlink: make sure we allow at least one dump skb

[PATCH net] netlink: make sure we allow at least one dump skb

[Jakub Kicinski]

Commit under Fixes tightened up the memory accounting for Netlink
sockets. Looks like the accounting is too strict for some existing
use cases, Marek reported issues with nl80211 / WiFi iw CLI.

To reduce number of iterations Netlink dumps try to allocate
messages based on the size of the buffer passed to previous
recvmsg() calls. If user space uses a larger buffer in recvmsg()
than sk_rcvbuf we will allocate an skb we won't be able to queue.

Make sure we always allow at least one skb to be queued.
Same workaround is already present in netlink_attachskb().
Alternative would be to cap the allocation size to
  rcvbuf - rmem_alloc
but as I said, the workaround is already present in other places.

Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Link: https://lore.kernel.org/9794af18-4905-46c6-b12c-365ea2f05858@samsung.com
Fixes: ae8f160e7eb2 ("netlink: Fix wraparounds of sk->sk_rmem_alloc.")
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: kuniyu@google.com
---
 net/netlink/af_netlink.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 79fbaf7333ce..aeb05d99e016 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2258,11 +2258,11 @@ static int netlink_dump(struct sock *sk, bool lock_taken)
 	struct netlink_ext_ack extack = {};
 	struct netlink_callback *cb;
 	struct sk_buff *skb = NULL;
+	unsigned int rmem, rcvbuf;
 	size_t max_recvmsg_len;
 	struct module *module;
 	int err = -ENOBUFS;
 	int alloc_min_size;
-	unsigned int rmem;
 	int alloc_size;
 
 	if (!lock_taken)
@@ -2294,8 +2294,9 @@ static int netlink_dump(struct sock *sk, bool lock_taken)
 	if (!skb)
 		goto errout_skb;
 
+	rcvbuf = READ_ONCE(sk->sk_rcvbuf);
 	rmem = atomic_add_return(skb->truesize, &sk->sk_rmem_alloc);
-	if (rmem >= READ_ONCE(sk->sk_rcvbuf)) {
+	if (rmem != skb->truesize && rmem >= rcvbuf) {
 		atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
 		goto errout_skb;
 	}
-- 
2.50.0

Re: [PATCH net] netlink: make sure we allow at least one dump skb

[Kuniyuki Iwashima]

On Thu, Jul 10, 2025 at 5:11 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> Commit under Fixes tightened up the memory accounting for Netlink
> sockets. Looks like the accounting is too strict for some existing
> use cases, Marek reported issues with nl80211 / WiFi iw CLI.
>
> To reduce number of iterations Netlink dumps try to allocate
> messages based on the size of the buffer passed to previous
> recvmsg() calls. If user space uses a larger buffer in recvmsg()
> than sk_rcvbuf we will allocate an skb we won't be able to queue.
>
> Make sure we always allow at least one skb to be queued.
> Same workaround is already present in netlink_attachskb().
> Alternative would be to cap the allocation size to
>   rcvbuf - rmem_alloc
> but as I said, the workaround is already present in other places.
>
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Link: https://lore.kernel.org/9794af18-4905-46c6-b12c-365ea2f05858@samsung.com
> Fixes: ae8f160e7eb2 ("netlink: Fix wraparounds of sk->sk_rmem_alloc.")
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Thanks for the quick fix!

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

> ---
> CC: kuniyu@google.com
> ---
>  net/netlink/af_netlink.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 79fbaf7333ce..aeb05d99e016 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -2258,11 +2258,11 @@ static int netlink_dump(struct sock *sk, bool lock_taken)
>         struct netlink_ext_ack extack = {};
>         struct netlink_callback *cb;
>         struct sk_buff *skb = NULL;
> +       unsigned int rmem, rcvbuf;
>         size_t max_recvmsg_len;
>         struct module *module;
>         int err = -ENOBUFS;
>         int alloc_min_size;
> -       unsigned int rmem;
>         int alloc_size;
>
>         if (!lock_taken)
> @@ -2294,8 +2294,9 @@ static int netlink_dump(struct sock *sk, bool lock_taken)
>         if (!skb)
>                 goto errout_skb;
>
> +       rcvbuf = READ_ONCE(sk->sk_rcvbuf);
>         rmem = atomic_add_return(skb->truesize, &sk->sk_rmem_alloc);
> -       if (rmem >= READ_ONCE(sk->sk_rcvbuf)) {
> +       if (rmem != skb->truesize && rmem >= rcvbuf) {
>                 atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
>                 goto errout_skb;
>         }
> --
> 2.50.0
>

Re: [PATCH net] netlink: make sure we allow at least one dump skb

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 10 Jul 2025 17:11:21 -0700 you wrote:
> Commit under Fixes tightened up the memory accounting for Netlink
> sockets. Looks like the accounting is too strict for some existing
> use cases, Marek reported issues with nl80211 / WiFi iw CLI.
> 
> To reduce number of iterations Netlink dumps try to allocate
> messages based on the size of the buffer passed to previous
> recvmsg() calls. If user space uses a larger buffer in recvmsg()
> than sk_rcvbuf we will allocate an skb we won't be able to queue.
> 
> [...]

Here is the summary with links:
  - [net] netlink: make sure we allow at least one dump skb
    https://git.kernel.org/netdev/net/c/a215b5723922

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html