No need to declare a separate stack variable if you access it only once.
Better with:
if (node == tipc_own_addr(net)) {...}
Otherwise,
Acked-by: jon
> -----Original Message-----
> From: Hoang Le <[email protected]>
> Sent: 21-Mar-19 09:29
> To: Jon Maloy <[email protected]>; [email protected];
> [email protected]; [email protected]
> Subject: [PATCH 2/2] tipc: fix a null pointer deref
>
> In commit c55c8edafa91 ("tipc: smooth change between replicast and
> broadcast") we introduced new method to eliminate the risk of message
> reordering that happen in between different nodes.
> Unfortunately, we forgot checking at receiving side to ignore intra node.
>
> We fix this by checking and returning if arrived message from intra node.
>
> syzbot report:
> ==========================================================
> ========
> kasan: CONFIG_KASAN_INLINE enabled
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault: 0000 [#1] PREEMPT SMP KASAN
> CPU: 0 PID: 7820 Comm: syz-executor418 Not tainted 5.0.0+ #61 Hardware
> name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:tipc_mcast_filter_msg+0x21b/0x13d0 net/tipc/bcast.c:782
> Code: 45 c0 0f 84 39 06 00 00 48 89 5d 98 e8 ce ab a5 fa 49 8d bc
> 24 c8 00 00 00 48 b9 00 00 00 00 00 fc ff df 48 89 f8 48 c1 e8 03 <80> 3c
> 08 00 0f
> 85 9a 0e 00 00 49 8b 9c 24 c8 00 00 00 48 be 00 00
> RSP: 0018:ffff8880959defc8 EFLAGS: 00010202
> RAX: 0000000000000019 RBX: ffff888081258a48 RCX: dffffc0000000000
> RDX: 0000000000000000 RSI: ffffffff86cab862 RDI: 00000000000000c8
> RBP: ffff8880959df030 R08: ffff8880813d0200 R09: ffffed1015d05bc8
> R10: ffffed1015d05bc7 R11: ffff8880ae82de3b R12: 0000000000000000
> R13: 000000000000002c R14: 0000000000000000 R15: ffff888081258a48
> FS: 000000000106a880(0000) GS:ffff8880ae800000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020001cc0 CR3: 0000000094a20000 CR4: 00000000001406f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call
> Trace:
> tipc_sk_filter_rcv+0x182d/0x34f0 net/tipc/socket.c:2168 tipc_sk_enqueue
> net/tipc/socket.c:2254 [inline]
> tipc_sk_rcv+0xc45/0x25a0 net/tipc/socket.c:2305
> tipc_sk_mcast_rcv+0x724/0x1020 net/tipc/socket.c:1209
> tipc_mcast_xmit+0x7fe/0x1200 net/tipc/bcast.c:410
> tipc_sendmcast+0xb36/0xfc0 net/tipc/socket.c:820
> __tipc_sendmsg+0x10df/0x18d0 net/tipc/socket.c:1358
> tipc_sendmsg+0x53/0x80 net/tipc/socket.c:1291 sock_sendmsg_nosec
> net/socket.c:651 [inline]
> sock_sendmsg+0xdd/0x130 net/socket.c:661
> ___sys_sendmsg+0x806/0x930 net/socket.c:2260
> __sys_sendmsg+0x105/0x1d0 net/socket.c:2298 __do_sys_sendmsg
> net/socket.c:2307 [inline] __se_sys_sendmsg net/socket.c:2305 [inline]
> __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2305
> do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x4401c9
> Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8
> 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d
> 01 f0 ff
> ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007ffd887fa9d8 EFLAGS: 00000246 ORIG_RAX:
> 000000000000002e
> RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004401c9
> RDX: 0000000000000000 RSI: 0000000020002140 RDI: 0000000000000003
> RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000401a50
> R13: 0000000000401ae0 R14: 0000000000000000 R15: 0000000000000000
> Modules linked in:
> ---[ end trace ba79875754e1708f ]---
>
> Reported-by: [email protected]
> Fixes: c55c8eda ("tipc: smooth change between replicast and broadcast")
> Signed-off-by: Hoang Le <[email protected]>
> ---
> net/tipc/bcast.c | 6 +++++-
> net/tipc/bcast.h | 2 +-
> net/tipc/socket.c | 2 +-
> 3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c index
> 5264a8ff6e01..b3e6b4892425 100644
> --- a/net/tipc/bcast.c
> +++ b/net/tipc/bcast.c
> @@ -760,10 +760,11 @@ u32 tipc_bcast_get_broadcast_ratio(struct net
> *net)
> return bb->rc_ratio;
> }
>
> -void tipc_mcast_filter_msg(struct sk_buff_head *defq,
> +void tipc_mcast_filter_msg(struct net *net, struct sk_buff_head *defq,
> struct sk_buff_head *inputq)
> {
> struct sk_buff *skb, *_skb, *tmp;
> + u32 self = tipc_own_addr(net);
> struct tipc_msg *hdr, *_hdr;
> bool match = false;
> u32 node, port;
> @@ -775,6 +776,9 @@ void tipc_mcast_filter_msg(struct sk_buff_head
> *defq,
> return;
>
> node = msg_orignode(hdr);
> + if (node == self)
> + return;
> +
> port = msg_origport(hdr);
>
> /* Has the twin SYN message already arrived ? */ diff --git
> a/net/tipc/bcast.h b/net/tipc/bcast.h index 484bde289d3a..dadad953e2be
> 100644
> --- a/net/tipc/bcast.h
> +++ b/net/tipc/bcast.h
> @@ -101,7 +101,7 @@ int tipc_bclink_reset_stats(struct net *net);
> u32 tipc_bcast_get_broadcast_mode(struct net *net);
> u32 tipc_bcast_get_broadcast_ratio(struct net *net);
>
> -void tipc_mcast_filter_msg(struct sk_buff_head *defq,
> +void tipc_mcast_filter_msg(struct net *net, struct sk_buff_head *defq,
> struct sk_buff_head *inputq);
>
> static inline void tipc_bcast_lock(struct net *net) diff --git
> a/net/tipc/socket.c b/net/tipc/socket.c index a7b3e1a070e4..8ac8ddf1e324
> 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -2166,7 +2166,7 @@ static void tipc_sk_filter_rcv(struct sock *sk, struct
> sk_buff *skb,
> tipc_group_filter_msg(grp, &inputq, xmitq);
>
> if (unlikely(!grp) && mtyp == TIPC_MCAST_MSG)
> - tipc_mcast_filter_msg(&tsk->mc_method.deferredq,
> &inputq);
> + tipc_mcast_filter_msg(net, &tsk->mc_method.deferredq,
> &inputq);
>
> /* Validate and add to receive buffer if there is space */
> while ((skb = __skb_dequeue(&inputq))) {
> --
> 2.1.4
_______________________________________________
tipc-discussion mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
