| Message ID | 25A14D9CFAB7B34FB9440F90AFD352330101910702@ALA-MBA.corp.ad.wrs.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Hi, I didn't see this patch go in yet. Jon Maloy, ping? Should this go through somebody else? Vegard On 07/23/2016 11:49 AM, Xue, Ying wrote: > Acked-by: Ying Xue <ying.xue@windriver.com> > > -----Original Message----- > From: Vegard Nossum [mailto:vegard.nossum@oracle.com] > Sent: Saturday, July 23, 2016 2:15 PM > To: Jon Maloy; Xue, Ying > Cc: netdev@vger.kernel.org; David S. Miller; Michael Kerrisk; linux-kernel@vger.kernel.org; Vegard Nossum; stable@vger.kernel.org > Subject: [PATCH] tipc: fix NULL pointer dereference in shutdown() > > tipc_msg_create() can return a NULL skb and if so, we shouldn't try to call tipc_node_xmit_skb() on it. > > general protection fault: 0000 [#1] PREEMPT SMP KASAN > CPU: 3 PID: 30298 Comm: trinity-c0 Not tainted 4.7.0-rc7+ #19 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014 > task: ffff8800baf09980 ti: ffff8800595b8000 task.ti: ffff8800595b8000 > RIP: 0010:[<ffffffff830bb46b>] [<ffffffff830bb46b>] tipc_node_xmit_skb+0x6b/0x140 > RSP: 0018:ffff8800595bfce8 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: 0000000000000000 RCX: 000000003023b0e0 > RDX: 0000000000000000 RSI: dffffc0000000000 RDI: ffffffff83d12580 > RBP: ffff8800595bfd78 R08: ffffed000b2b7f32 R09: 0000000000000000 > R10: fffffbfff0759725 R11: 0000000000000000 R12: 1ffff1000b2b7f9f > R13: ffff8800595bfd58 R14: ffffffff83d12580 R15: dffffc0000000000 > FS: 00007fcdde242700(0000) GS:ffff88011af80000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007fcddde1db10 CR3: 000000006874b000 CR4: 00000000000006e0 > DR0: 00007fcdde248000 DR1: 00007fcddd73d000 DR2: 00007fcdde248000 > DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000090602 > Stack: > 0000000000000018 0000000000000018 0000000041b58ab3 ffffffff83954208 > ffffffff830bb400 ffff8800595bfd30 ffffffff8309d767 0000000000000018 > 0000000000000018 ffff8800595bfd78 ffffffff8309da1a 00000000810ee611 > Call Trace: > [<ffffffff830c84a3>] tipc_shutdown+0x553/0x880 > [<ffffffff825b4a3b>] SyS_shutdown+0x14b/0x170 > [<ffffffff8100334c>] do_syscall_64+0x19c/0x410 > [<ffffffff83295ca5>] entry_SYSCALL64_slow_path+0x25/0x25 > Code: 90 00 b4 0b 83 c7 00 f1 f1 f1 f1 4c 8d 6d e0 c7 40 04 00 00 00 f4 c7 40 08 f3 f3 f3 f3 48 89 d8 48 c1 e8 03 c7 45 b4 00 00 00 00 <80> 3c 30 00 75 78 48 8d 7b 08 49 8d 75 c0 48 b8 00 00 00 00 00 > RIP [<ffffffff830bb46b>] tipc_node_xmit_skb+0x6b/0x140 > RSP <ffff8800595bfce8> > ---[ end trace 57b0484e351e71f1 ]--- > > I feel like we should maybe return -ENOMEM or -ENOBUFS, but I'm not sure userspace is equipped to handle that. Anyway, this is better than a GPF and looks somewhat consistent with other tipc_msg_create() callers. > > Cc: stable@vger.kernel.org > Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> > --- > net/tipc/socket.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/tipc/socket.c b/net/tipc/socket.c index c49b8df..f9f5f3c 100644 > --- a/net/tipc/socket.c > +++ b/net/tipc/socket.c > @@ -2180,7 +2180,8 @@ restart: > TIPC_CONN_MSG, SHORT_H_SIZE, > 0, dnode, onode, dport, oport, > TIPC_CONN_SHUTDOWN); > - tipc_node_xmit_skb(net, skb, dnode, tsk->portid); > + if (skb) > + tipc_node_xmit_skb(net, skb, dnode, tsk->portid); > } > tsk->connected = 0; > sock->state = SS_DISCONNECTING; > -- > 1.9.1 >
Sorry, I just came back from vacation today. Acked-by: Jon Maloy <jon.maloy@ericsson.com> > -----Original Message----- > From: Vegard Nossum [mailto:vegard.nossum@oracle.com] > Sent: Friday, 12 August, 2016 10:38 > To: Ying Xue <ying.xue@windriver.com>; Jon Maloy <jon.maloy@ericsson.com>; > David S. Miller <davem@davemloft.net> > Cc: netdev@vger.kernel.org; Michael Kerrisk <mtk.manpages@gmail.com>; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH] tipc: fix NULL pointer dereference in shutdown() > > Hi, > > I didn't see this patch go in yet. Jon Maloy, ping? > > Should this go through somebody else? > > > Vegard > > On 07/23/2016 11:49 AM, Xue, Ying wrote: > > Acked-by: Ying Xue <ying.xue@windriver.com> > > > > -----Original Message----- > > From: Vegard Nossum [mailto:vegard.nossum@oracle.com] > > Sent: Saturday, July 23, 2016 2:15 PM > > To: Jon Maloy; Xue, Ying > > Cc: netdev@vger.kernel.org; David S. Miller; Michael Kerrisk; linux- > kernel@vger.kernel.org; Vegard Nossum; stable@vger.kernel.org > > Subject: [PATCH] tipc: fix NULL pointer dereference in shutdown() > > > > tipc_msg_create() can return a NULL skb and if so, we shouldn't try to call > tipc_node_xmit_skb() on it. > > > > general protection fault: 0000 [#1] PREEMPT SMP KASAN > > CPU: 3 PID: 30298 Comm: trinity-c0 Not tainted 4.7.0-rc7+ #19 > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu- > 1.8.2-1ubuntu1 04/01/2014 > > task: ffff8800baf09980 ti: ffff8800595b8000 task.ti: ffff8800595b8000 > > RIP: 0010:[<ffffffff830bb46b>] [<ffffffff830bb46b>] > tipc_node_xmit_skb+0x6b/0x140 > > RSP: 0018:ffff8800595bfce8 EFLAGS: 00010246 > > RAX: 0000000000000000 RBX: 0000000000000000 RCX: 000000003023b0e0 > > RDX: 0000000000000000 RSI: dffffc0000000000 RDI: ffffffff83d12580 > > RBP: ffff8800595bfd78 R08: ffffed000b2b7f32 R09: 0000000000000000 > > R10: fffffbfff0759725 R11: 0000000000000000 R12: 1ffff1000b2b7f9f > > R13: ffff8800595bfd58 R14: ffffffff83d12580 R15: dffffc0000000000 > > FS: 00007fcdde242700(0000) GS:ffff88011af80000(0000) > knlGS:0000000000000000 > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > CR2: 00007fcddde1db10 CR3: 000000006874b000 CR4: 00000000000006e0 > > DR0: 00007fcdde248000 DR1: 00007fcddd73d000 DR2: 00007fcdde248000 > > DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000090602 > > Stack: > > 0000000000000018 0000000000000018 0000000041b58ab3 ffffffff83954208 > > ffffffff830bb400 ffff8800595bfd30 ffffffff8309d767 0000000000000018 > > 0000000000000018 ffff8800595bfd78 ffffffff8309da1a 00000000810ee611 > > Call Trace: > > [<ffffffff830c84a3>] tipc_shutdown+0x553/0x880 > > [<ffffffff825b4a3b>] SyS_shutdown+0x14b/0x170 > > [<ffffffff8100334c>] do_syscall_64+0x19c/0x410 > > [<ffffffff83295ca5>] entry_SYSCALL64_slow_path+0x25/0x25 > > Code: 90 00 b4 0b 83 c7 00 f1 f1 f1 f1 4c 8d 6d e0 c7 40 04 00 00 00 f4 c7 40 08 f3 > f3 f3 f3 48 89 d8 48 c1 e8 03 c7 45 b4 00 00 00 00 <80> 3c 30 00 75 78 48 8d 7b 08 49 > 8d 75 c0 48 b8 00 00 00 00 00 > > RIP [<ffffffff830bb46b>] tipc_node_xmit_skb+0x6b/0x140 > > RSP <ffff8800595bfce8> > > ---[ end trace 57b0484e351e71f1 ]--- > > > > I feel like we should maybe return -ENOMEM or -ENOBUFS, but I'm not sure > userspace is equipped to handle that. Anyway, this is better than a GPF and looks > somewhat consistent with other tipc_msg_create() callers. > > > > Cc: stable@vger.kernel.org > > Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> > > --- > > net/tipc/socket.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/net/tipc/socket.c b/net/tipc/socket.c index c49b8df..f9f5f3c 100644 > > --- a/net/tipc/socket.c > > +++ b/net/tipc/socket.c > > @@ -2180,7 +2180,8 @@ restart: > > TIPC_CONN_MSG, SHORT_H_SIZE, > > 0, dnode, onode, dport, oport, > > TIPC_CONN_SHUTDOWN); > > - tipc_node_xmit_skb(net, skb, dnode, tsk->portid); > > + if (skb) > > + tipc_node_xmit_skb(net, skb, dnode, tsk- > >portid); > > } > > tsk->connected = 0; > > sock->state = SS_DISCONNECTING; > > -- > > 1.9.1 > >
From: Jon Maloy <jon.maloy@ericsson.com> Date: Mon, 15 Aug 2016 19:48:24 +0000 > Sorry, I just came back from vacation today. > > Acked-by: Jon Maloy <jon.maloy@ericsson.com> Applied.
diff --git a/net/tipc/socket.c b/net/tipc/socket.c index c49b8df..f9f5f3c 100644 --- a/net/tipc/socket.c +++ b/net/tipc/socket.c @@ -2180,7 +2180,8 @@ restart: TIPC_CONN_MSG, SHORT_H_SIZE, 0, dnode, onode, dport, oport, TIPC_CONN_SHUTDOWN); - tipc_node_xmit_skb(net, skb, dnode, tsk->portid); + if (skb) + tipc_node_xmit_skb(net, skb, dnode, tsk->portid); } tsk->connected = 0; sock->state = SS_DISCONNECTING;