[PATCH net v2] af_packet: fix integer overflow in prb_calc_retire_blk_tmo()

[PATCH net v2] af_packet: fix integer overflow in prb_calc_retire_blk_tmo()

[Dairui Zhang]

prb_calc_retire_blk_tmo() computes in 32-bit int arithmetic:

        mbits = (blk_size_in_bytes * 8) / (1024 * 1024);

If I'm reading the validation right, tp_block_size is user
controlled and packet_set_ring() only rejects values that are <= 0
as int or not page aligned, so a 256MiB block goes right through
(and alloc_one_pg_vec_page() even has a vzalloc fallback for it).
0x10000000 * 8 wraps to INT_MIN, and on a NIC reporting 1 Gbps
(div == 1) the function ends up returning -2047.

The condition is actually (8 * size) mod 2^32 >= 2^31 && div == 1,
so the trigger set is [256,512), [768,1024), [1280,1536) and
[1792,2048) MiB. Other sizes wrap to non-negative values and faster
links divide the unsigned value back below 2^31, which is why this
doesn't blow up for everyone.

What makes it fatal is what happens next in init_prb_bdqc():

        p1->interval_ktime = ms_to_ktime(prb_calc_retire_blk_tmo(...));
        hrtimer_start(&p1->retire_blk_timer, p1->interval_ktime,
                      HRTIMER_MODE_REL_SOFT);

A negative relative timeout expires immediately. The callback
unconditionally returns HRTIMER_RESTART, and hrtimer_forward() turns
the negative interval into hrtimer_resolution:

        if (interval < hrtimer_resolution)
                interval = hrtimer_resolution;

So the SOFT timer re-fires at the maximum rate forever, holding
sk_receive_queue.lock each pass. One CPU spins in softirq until the
socket is closed. Repeat with more rings and the machine is gone.

The overflow itself is ancient - it was introduced together with
TPACKET_V3 in f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer
implementation."). Its effect prior to f7460d2989fa ("net:
af_packet: Use hrtimer to do the retire operation", v6.18) was not
as clear-cut, though: the return value was stored into an unsigned
short retire_blk_tov, so a negative result was truncated, and a
0-jiffy delay loop could be programmed as well. Neither is nearly
as detrimental as the immediate maximum-rate spin the hrtimer
conversion turned it into.

(Unrelated to CVE-2019-20812 - that one was the ethtool failure path
returning 0, which now returns DEFAULT_PRB_RETIRE_TOV.)

Reproducer, needs CAP_NET_RAW (a --network host container has it by
default) and a 1 Gbps NIC (QEMU e1000 works):

        int fd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
        bind(fd, ...);
        int v = TPACKET_V3;
        setsockopt(fd, SOL_PACKET, PACKET_VERSION, &v, sizeof(v));
        struct tpacket_req3 req = {
                .tp_block_size = 0x10000000,
                .tp_block_nr = 1,
                .tp_frame_size = 2048,
                .tp_frame_nr = 0x10000000 / 2048,
                .tp_retire_blk_tov = 0,
        };
        setsockopt(fd, SOL_PACKET, PACKET_RX_RING, &req, sizeof(req));

Compute in 64 bits instead. The operands are already bounded by the
existing validation, so nothing else changes. If you'd prefer a
different fix, just say so and I'll respin.

Fixes: f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer implementation.")
Cc: stable@vger.kernel.org
Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>
---
v1 -> v2:
- Fixes: now points to f6fb8f100b80, the original TPACKET_V3
  commit, per Willem's review;
- pre-hrtimer effect description corrected (unsigned short
  truncation / 0-jiffy loop, not "timer effectively never fired").
- v1: https://lore.kernel.org/netdev/20260921192608.1420047-1-zhangdairui@gmail.com/

