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

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: <CAAVpQUBAC754ht-EJJpcBBf3HB3h_W500KFeUgtUPpm2AmmDkw@mail.gmail.com>
Date: Thu, 10 Jul 2025 19:39:16 -0700
From: Kuniyuki Iwashima <kuniyu@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com, 
	pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org, 
	Marek Szyprowski <m.szyprowski@...sung.com>
Subject: Re: [PATCH net] netlink: make sure we allow at least one dump skb

On Thu, Jul 10, 2025 at 5:11 PM Jakub Kicinski <kuba@...nel.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@...sung.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@...sung.com>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>

Thanks for the quick fix!

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

> ---
> CC: kuniyu@...gle.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
>