netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] net_sched: cls_flow: validate TCA_FLOW_RSHIFT attribute
@ 2025-01-03 10:45 Eric Dumazet
  2025-01-03 18:24 ` [PATCH net] selftests: tc-testing: reduce rshift value Jakub Kicinski
  2025-01-04 17:00 ` [PATCH net] net_sched: cls_flow: validate TCA_FLOW_RSHIFT attribute patchwork-bot+netdevbpf
  0 siblings, 2 replies; 5+ messages in thread
From: Eric Dumazet @ 2025-01-03 10:45 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Jamal Hadi Salim, Victor Nogueira, Cong Wang, Jiri Pirko, netdev,
	eric.dumazet, Eric Dumazet, syzbot+1dbb57d994e54aaa04d2

syzbot found that TCA_FLOW_RSHIFT attribute was not validated.
Right shitfing a 32bit integer is undefined for large shift values.

UBSAN: shift-out-of-bounds in net/sched/cls_flow.c:329:23
shift exponent 9445 is too large for 32-bit type 'u32' (aka 'unsigned int')
CPU: 1 UID: 0 PID: 54 Comm: kworker/u8:3 Not tainted 6.13.0-rc3-syzkaller-00180-g4f619d518db9 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
Workqueue: ipv6_addrconf addrconf_dad_work
Call Trace:
 <TASK>
  __dump_stack lib/dump_stack.c:94 [inline]
  dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
  ubsan_epilogue lib/ubsan.c:231 [inline]
  __ubsan_handle_shift_out_of_bounds+0x3c8/0x420 lib/ubsan.c:468
  flow_classify+0x24d5/0x25b0 net/sched/cls_flow.c:329
  tc_classify include/net/tc_wrapper.h:197 [inline]
  __tcf_classify net/sched/cls_api.c:1771 [inline]
  tcf_classify+0x420/0x1160 net/sched/cls_api.c:1867
  sfb_classify net/sched/sch_sfb.c:260 [inline]
  sfb_enqueue+0x3ad/0x18b0 net/sched/sch_sfb.c:318
  dev_qdisc_enqueue+0x4b/0x290 net/core/dev.c:3793
  __dev_xmit_skb net/core/dev.c:3889 [inline]
  __dev_queue_xmit+0xf0e/0x3f50 net/core/dev.c:4400
  dev_queue_xmit include/linux/netdevice.h:3168 [inline]
  neigh_hh_output include/net/neighbour.h:523 [inline]
  neigh_output include/net/neighbour.h:537 [inline]
  ip_finish_output2+0xd41/0x1390 net/ipv4/ip_output.c:236
  iptunnel_xmit+0x55d/0x9b0 net/ipv4/ip_tunnel_core.c:82
  udp_tunnel_xmit_skb+0x262/0x3b0 net/ipv4/udp_tunnel_core.c:173
  geneve_xmit_skb drivers/net/geneve.c:916 [inline]
  geneve_xmit+0x21dc/0x2d00 drivers/net/geneve.c:1039
  __netdev_start_xmit include/linux/netdevice.h:5002 [inline]
  netdev_start_xmit include/linux/netdevice.h:5011 [inline]
  xmit_one net/core/dev.c:3590 [inline]
  dev_hard_start_xmit+0x27a/0x7d0 net/core/dev.c:3606
  __dev_queue_xmit+0x1b73/0x3f50 net/core/dev.c:4434

Fixes: e5dfb815181f ("[NET_SCHED]: Add flow classifier")
Reported-by: syzbot+1dbb57d994e54aaa04d2@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/6777bf49.050a0220.178762.0040.GAE@google.com/T/#u
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/cls_flow.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 5502998aace74157320efed71faf40afd196fac4..5c2580a07530e4e0615a7df7ff9d0a35ae9e94b4 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -356,7 +356,8 @@ static const struct nla_policy flow_policy[TCA_FLOW_MAX + 1] = {
 	[TCA_FLOW_KEYS]		= { .type = NLA_U32 },
 	[TCA_FLOW_MODE]		= { .type = NLA_U32 },
 	[TCA_FLOW_BASECLASS]	= { .type = NLA_U32 },
-	[TCA_FLOW_RSHIFT]	= { .type = NLA_U32 },
+	[TCA_FLOW_RSHIFT]	= NLA_POLICY_MAX(NLA_U32,
+						 31 /* BITS_PER_U32 - 1 */),
 	[TCA_FLOW_ADDEND]	= { .type = NLA_U32 },
 	[TCA_FLOW_MASK]		= { .type = NLA_U32 },
 	[TCA_FLOW_XOR]		= { .type = NLA_U32 },
-- 
2.47.1.613.gc27f4b7a9f-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net] selftests: tc-testing: reduce rshift value
  2025-01-03 10:45 [PATCH net] net_sched: cls_flow: validate TCA_FLOW_RSHIFT attribute Eric Dumazet
