netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] net/sched: flower: fix fl_change() error recovery path
@ 2023-02-27 18:44 Eric Dumazet
  2023-02-28 10:55 ` Simon Horman
  2023-03-01 17:30 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Eric Dumazet @ 2023-02-27 18:44 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, eric.dumazet, Eric Dumazet, syzbot+baabf3efa7c1e57d28b2,
	syzbot, Paul Blakey

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>
---
 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 e960a46b05205bb0bca7dc0d21531e4d6a3853e3..475fe222a85566639bac75fc4a95bf649a10357d 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -2200,8 +2200,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;
 		}
 	}
 
@@ -2226,8 +2227,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;
 
@@ -2337,7 +2340,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);
-- 
2.39.2.722.g9855ee24e9-goog


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

* Re: [PATCH net] net/sched: flower: fix fl_change() error recovery path
  2023-02-27 18:44 [PATCH net] net/sched: flower: fix fl_change() error recovery path Eric Dumazet
@ 2023-02-28 10:55 ` Simon Horman
  2023-03-01 17:30 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Simon Horman @ 2023-02-28 10:55 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
	eric.dumazet, syzbot+baabf3efa7c1e57d28b2, syzbot, Paul Blakey

On Mon, Feb 27, 2023 at 06:44:36PM +0000, Eric Dumazet wrote:
> 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>

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

* Re: [PATCH net] net/sched: flower: fix fl_change() error recovery path
  2023-02-27 18:44 [PATCH net] net/sched: flower: fix fl_change() error recovery path Eric Dumazet
  2023-02-28 10:55 ` Simon Horman
@ 2023-03-01 17:30 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-03-01 17:30 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: davem, kuba, pabeni, netdev, eric.dumazet,
	syzbot+baabf3efa7c1e57d28b2, syzkaller, paulb

Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:

On Mon, 27 Feb 2023 18:44:36 +0000 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [net] net/sched: flower: fix fl_change() error recovery path
    https://git.kernel.org/netdev/net/c/dfd2f0eb2347

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] 3+ messages in thread

end of thread, other threads:[~2023-03-01 17:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-27 18:44 [PATCH net] net/sched: flower: fix fl_change() error recovery path Eric Dumazet
2023-02-28 10:55 ` Simon Horman
2023-03-01 17:30 ` 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).