* Re: [PATCH net] net: sock: prevent integer overflow in sock_reserve_memory()
2026-07-11 0:59 [PATCH net] net: sock: prevent integer overflow in sock_reserve_memory() Xiang Mei (Microsoft)
@ 2026-07-11 13:13 ` Kuniyuki Iwashima
2026-07-21 19:28 ` Jakub Kicinski
1 sibling, 0 replies; 3+ messages in thread
From: Kuniyuki Iwashima @ 2026-07-11 13:13 UTC (permalink / raw)
To: Xiang Mei (Microsoft)
Cc: Eric Dumazet, Paolo Abeni, Willem de Bruijn, David S . Miller,
Jakub Kicinski, Simon Horman, Wei Wang, netdev, linux-kernel,
AutonomousCodeSecurity, tgopinath, kys
On Fri, Jul 10, 2026 at 6:00 PM Xiang Mei (Microsoft) <xmei5@asu.edu> wrote:
>
> sock_reserve_memory() adds 'pages << PAGE_SHIFT' (a plain int) to the int
> sk->sk_forward_alloc. An unprivileged SO_RESERVE_MEM caller can drive the
> accumulating counter past INT_MAX and wrap it negative, either in one
> INT_MAX request (0x80000000) or across several smaller ones.
> The corrupted sk_forward_alloc then trips WARN_ON_ONCE() in
> inet_sock_destruct() on close (a panic under panic_on_warn/oops=panic).
>
> Bound the field that overflows: compute the new sk_forward_alloc in u64 and
> reject with -EINVAL anything exceeding INT_MAX.
>
> Kernel panic - not syncing: kernel: panic_on_warn set ...
> __warn (kernel/panic.c:1054)
> ...
> RIP: 0010:inet_sock_destruct (net/ipv4/af_inet.c:161)
> __sk_destruct (net/core/sock.c:2357)
> inet_release (net/ipv4/af_inet.c:442)
> sock_close (net/socket.c:1501)
> __fput (fs/file_table.c:512)
> __x64_sys_close (fs/open.c:1511)
> do_syscall_64 (arch/x86/entry/syscall_64.c:94)
>
> Fixes: 2bb2f5fb21b0 ("net: add new socket option SO_RESERVE_MEM")
> Reported-by: AutonomousCodeSecurity@microsoft.com
> Signed-off-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net] net: sock: prevent integer overflow in sock_reserve_memory()
2026-07-11 0:59 [PATCH net] net: sock: prevent integer overflow in sock_reserve_memory() Xiang Mei (Microsoft)
2026-07-11 13:13 ` Kuniyuki Iwashima
@ 2026-07-21 19:28 ` Jakub Kicinski
1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2026-07-21 19:28 UTC (permalink / raw)
To: xmei5
Cc: Jakub Kicinski, edumazet, kuniyu, pabeni, willemb, davem, horms,
weiwan, netdev, linux-kernel, AutonomousCodeSecurity, tgopinath,
kys
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
--- Patch [1]: [PATCH net] net: sock: prevent integer overflow in sock_reserve_memory() ---
commit 16cb32b81c740c27028a52ef3c27a62623c808ad
Author: Xiang Mei (Microsoft) <xmei5@asu.edu>
net: sock: prevent integer overflow in sock_reserve_memory()
This commit attempts to prevent an integer overflow in sk_forward_alloc by
computing the new value in u64 and rejecting allocations that exceed
INT_MAX.
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 8a59bfaa8096e..2ba95bead1aed 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1042,6 +1042,9 @@ static int sock_reserve_memory(struct sock *sk, int bytes)
>
> pages = sk_mem_pages(bytes);
>
> + if ((u64)sk->sk_forward_alloc + ((u64)pages << PAGE_SHIFT) > INT_MAX)
> + return -EINVAL;
> +
[Severity: Medium]
Does casting sk_forward_alloc to u64 sign-extend negative values?
Since sk_forward_alloc is a signed int and can legitimately be negative
(e.g., due to over-reclaim), casting it to u64 when negative produces a
massive value (e.g., -4096 becomes 0xfffffffffffff000).
If a user requests a small reservation such that sk_forward_alloc +
(pages << PAGE_SHIFT) remains negative, the 64-bit sum remains a large
unsigned value greater than INT_MAX.
Does this falsely trigger the > INT_MAX check, rejecting valid reservations
with -EINVAL and causing a functional denial of service for the socket?
[Severity: High]
This is a pre-existing issue, but does checking sk_forward_alloc allow
a state-change (TOCTOU) bypass?
sk_forward_alloc dynamically decreases when packets are queued on the
socket, as memory is temporarily moved to sk_rmem_alloc or
sk_wmem_queued.
Could an attacker reserve memory near INT_MAX, queue packets to reduce
sk_forward_alloc, and then call SO_RESERVE_MEM again? The temporarily
reduced sk_forward_alloc allows the check to pass.
When the attacker reads or drops the packets, sk_mem_uncharge()
unconditionally adds the memory back, overflowing sk_forward_alloc past
INT_MAX into a negative value.
On socket destruction, this negative value bypasses sk_mem_reclaim(),
leaving a non-zero balance that triggers a WARN_ON_ONCE kernel panic in
inet_sock_destruct().
Should this check use the total tracked sk->sk_reserved_mem instead of
the fluctuating sk_forward_alloc?
[Severity: High]
This is a pre-existing issue, but does sk_forward_alloc modification in
this function race with UDP receive paths?
sock_reserve_memory() modifies sk_forward_alloc using the non-atomic
WRITE_ONCE() helper via sk_forward_alloc_add() under lock_sock().
For UDP sockets, receive-side modifications (like udp_rmem_release())
occur in softirq without holding lock_sock() (using
sk_receive_queue.lock).
Can an unprivileged user repeatedly call setsockopt(SO_RESERVE_MEM) while
receiving UDP packets, causing a read-modify-write data race?
Could these lost updates corrupt socket memory accounting, leading to memory
limit bypasses or triggering panics during socket destruction?
> /* pre-charge to memcg */
> charged = mem_cgroup_sk_charge(sk, pages,
> GFP_KERNEL | __GFP_RETRY_MAYFAIL);
--
pw-bot: cr
^ permalink raw reply [flat|nested] 3+ messages in thread