| Message ID | 20230214014729.648564-1-pctammela@mojatatu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 42018a322bd453e38b3ffee294982243e50a484f |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net/sched: tcindex: search key must be 16 bits | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net |
| netdev/fixes_present | success | Fixes tag present in non-next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Single patches do not need cover letters |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 9 of 9 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | warning | WARNING: Possible repeated word: 'Google' |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Mon, 13 Feb 2023 22:47:29 -0300 Pedro Tammela <pctammela@mojatatu.com> wrote: > diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c > index ba7f22a49..6640e75ea 100644 > --- a/net/sched/cls_tcindex.c > +++ b/net/sched/cls_tcindex.c > @@ -503,7 +503,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, > /* lookup the filter, guaranteed to exist */ > for (cf = rcu_dereference_bh_rtnl(*fp); cf; > fp = &cf->next, cf = rcu_dereference_bh_rtnl(*fp)) > - if (cf->key == handle) > + if (cf->key == (u16)handle) > break; Rather than truncating silently. I think the code should first test that handle is not outside of range and return EINVAL instead.
On Tue, Feb 14, 2023 at 5:46 PM Stephen Hemminger <stephen@networkplumber.org> wrote: > > On Mon, 13 Feb 2023 22:47:29 -0300 > Pedro Tammela <pctammela@mojatatu.com> wrote: > > > diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c > > index ba7f22a49..6640e75ea 100644 > > --- a/net/sched/cls_tcindex.c > > +++ b/net/sched/cls_tcindex.c > > @@ -503,7 +503,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, > > /* lookup the filter, guaranteed to exist */ > > for (cf = rcu_dereference_bh_rtnl(*fp); cf; > > fp = &cf->next, cf = rcu_dereference_bh_rtnl(*fp)) > > - if (cf->key == handle) > > + if (cf->key == (u16)handle) > > break; > > Rather than truncating silently. I think the code should first test that handle is > not outside of range and return EINVAL instead. Stephen, It is a fix to a fix that is on -net right now. Note: tcindex will disappear shortly altogether from the tree - we are deleting it. Is it worth making the change? cheers, jamal
On Tue, Feb 14, 2023 at 2:47 AM Pedro Tammela <pctammela@mojatatu.com> wrote: > > Syzkaller found an issue where a handle greater than 16 bits would trigger > a null-ptr-deref in the imperfect hash area update. > > > Fixes: ee059170b1f7 ("net/sched: tcindex: update imperfect hash filters respecting rcu") > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Reported-by: syzbot <syzkaller@googlegroups.com> Reviewed-by: Eric Dumazet <edumazet@google.com> > --- > net/sched/cls_tcindex.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c > index ba7f22a49..6640e75ea 100644 > --- a/net/sched/cls_tcindex.c > +++ b/net/sched/cls_tcindex.c > @@ -503,7 +503,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, > /* lookup the filter, guaranteed to exist */ > for (cf = rcu_dereference_bh_rtnl(*fp); cf; > fp = &cf->next, cf = rcu_dereference_bh_rtnl(*fp)) > - if (cf->key == handle) > + if (cf->key == (u16)handle) > break; > > f->next = cf->next; > -- > 2.34.1 >
Hello: This patch was applied to netdev/net.git (master) by David S. Miller <davem@davemloft.net>: On Mon, 13 Feb 2023 22:47:29 -0300 you wrote: > Syzkaller found an issue where a handle greater than 16 bits would trigger > a null-ptr-deref in the imperfect hash area update. > > general protection fault, probably for non-canonical address > 0xdffffc0000000015: 0000 [#1] PREEMPT SMP KASAN > KASAN: null-ptr-deref in range [0x00000000000000a8-0x00000000000000af] > CPU: 0 PID: 5070 Comm: syz-executor456 Not tainted > 6.2.0-rc7-syzkaller-00112-gc68f345b7c42 #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, > BIOS Google 01/21/2023 > RIP: 0010:tcindex_set_parms+0x1a6a/0x2990 net/sched/cls_tcindex.c:509 > Code: 01 e9 e9 fe ff ff 4c 8b bd 28 fe ff ff e8 0e 57 7d f9 48 8d bb > a8 00 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c > 02 00 0f 85 94 0c 00 00 48 8b 85 f8 fd ff ff 48 8b 9b a8 00 > RSP: 0018:ffffc90003d3ef88 EFLAGS: 00010202 > RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000 > RDX: 0000000000000015 RSI: ffffffff8803a102 RDI: 00000000000000a8 > RBP: ffffc90003d3f1d8 R08: 0000000000000001 R09: 0000000000000000 > R10: 0000000000000001 R11: 0000000000000000 R12: ffff88801e2b10a8 > R13: dffffc0000000000 R14: 0000000000030000 R15: ffff888017b3be00 > FS: 00005555569af300(0000) GS:ffff8880b9800000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 000056041c6d2000 CR3: 000000002bfca000 CR4: 00000000003506f0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > <TASK> > tcindex_change+0x1ea/0x320 net/sched/cls_tcindex.c:572 > tc_new_tfilter+0x96e/0x2220 net/sched/cls_api.c:2155 > rtnetlink_rcv_msg+0x959/0xca0 net/core/rtnetlink.c:6132 > netlink_rcv_skb+0x165/0x440 net/netlink/af_netlink.c:2574 > netlink_unicast_kernel net/netlink/af_netlink.c:1339 [inline] > netlink_unicast+0x547/0x7f0 net/netlink/af_netlink.c:1365 > netlink_sendmsg+0x91b/0xe10 net/netlink/af_netlink.c:1942 > sock_sendmsg_nosec net/socket.c:714 [inline] > sock_sendmsg+0xd3/0x120 net/socket.c:734 > ____sys_sendmsg+0x334/0x8c0 net/socket.c:2476 > ___sys_sendmsg+0x110/0x1b0 net/socket.c:2530 > __sys_sendmmsg+0x18f/0x460 net/socket.c:2616 > __do_sys_sendmmsg net/socket.c:2645 [inline] > __se_sys_sendmmsg net/socket.c:2642 [inline] > __x64_sys_sendmmsg+0x9d/0x100 net/socket.c:2642 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80 > > [...] Here is the summary with links: - [net] net/sched: tcindex: search key must be 16 bits https://git.kernel.org/netdev/net/c/42018a322bd4 You are awesome, thank you!
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c index ba7f22a49..6640e75ea 100644 --- a/net/sched/cls_tcindex.c +++ b/net/sched/cls_tcindex.c @@ -503,7 +503,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, /* lookup the filter, guaranteed to exist */ for (cf = rcu_dereference_bh_rtnl(*fp); cf; fp = &cf->next, cf = rcu_dereference_bh_rtnl(*fp)) - if (cf->key == handle) + if (cf->key == (u16)handle) break; f->next = cf->next;
Syzkaller found an issue where a handle greater than 16 bits would trigger a null-ptr-deref in the imperfect hash area update. general protection fault, probably for non-canonical address 0xdffffc0000000015: 0000 [#1] PREEMPT SMP KASAN KASAN: null-ptr-deref in range [0x00000000000000a8-0x00000000000000af] CPU: 0 PID: 5070 Comm: syz-executor456 Not tainted 6.2.0-rc7-syzkaller-00112-gc68f345b7c42 #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/21/2023 RIP: 0010:tcindex_set_parms+0x1a6a/0x2990 net/sched/cls_tcindex.c:509 Code: 01 e9 e9 fe ff ff 4c 8b bd 28 fe ff ff e8 0e 57 7d f9 48 8d bb a8 00 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 94 0c 00 00 48 8b 85 f8 fd ff ff 48 8b 9b a8 00 RSP: 0018:ffffc90003d3ef88 EFLAGS: 00010202 RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000 RDX: 0000000000000015 RSI: ffffffff8803a102 RDI: 00000000000000a8 RBP: ffffc90003d3f1d8 R08: 0000000000000001 R09: 0000000000000000 R10: 0000000000000001 R11: 0000000000000000 R12: ffff88801e2b10a8 R13: dffffc0000000000 R14: 0000000000030000 R15: ffff888017b3be00 FS: 00005555569af300(0000) GS:ffff8880b9800000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 000056041c6d2000 CR3: 000000002bfca000 CR4: 00000000003506f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> tcindex_change+0x1ea/0x320 net/sched/cls_tcindex.c:572 tc_new_tfilter+0x96e/0x2220 net/sched/cls_api.c:2155 rtnetlink_rcv_msg+0x959/0xca0 net/core/rtnetlink.c:6132 netlink_rcv_skb+0x165/0x440 net/netlink/af_netlink.c:2574 netlink_unicast_kernel net/netlink/af_netlink.c:1339 [inline] netlink_unicast+0x547/0x7f0 net/netlink/af_netlink.c:1365 netlink_sendmsg+0x91b/0xe10 net/netlink/af_netlink.c:1942 sock_sendmsg_nosec net/socket.c:714 [inline] sock_sendmsg+0xd3/0x120 net/socket.c:734 ____sys_sendmsg+0x334/0x8c0 net/socket.c:2476 ___sys_sendmsg+0x110/0x1b0 net/socket.c:2530 __sys_sendmmsg+0x18f/0x460 net/socket.c:2616 __do_sys_sendmmsg net/socket.c:2645 [inline] __se_sys_sendmmsg net/socket.c:2642 [inline] __x64_sys_sendmmsg+0x9d/0x100 net/socket.c:2642 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80 Fixes: ee059170b1f7 ("net/sched: tcindex: update imperfect hash filters respecting rcu") Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> --- net/sched/cls_tcindex.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)