| Message ID | 20230323191744.222183-6-witu@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support hardware miss to tc action | expand |
On Thu, Mar 23, 2023 at 8:18 PM William Tu <witu@nvidia.com> wrote: > > From: Eric Dumazet <edumazet@google.com> > > BugLink: https://bugs.launchpad.net/bugs/2012571 > > The two "goto errout;" paths in fl_change() became wrong > after cited commit. > > Indeed we only must not call __fl_put() until the net pointer > has been set in tcf_exts_init_ex() > > This is a minimal fix. We might in the future validate TCA_FLOWER_FLAGS > before we allocate @fnew. > > BUG: KASAN: null-ptr-deref in instrument_atomic_read include/linux/instrumented.h:72 [inline] > BUG: KASAN: null-ptr-deref in atomic_read include/linux/atomic/atomic-instrumented.h:27 [inline] > BUG: KASAN: null-ptr-deref in refcount_read include/linux/refcount.h:147 [inline] > BUG: KASAN: null-ptr-deref in __refcount_add_not_zero include/linux/refcount.h:152 [inline] > BUG: KASAN: null-ptr-deref in __refcount_inc_not_zero include/linux/refcount.h:227 [inline] > BUG: KASAN: null-ptr-deref in refcount_inc_not_zero include/linux/refcount.h:245 [inline] > BUG: KASAN: null-ptr-deref in maybe_get_net include/net/net_namespace.h:269 [inline] > BUG: KASAN: null-ptr-deref in tcf_exts_get_net include/net/pkt_cls.h:260 [inline] > BUG: KASAN: null-ptr-deref in __fl_put net/sched/cls_flower.c:513 [inline] > BUG: KASAN: null-ptr-deref in __fl_put+0x13e/0x3b0 net/sched/cls_flower.c:508 > Read of size 4 at addr 000000000000014c by task syz-executor548/5082 > > CPU: 0 PID: 5082 Comm: syz-executor548 Not tainted 6.2.0-syzkaller-05251-g5b7c4cabbb65 #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/21/2023 > Call Trace: > <TASK> > __dump_stack lib/dump_stack.c:88 [inline] > dump_stack_lvl+0xd9/0x150 lib/dump_stack.c:106 > print_report mm/kasan/report.c:420 [inline] > kasan_report+0xec/0x130 mm/kasan/report.c:517 > check_region_inline mm/kasan/generic.c:183 [inline] > kasan_check_range+0x141/0x190 mm/kasan/generic.c:189 > instrument_atomic_read include/linux/instrumented.h:72 [inline] > atomic_read include/linux/atomic/atomic-instrumented.h:27 [inline] > refcount_read include/linux/refcount.h:147 [inline] > __refcount_add_not_zero include/linux/refcount.h:152 [inline] > __refcount_inc_not_zero include/linux/refcount.h:227 [inline] > refcount_inc_not_zero include/linux/refcount.h:245 [inline] > maybe_get_net include/net/net_namespace.h:269 [inline] > tcf_exts_get_net include/net/pkt_cls.h:260 [inline] > __fl_put net/sched/cls_flower.c:513 [inline] > __fl_put+0x13e/0x3b0 net/sched/cls_flower.c:508 > fl_change+0x101b/0x4ab0 net/sched/cls_flower.c:2341 > tc_new_tfilter+0x97c/0x2290 net/sched/cls_api.c:2310 > rtnetlink_rcv_msg+0x996/0xd50 net/core/rtnetlink.c:6165 > 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+0x925/0xe30 net/netlink/af_netlink.c:1942 > sock_sendmsg_nosec net/socket.c:722 [inline] > sock_sendmsg+0xde/0x190 net/socket.c:745 > ____sys_sendmsg+0x334/0x900 net/socket.c:2504 > ___sys_sendmsg+0x110/0x1b0 net/socket.c:2558 > __sys_sendmmsg+0x18f/0x460 net/socket.c:2644 > __do_sys_sendmmsg net/socket.c:2673 [inline] > __se_sys_sendmmsg net/socket.c:2670 [inline] > __x64_sys_sendmmsg+0x9d/0x100 net/socket.c:2670 > > Fixes: 08a0063df3ae ("net/sched: flower: Move filter handle initialization earlier") > Reported-by: syzbot+baabf3efa7c1e57d28b2@syzkaller.appspotmail.com > Reported-by: syzbot <syzkaller@googlegroups.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Paul Blakey <paulb@nvidia.com> > Reviewed-by: Simon Horman <simon.horman@corigine.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > Signed-off-by: Paul Blakey <paulb@nvidia.com> > (Cherry picked from upstream commit dfd2f0eb2347dbdf391fd5b8255fefc58a745472) To correctly document the patch history the "cherry-pick" line should be before Paul's "S-o-b:" (assuming that the cherry pick was actually done by Paul) and your own "S-o-b:" should be added at the end. Best Regards, Bartlomiej > --- > net/sched/cls_flower.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c > index 99dfc01dee9c..9605f22c1877 100644 > --- a/net/sched/cls_flower.c > +++ b/net/sched/cls_flower.c > @@ -1652,8 +1652,9 @@ static int fl_change(struct net *net, struct sk_buff *in_skb, > fnew->flags = nla_get_u32(tb[TCA_FLOWER_FLAGS]); > > if (!tc_flags_valid(fnew->flags)) { > + kfree(fnew); > err = -EINVAL; > - goto errout; > + goto errout_tb; > } > } > > @@ -1678,8 +1679,10 @@ static int fl_change(struct net *net, struct sk_buff *in_skb, > } > spin_unlock(&tp->lock); > > - if (err) > - goto errout; > + if (err) { > + kfree(fnew); > + goto errout_tb; > + } > } > fnew->handle = handle; > > @@ -1788,7 +1791,6 @@ static int fl_change(struct net *net, struct sk_buff *in_skb, > fl_mask_put(head, fnew->mask); > errout_idr: > idr_remove(&head->handle_idr, fnew->handle); > -errout: > __fl_put(fnew); > errout_tb: > kfree(tb);
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index 99dfc01dee9c..9605f22c1877 100644 --- a/net/sched/cls_flower.c +++ b/net/sched/cls_flower.c @@ -1652,8 +1652,9 @@ static int fl_change(struct net *net, struct sk_buff *in_skb, fnew->flags = nla_get_u32(tb[TCA_FLOWER_FLAGS]); if (!tc_flags_valid(fnew->flags)) { + kfree(fnew); err = -EINVAL; - goto errout; + goto errout_tb; } } @@ -1678,8 +1679,10 @@ static int fl_change(struct net *net, struct sk_buff *in_skb, } spin_unlock(&tp->lock); - if (err) - goto errout; + if (err) { + kfree(fnew); + goto errout_tb; + } } fnew->handle = handle; @@ -1788,7 +1791,6 @@ static int fl_change(struct net *net, struct sk_buff *in_skb, fl_mask_put(head, fnew->mask); errout_idr: idr_remove(&head->handle_idr, fnew->handle); -errout: __fl_put(fnew); errout_tb: kfree(tb);