@ 2025-01-03 18:24 ` Jakub Kicinski
  2025-01-03 18:35   ` Eric Dumazet
  2025-01-04 17:00   ` patchwork-bot+netdevbpf
  2025-01-04 17:00 ` [PATCH net] net_sched: cls_flow: validate TCA_FLOW_RSHIFT attribute patchwork-bot+netdevbpf
  1 sibling, 2 replies; 5+ messages in thread
From: Jakub Kicinski @ 2025-01-03 18:24 UTC (permalink / raw)
  To: edumazet
  Cc: netdev, davem, pabeni, Jakub Kicinski, jhs, xiyou.wangcong, jiri,
	shuah, karansanghvi98, linux-kselftest

After previous change rshift >= 32 is no longer allowed.
Modify the test to use 31, the test doesn't seem to send
any traffic so the exact value shouldn't matter.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: jhs@mojatatu.com
CC: xiyou.wangcong@gmail.com
CC: jiri@resnulli.us
CC: shuah@kernel.org
CC: karansanghvi98@gmail.com
CC: linux-kselftest@vger.kernel.org
---
 tools/testing/selftests/tc-testing/tc-tests/filters/flow.json | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/filters/flow.json b/tools/testing/selftests/tc-testing/tc-tests/filters/flow.json
index 996448afe31b..91d120548bf5 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/filters/flow.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/filters/flow.json
@@ -78,10 +78,10 @@
         "setup": [
             "$TC qdisc add dev $DEV1 ingress"
         ],
-        "cmdUnderTest": "$TC filter add dev $DEV1 parent ffff: handle 1 prio 1 protocol ip flow map key dst rshift 0xff",
+        "cmdUnderTest": "$TC filter add dev $DEV1 parent ffff: handle 1 prio 1 protocol ip flow map key dst rshift 0x1f",
         "expExitCode": "0",
         "verifyCmd": "$TC filter get dev $DEV1 parent ffff: handle 1 protocol ip prio 1 flow",
-        "matchPattern": "filter parent ffff: protocol ip pref 1 flow chain [0-9]+ handle 0x1 map keys dst rshift 255 baseclass",
+        "matchPattern": "filter parent ffff: protocol ip pref 1 flow chain [0-9]+ handle 0x1 map keys dst rshift 31 baseclass",
         "matchCount": "1",
         "teardown": [
             "$TC qdisc del dev $DEV1 ingress"
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net] selftests: tc-testing: reduce rshift value
  2025-01-03 18:24 ` [PATCH net] selftests: tc-testing: reduce rshift value Jakub Kicinski
@ 2025-01-03 18:35   ` Eric Dumazet
  2025-01-04 17:00   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2025-01-03 18:35 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, davem, pabeni, jhs, xiyou.wangcong, jiri, shuah,
	karansanghvi98, linux-kselftest

On Fri, Jan 3, 2025 at 7:25 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> After previous change rshift >= 32 is no longer allowed.
> Modify the test to use 31, the test doesn't seem to send
> any traffic so the exact value shouldn't matter.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Thanks !

Reviewed-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] selftests: tc-testing: reduce rshift value
  2025-01-03 18:24 ` [PATCH net] selftests: tc-testing: reduce rshift value Jakub Kicinski
  2025-01-03 18:35   ` Eric Dumazet
@ 2025-01-04 17:00   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-01-04 17:00 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: edumazet, netdev, davem, pabeni, jhs, xiyou.wangcong, jiri, shuah,
	karansanghvi98, linux-kselftest

Hello:

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

On Fri,  3 Jan 2025 10:24:58 -0800 you wrote:
> After previous change rshift >= 32 is no longer allowed.
> Modify the test to use 31, the test doesn't seem to send
> any traffic so the exact value shouldn't matter.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: jhs@mojatatu.com
> CC: xiyou.wangcong@gmail.com
> CC: jiri@resnulli.us
> CC: shuah@kernel.org
> CC: karansanghvi98@gmail.com
> CC: linux-kselftest@vger.kernel.org
> 
> [...]

