netdev - Re: [PATCH net] net: sched: Fix truncation of offloaded action statistics

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] [day] [month] [year] [list]

Message-ID: <20250204200341.GN234677@kernel.org>
Date: Tue, 4 Feb 2025 20:03:41 +0000
From: Simon Horman <horms@...nel.org>
To: Ido Schimmel <idosch@...dia.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
	pabeni@...hat.com, edumazet@...gle.com, amirva@...lanox.com,
	petrm@...dia.com, joe@...mic.ac
Subject: Re: [PATCH net] net: sched: Fix truncation of offloaded action
 statistics

On Tue, Feb 04, 2025 at 02:38:39PM +0200, Ido Schimmel wrote:
> In case of tc offload, when user space queries the kernel for tc action
> statistics, tc will query the offloaded statistics from device drivers.
> Among other statistics, drivers are expected to pass the number of
> packets that hit the action since the last query as a 64-bit number.
> 
> Unfortunately, tc treats the number of packets as a 32-bit number,
> leading to truncation and incorrect statistics when the number of
> packets since the last query exceeds 0xffffffff:
> 
> $ tc -s filter show dev swp2 ingress
> filter protocol all pref 1 flower chain 0
> filter protocol all pref 1 flower chain 0 handle 0x1
>   skip_sw
>   in_hw in_hw_count 1
>         action order 1: mirred (Egress Redirect to device swp1) stolen
>         index 1 ref 1 bind 1 installed 58 sec used 0 sec
>         Action statistics:
>         Sent 1133877034176 bytes 536959475 pkt (dropped 0, overlimits 0 requeues 0)
> [...]
> 
> According to the above, 2111-byte packets were redirected which is
> impossible as only 64-byte packets were transmitted and the MTU was
> 1500.
> 
> Fix by treating packets as a 64-bit number:
> 
> $ tc -s filter show dev swp2 ingress
> filter protocol all pref 1 flower chain 0
> filter protocol all pref 1 flower chain 0 handle 0x1
>   skip_sw
>   in_hw in_hw_count 1
>         action order 1: mirred (Egress Redirect to device swp1) stolen
>         index 1 ref 1 bind 1 installed 61 sec used 0 sec
>         Action statistics:
>         Sent 1370624380864 bytes 21416005951 pkt (dropped 0, overlimits 0 requeues 0)
> [...]
> 
> Which shows that only 64-byte packets were redirected (1370624380864 /
> 21416005951 = 64).
> 
> Fixes: 380407023526 ("net/sched: Enable netdev drivers to update statistics of offloaded actions")
> Reported-by: Joe Botha <joe@...mic.ac>
> Signed-off-by: Ido Schimmel <idosch@...dia.com>
> Reviewed-by: Petr Machata <petrm@...dia.com>

Thanks Ido, all,

I agree that this function operates on packets as if it was 64-bit.  And in
a quick audit it seems that all callers, except qfq_enqueue() pass a 64-bit
rather than 32-bit integer (I did not check if the values passed can indeed
exceed 0xffffffff).

I also agree that the problem was introduced by the cited commit.

Reviewed-by: Simon Horman <horms@...nel.org>