[SRU][F:linux-bluefield][PATCH v1 5/5] net/sched: flower: fix fl_change() error recovery path

[Bartlomiej Zolnierkiewicz]

On Thu, Mar 23, 2023 at 8:18 PM William Tu <witu at nvidia.com> wrote:
>
> From: Eric Dumazet <edumazet at 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 at syzkaller.appspotmail.com
> Reported-by: syzbot <syzkaller at googlegroups.com>
> Signed-off-by: Eric Dumazet <edumazet at google.com>
> Cc: Paul Blakey <paulb at nvidia.com>
> Reviewed-by: Simon Horman <simon.horman at corigine.com>
> Signed-off-by: David S. Miller <davem at davemloft.net>
> Signed-off-by: Paul Blakey <paulb at 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);