First patch to netdev, and compile-tested only - I don't have a
1 Gbps setup to trigger it live. If I've misread the code, got the
Fixes: tag wrong, or picked the wrong fix, please say so and I'll
respin.

 net/packet/af_packet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index b22cda3..24cf2d2 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -617,7 +617,7 @@ static int prb_calc_retire_blk_tmo(struct packet_sock *po,
 		return DEFAULT_PRB_RETIRE_TOV;
 
 	div = ecmd.base.speed / 1000;
-	mbits = (blk_size_in_bytes * 8) / (1024 * 1024);
+	mbits = (u64)blk_size_in_bytes * 8 / (1024 * 1024);
 
 	if (div)
 		mbits /= div;

Re: [PATCH net v2] af_packet: fix integer overflow in prb_calc_retire_blk_tmo()

[Willem de Bruijn]

Dairui Zhang wrote:
> prb_calc_retire_blk_tmo() computes in 32-bit int arithmetic:
> 
>         mbits = (blk_size_in_bytes * 8) / (1024 * 1024);
> 
> If I'm reading the validation right, tp_block_size is user
> controlled and packet_set_ring() only rejects values that are <= 0
> as int or not page aligned, so a 256MiB block goes right through
> (and alloc_one_pg_vec_page() even has a vzalloc fallback for it).
> 0x10000000 * 8 wraps to INT_MIN, and on a NIC reporting 1 Gbps
> (div == 1) the function ends up returning -2047.
> 
> The condition is actually (8 * size) mod 2^32 >= 2^31 && div == 1,
> so the trigger set is [256,512), [768,1024), [1280,1536) and
> [1792,2048) MiB. Other sizes wrap to non-negative values and faster
> links divide the unsigned value back below 2^31, which is why this
> doesn't blow up for everyone.
> 
> What makes it fatal is what happens next in init_prb_bdqc():
> 
>         p1->interval_ktime = ms_to_ktime(prb_calc_retire_blk_tmo(...));
>         hrtimer_start(&p1->retire_blk_timer, p1->interval_ktime,
>                       HRTIMER_MODE_REL_SOFT);
> 
> A negative relative timeout expires immediately. The callback
> unconditionally returns HRTIMER_RESTART, and hrtimer_forward() turns
> the negative interval into hrtimer_resolution:
> 
>         if (interval < hrtimer_resolution)
>                 interval = hrtimer_resolution;
> 
> So the SOFT timer re-fires at the maximum rate forever, holding
> sk_receive_queue.lock each pass. One CPU spins in softirq until the
> socket is closed. Repeat with more rings and the machine is gone.
> 
> The overflow itself is ancient - it was introduced together with
> TPACKET_V3 in f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer
> implementation."). Its effect prior to f7460d2989fa ("net:
> af_packet: Use hrtimer to do the retire operation", v6.18) was not
> as clear-cut, though: the return value was stored into an unsigned
> short retire_blk_tov, so a negative result was truncated, and a
> 0-jiffy delay loop could be programmed as well. Neither is nearly
> as detrimental as the immediate maximum-rate spin the hrtimer
> conversion turned it into.
> 
> (Unrelated to CVE-2019-20812 - that one was the ethtool failure path
> returning 0, which now returns DEFAULT_PRB_RETIRE_TOV.)
> 
> Reproducer, needs CAP_NET_RAW (a --network host container has it by
> default) and a 1 Gbps NIC (QEMU e1000 works):
> 
>         int fd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
>         bind(fd, ...);
>         int v = TPACKET_V3;
>         setsockopt(fd, SOL_PACKET, PACKET_VERSION, &v, sizeof(v));
>         struct tpacket_req3 req = {
>                 .tp_block_size = 0x10000000,
>                 .tp_block_nr = 1,
>                 .tp_frame_size = 2048,
>                 .tp_frame_nr = 0x10000000 / 2048,
>                 .tp_retire_blk_tov = 0,
>         };
>         setsockopt(fd, SOL_PACKET, PACKET_RX_RING, &req, sizeof(req));
> 
> Compute in 64 bits instead. The operands are already bounded by the
> existing validation, so nothing else changes. If you'd prefer a
> different fix, just say so and I'll respin.
> 
> Fixes: f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer implementation.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Re: [PATCH net v2] af_packet: fix integer overflow in prb_calc_retire_blk_tmo()

[patchwork-bot+netdevbpf]

Hello:

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

On Wed, 23 Sep 2026 13:01:01 +0800 you wrote:
> prb_calc_retire_blk_tmo() computes in 32-bit int arithmetic:
> 
>         mbits = (blk_size_in_bytes * 8) / (1024 * 1024);
> 
> If I'm reading the validation right, tp_block_size is user
> controlled and packet_set_ring() only rejects values that are <= 0
> as int or not page aligned, so a 256MiB block goes right through
> (and alloc_one_pg_vec_page() even has a vzalloc fallback for it).
> 0x10000000 * 8 wraps to INT_MIN, and on a NIC reporting 1 Gbps
> (div == 1) the function ends up returning -2047.
> 
> [...]

Here is the summary with links:
  - [net,v2] af_packet: fix integer overflow in prb_calc_retire_blk_tmo()
    https://git.kernel.org/netdev/net/c/56d82862a0a2

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

Re: [PATCH net v2] af_packet: fix integer overflow in prb_calc_retire_blk_tmo()

[netdev-bot+sashiko]

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

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

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).