Here is the summary with links:
  - [net] selftests: tc-testing: reduce rshift value
    https://git.kernel.org/netdev/net/c/e95274dfe864

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] net_sched: cls_flow: validate TCA_FLOW_RSHIFT attribute
  2025-01-03 10:45 [PATCH net] net_sched: cls_flow: validate TCA_FLOW_RSHIFT attribute Eric Dumazet
  2025-01-03 18:24 ` [PATCH net] selftests: tc-testing: reduce rshift value Jakub Kicinski
@ 2025-01-04 17:00 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-01-04 17:00 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: davem, kuba, pabeni, jhs, victor, xiyou.wangcong, jiri, netdev,
	eric.dumazet, syzbot+1dbb57d994e54aaa04d2

Hello:

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

On Fri,  3 Jan 2025 10:45:46 +0000 you wrote:
> syzbot found that TCA_FLOW_RSHIFT attribute was not validated.
> Right shitfing a 32bit integer is undefined for large shift values.
> 
> UBSAN: shift-out-of-bounds in net/sched/cls_flow.c:329:23
> shift exponent 9445 is too large for 32-bit type 'u32' (aka 'unsigned int')
> CPU: 1 UID: 0 PID: 54 Comm: kworker/u8:3 Not tainted 6.13.0-rc3-syzkaller-00180-g4f619d518db9 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
> Workqueue: ipv6_addrconf addrconf_dad_work
> Call Trace:
>  <TASK>
>   __dump_stack lib/dump_stack.c:94 [inline]
>   dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
>   ubsan_epilogue lib/ubsan.c:231 [inline]
>   __ubsan_handle_shift_out_of_bounds+0x3c8/0x420 lib/ubsan.c:468
>   flow_classify+0x24d5/0x25b0 net/sched/cls_flow.c:329
>   tc_classify include/net/tc_wrapper.h:197 [inline]
>   __tcf_classify net/sched/cls_api.c:1771 [inline]
>   tcf_classify+0x420/0x1160 net/sched/cls_api.c:1867
>   sfb_classify net/sched/sch_sfb.c:260 [inline]
>   sfb_enqueue+0x3ad/0x18b0 net/sched/sch_sfb.c:318
>   dev_qdisc_enqueue+0x4b/0x290 net/core/dev.c:3793
>   __dev_xmit_skb net/core/dev.c:3889 [inline]
>   __dev_queue_xmit+0xf0e/0x3f50 net/core/dev.c:4400
>   dev_queue_xmit include/linux/netdevice.h:3168 [inline]
>   neigh_hh_output include/net/neighbour.h:523 [inline]
>   neigh_output include/net/neighbour.h:537 [inline]
>   ip_finish_output2+0xd41/0x1390 net/ipv4/ip_output.c:236
>   iptunnel_xmit+0x55d/0x9b0 net/ipv4/ip_tunnel_core.c:82
>   udp_tunnel_xmit_skb+0x262/0x3b0 net/ipv4/udp_tunnel_core.c:173
>   geneve_xmit_skb drivers/net/geneve.c:916 [inline]
>   geneve_xmit+0x21dc/0x2d00 drivers/net/geneve.c:1039
>   __netdev_start_xmit include/linux/netdevice.h:5002 [inline]
>   netdev_start_xmit include/linux/netdevice.h:5011 [inline]
>   xmit_one net/core/dev.c:3590 [inline]
>   dev_hard_start_xmit+0x27a/0x7d0 net/core/dev.c:3606
>   __dev_queue_xmit+0x1b73/0x3f50 net/core/dev.c:4434
> 
> [...]

Here is the summary with links:
  - [net] net_sched: cls_flow: validate TCA_FLOW_RSHIFT attribute
    https://git.kernel.org/netdev/net/c/a039e54397c6

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-01-04 17:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-03 10:45 [PATCH net] net_sched: cls_flow: validate TCA_FLOW_RSHIFT attribute Eric Dumazet
2025-01-03 18:24 ` [PATCH net] selftests: tc-testing: reduce rshift value Jakub Kicinski
2025-01-03 18:35   ` Eric Dumazet
2025-01-04 17:00   ` patchwork-bot+netdevbpf
2025-01-04 17:00 ` [PATCH net] net_sched: cls_flow: validate TCA_FLOW_RSHIFT attribute patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).