netdev - Re: [PATCH net] net: Check for oversized requests in sock

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: <CANn89i+oL+qoPmbbGvE_RT3_3OWgeck7cCPcTafeehKrQZ8kyw@mail.gmail.com>
Date: Mon, 16 Dec 2024 13:43:30 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Michal Luczaj <mhal@...x.co>
Cc: "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH net] net: Check for oversized requests in sock_kmalloc()

On Mon, Dec 16, 2024 at 12:51 PM Michal Luczaj <mhal@...x.co> wrote:
>
> Allocator explicitly rejects requests of order > MAX_PAGE_ORDER, triggering
> a WARN_ON_ONCE_GFP().
>
> Put a size limit in sock_kmalloc().
>
> WARNING: CPU: 6 PID: 1676 at mm/page_alloc.c:4727 __alloc_pages_noprof+0x32e/0x3a0
> Call Trace:
>  ___kmalloc_large_node+0x71/0xf0
>  __kmalloc_large_node_noprof+0x1b/0xf0
>  __kmalloc_noprof+0x436/0x560
>  sock_kmalloc+0x44/0x60
>  ____sys_sendmsg+0x208/0x3a0
>  ___sys_sendmsg+0x84/0xd0
>  __sys_sendmsg+0x56/0xa0
>  do_syscall_64+0x93/0x180
>  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

I would rather change ____sys_sendmsg() to use something different
than sock_kmalloc().

This would avoid false sharing (on sk->sk_omem_alloc) for a short-lived buffer,
and could help UDP senders sharing a socket...

sock_kmalloc() was really meant for small and long lived allocations
(corroborated by net.core.optmem_max small default value)

diff --git a/net/socket.c b/net/socket.c
index 9a117248f18f13d574d099c80128986c744fa97f..c23d8e20c5c626c54b9a04a416b82f696fa2310c
100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2552,7 +2552,8 @@ static int ____sys_sendmsg(struct socket *sock,
struct msghdr *msg_sys,
                BUILD_BUG_ON(sizeof(struct cmsghdr) !=
                             CMSG_ALIGN(sizeof(struct cmsghdr)));
                if (ctl_len > sizeof(ctl)) {
-                       ctl_buf = sock_kmalloc(sock->sk, ctl_len, GFP_KERNEL);
+                       ctl_buf = kvmalloc(ctl_len, GFP_KERNEL_ACCOUNT |
+                                                   __GFP_NOWARN);
                        if (ctl_buf == NULL)
                                goto out;
                }
@@ -2594,7 +2595,7 @@ static int ____sys_sendmsg(struct socket *sock,
struct msghdr *msg_sys,

 out_freectl:
        if (ctl_buf != ctl)
-               sock_kfree_s(sock->sk, ctl_buf, ctl_len);
+               kvfree(ctl_buf);
 out:
        return err;
 }