- [Medium] The commit message says the negative timeout needs `div == 1`,
  i.e. a 1 Gbps link.
- [Low] The commit message says the reproducer "needs CAP_NET_RAW (a
  --network host container has it by default) and a 1 Gbps NIC (QEMU…

--

af_packet: fix integer overflow in prb_calc_retire_blk_tmo()

This patch makes prb_calc_retire_blk_tmo() compute mbits in 64-bit
arithmetic. The old 32-bit int product could wrap negative for large
TPACKET_V3 block sizes. The negative result then armed the retire hrtimer
with a negative relative timeout, and the timer re-fired forever.

> The condition is actually (8 * size) mod 2^32 >= 2^31 && div == 1,
> so the trigger set is [256,512), [768,1024), [1280,1536) and
> [1792,2048) MiB. Other sizes wrap to non-negative values and faster
> links divide the unsigned value back below 2^31, which is why this
> doesn't blow up for everyone.

[Severity: Medium]
Does the negative return really need div == 1?

In the pre-patch prb_calc_retire_blk_tmo(), a link reporting 2000-2999
Mbps (for example SPEED_2500, so div == 2) can also return a negative
value. The final unsigned increment is what pushes it over:

    div = ecmd.base.speed / 1000;
    mbits = (blk_size_in_bytes * 8) / (1024 * 1024);

    if (div)
        mbits /= div;

    if (div)
        return mbits + 1;

Take tp_block_size = 0x1ffe0000. It is positive as an int and page
aligned, so packet_set_ring() accepts it:

    blk_size_in_bytes * 8    = 0xfff00000 (-1048576 as int)
    / (1024 * 1024)          = -1, so mbits = UINT_MAX
    mbits /= 2               = 0x7fffffff
    return mbits + 1         = 0x80000000 (INT_MIN as int)

ms_to_ktime() takes a u64, so INT_MIN is sign-extended and multiplied by
NSEC_PER_MSEC, which gives a negative ktime. init_prb_bdqc() passes that
to hrtimer_start(). prb_retire_rx_blk_timer_expired() keeps returning
HRTIMER_RESTART, so the timer spins just as it does when div == 1.

More generally, with div == 2 the old int quotient only has to be -1 or
-2. That covers sizes about 128-384 KiB below each 512 MiB multiple
(512, 1024, 1536 and 2048 MiB).

The u64 arithmetic in this patch fixes these sizes too. Could the
description of the trigger condition be corrected, so that stable and
security triage also covers 2.5 Gbps links?

> Reproducer, needs CAP_NET_RAW (a --network host container has it by
> default) and a 1 Gbps NIC (QEMU e1000 works):

[Severity: Low]
Is the set of users who can reach this wider than described here?

packet_create() checks the capability against the user namespace that
owns the netns:

    if (!ns_capable(net->user_ns, CAP_NET_RAW))
        return -EPERM;

Root inside an unprivileged user namespace passes this check.

The same user can create a tun device in its own netns, since
tun_set_iff() checks ns_capable(net->user_ns, CAP_NET_ADMIN). It can also
set the device speed with ETHTOOL_SLINKSETTINGS:

- dev_ethtool() does the same ns_capable() check.
- ethtool_set_link_ksettings() does not validate the speed.
- tun_set_link_ksettings() copies the settings unchanged:

    memcpy(&tun->link_ksettings, cmd, sizeof(*cmd));

prb_calc_retire_blk_tmo() then reads that speed back through
__ethtool_get_link_ksettings() for the bound ifindex:

    unshare(CLONE_NEWUSER | CLONE_NEWNET)
      -> TUNSETIFF
      -> ETHTOOL_SLINKSETTINGS with speed = 1000 (or 2500)
      -> socket(AF_PACKET), bind to the tun device
      -> PACKET_VERSION = TPACKET_V3, PACKET_RX_RING

On systems that allow unprivileged user namespaces, this seems to let an
unprivileged local user start the maximum-rate soft hrtimer with no real
hardware. Would it be worth saying so in the commit message?

The same tun setup could also be used to test the fix at runtime. The
notes say the patch was only compile-tested because no 1 Gbps setup was
available.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923050101.1510064-1-zhangdairui%40gmail.com