* [Patch net] tipc: fix a null pointer deref on error path
@ 2017-12-04 3:54 Cong Wang
2017-12-04 12:43 ` Ying Xue
0 siblings, 1 reply; 3+ messages in thread
From: Cong Wang @ 2017-12-04 3:54 UTC (permalink / raw)
To: netdev; +Cc: tipc-discussion, Cong Wang, Jon Maloy, Ying Xue
In tipc_topsrv_kern_subscr() when s->tipc_conn_new() fails
we call tipc_close_conn() to clean up, but in this case
con->usr_data is NULL, tipc_subscrb_delete() should be skipped.
This fixes the folllowing crash:
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:
CPU: 0 PID: 3085 Comm: syzkaller064164 Not tainted 4.15.0-rc1+ #137
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
task: 00000000c24413a5 task.stack: 000000005e8160b5
RIP: 0010:__lock_acquire+0xd55/0x47f0 kernel/locking/lockdep.c:3378
RSP: 0018:ffff8801cb5474a8 EFLAGS: 00010002
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000004 RSI: 0000000000000000 RDI: ffffffff85ecb400
RBP: ffff8801cb547830 R08: 0000000000000001 R09: 0000000000000000
R10: 0000000000000000 R11: ffffffff87489d60 R12: ffff8801cd2980c0
R13: 0000000000000000 R14: 0000000000000001 R15: 0000000000000020
FS: 00000000014ee880(0000) GS:ffff8801db400000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ffee2426e40 CR3: 00000001cb85a000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:4004
__raw_spin_lock_bh include/linux/spinlock_api_smp.h:135 [inline]
_raw_spin_lock_bh+0x31/0x40 kernel/locking/spinlock.c:175
spin_lock_bh include/linux/spinlock.h:320 [inline]
tipc_subscrb_subscrp_delete+0x8f/0x470 net/tipc/subscr.c:201
tipc_subscrb_delete net/tipc/subscr.c:238 [inline]
tipc_subscrb_release_cb+0x17/0x30 net/tipc/subscr.c:316
tipc_close_conn+0x171/0x270 net/tipc/server.c:204
tipc_topsrv_kern_subscr+0x724/0x810 net/tipc/server.c:514
tipc_group_create+0x702/0x9c0 net/tipc/group.c:184
tipc_sk_join net/tipc/socket.c:2747 [inline]
tipc_setsockopt+0x249/0xc10 net/tipc/socket.c:2861
SYSC_setsockopt net/socket.c:1851 [inline]
SyS_setsockopt+0x189/0x360 net/socket.c:1830
entry_SYSCALL_64_fastpath+0x1f/0x96
Reported-by: syzbot <syzkaller@googlegroups.com>
Cc: Jon Maloy <jon.maloy@ericsson.com>
Cc: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/tipc/subscr.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c
index 251065dfd8df..2402a019b8ec 100644
--- a/net/tipc/subscr.c
+++ b/net/tipc/subscr.c
@@ -235,6 +235,8 @@ static struct tipc_subscriber *tipc_subscrb_create(int conid)
static void tipc_subscrb_delete(struct tipc_subscriber *subscriber)
{
+ if (!subscriber)
+ return;
tipc_subscrb_subscrp_delete(subscriber, NULL);
tipc_subscrb_put(subscriber);
}
--
2.13.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Patch net] tipc: fix a null pointer deref on error path
2017-12-04 3:54 [Patch net] tipc: fix a null pointer deref on error path Cong Wang
@ 2017-12-04 12:43 ` Ying Xue
2017-12-04 18:19 ` Cong Wang
0 siblings, 1 reply; 3+ messages in thread
From: Ying Xue @ 2017-12-04 12:43 UTC (permalink / raw)
To: Cong Wang, netdev; +Cc: tipc-discussion
On 12/04/2017 11:54 AM, Cong Wang wrote:
> In tipc_topsrv_kern_subscr() when s->tipc_conn_new() fails
> we call tipc_close_conn() to clean up, but in this case
> con->usr_data is NULL, tipc_subscrb_delete() should be skipped.
>
> This fixes the folllowing crash:
>
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault: 0000 [#1] SMP KASAN
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 0 PID: 3085 Comm: syzkaller064164 Not tainted 4.15.0-rc1+ #137
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> task: 00000000c24413a5 task.stack: 000000005e8160b5
> RIP: 0010:__lock_acquire+0xd55/0x47f0 kernel/locking/lockdep.c:3378
> RSP: 0018:ffff8801cb5474a8 EFLAGS: 00010002
> RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
> RDX: 0000000000000004 RSI: 0000000000000000 RDI: ffffffff85ecb400
> RBP: ffff8801cb547830 R08: 0000000000000001 R09: 0000000000000000
> R10: 0000000000000000 R11: ffffffff87489d60 R12: ffff8801cd2980c0
> R13: 0000000000000000 R14: 0000000000000001 R15: 0000000000000020
> FS: 00000000014ee880(0000) GS:ffff8801db400000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007ffee2426e40 CR3: 00000001cb85a000 CR4: 00000000001406f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:4004
> __raw_spin_lock_bh include/linux/spinlock_api_smp.h:135 [inline]
> _raw_spin_lock_bh+0x31/0x40 kernel/locking/spinlock.c:175
> spin_lock_bh include/linux/spinlock.h:320 [inline]
> tipc_subscrb_subscrp_delete+0x8f/0x470 net/tipc/subscr.c:201
> tipc_subscrb_delete net/tipc/subscr.c:238 [inline]
> tipc_subscrb_release_cb+0x17/0x30 net/tipc/subscr.c:316
> tipc_close_conn+0x171/0x270 net/tipc/server.c:204
> tipc_topsrv_kern_subscr+0x724/0x810 net/tipc/server.c:514
> tipc_group_create+0x702/0x9c0 net/tipc/group.c:184
> tipc_sk_join net/tipc/socket.c:2747 [inline]
> tipc_setsockopt+0x249/0xc10 net/tipc/socket.c:2861
> SYSC_setsockopt net/socket.c:1851 [inline]
> SyS_setsockopt+0x189/0x360 net/socket.c:1830
> entry_SYSCALL_64_fastpath+0x1f/0x96
>
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Cc: Jon Maloy <jon.maloy@ericsson.com>
> Cc: Ying Xue <ying.xue@windriver.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> net/tipc/subscr.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c
> index 251065dfd8df..2402a019b8ec 100644
> --- a/net/tipc/subscr.c
> +++ b/net/tipc/subscr.c
> @@ -235,6 +235,8 @@ static struct tipc_subscriber *tipc_subscrb_create(int conid)
>
> static void tipc_subscrb_delete(struct tipc_subscriber *subscriber)
> {
> + if (!subscriber)
> + return;
The real root cause of this crash is because when it was failed to
create a connection through s->tipc_conn_new() in
tipc_topsrv_kern_subscr(), we just should release the connection
instance with conn_put() rather than calling tipc_close_conn() to close
the connection. So a proper fix might look like:
diff --git a/net/tipc/server.c b/net/tipc/server.c
index acaef80..9f7afad 100644
--- a/net/tipc/server.c
+++ b/net/tipc/server.c
@@ -508,15 +508,15 @@ bool tipc_topsrv_kern_subscr(struct net *net, u32
port, u32 type,
return false;
*conid = con->conid;
+ con->sock = NULL;
s = con->server;
scbr = s->tipc_conn_new(*conid);
if (!scbr) {
- tipc_close_conn(con);
+ conn_put(con);
return false;
}
con->usr_data = scbr;
- con->sock = NULL;
s->tipc_conn_recvmsg(net, *conid, NULL, scbr, &sub, sizeof(sub));
return true;
}
> tipc_subscrb_subscrp_delete(subscriber, NULL);
> tipc_subscrb_put(subscriber);
> }
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Patch net] tipc: fix a null pointer deref on error path
2017-12-04 12:43 ` Ying Xue
@ 2017-12-04 18:19 ` Cong Wang
0 siblings, 0 replies; 3+ messages in thread
From: Cong Wang @ 2017-12-04 18:19 UTC (permalink / raw)
To: Ying Xue; +Cc: Linux Kernel Network Developers, tipc-discussion, Jon Maloy
On Mon, Dec 4, 2017 at 4:43 AM, Ying Xue <ying.xue@windriver.com> wrote:
> On 12/04/2017 11:54 AM, Cong Wang wrote:
>> In tipc_topsrv_kern_subscr() when s->tipc_conn_new() fails
>> we call tipc_close_conn() to clean up, but in this case
>> con->usr_data is NULL, tipc_subscrb_delete() should be skipped.
>>
>> This fixes the folllowing crash:
>>
>> kasan: GPF could be caused by NULL-ptr deref or user memory access
>> general protection fault: 0000 [#1] SMP KASAN
>> Dumping ftrace buffer:
>> (ftrace buffer empty)
>> Modules linked in:
>> CPU: 0 PID: 3085 Comm: syzkaller064164 Not tainted 4.15.0-rc1+ #137
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>> task: 00000000c24413a5 task.stack: 000000005e8160b5
>> RIP: 0010:__lock_acquire+0xd55/0x47f0 kernel/locking/lockdep.c:3378
>> RSP: 0018:ffff8801cb5474a8 EFLAGS: 00010002
>> RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
>> RDX: 0000000000000004 RSI: 0000000000000000 RDI: ffffffff85ecb400
>> RBP: ffff8801cb547830 R08: 0000000000000001 R09: 0000000000000000
>> R10: 0000000000000000 R11: ffffffff87489d60 R12: ffff8801cd2980c0
>> R13: 0000000000000000 R14: 0000000000000001 R15: 0000000000000020
>> FS: 00000000014ee880(0000) GS:ffff8801db400000(0000) knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 00007ffee2426e40 CR3: 00000001cb85a000 CR4: 00000000001406f0
>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> Call Trace:
>> lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:4004
>> __raw_spin_lock_bh include/linux/spinlock_api_smp.h:135 [inline]
>> _raw_spin_lock_bh+0x31/0x40 kernel/locking/spinlock.c:175
>> spin_lock_bh include/linux/spinlock.h:320 [inline]
>> tipc_subscrb_subscrp_delete+0x8f/0x470 net/tipc/subscr.c:201
>> tipc_subscrb_delete net/tipc/subscr.c:238 [inline]
>> tipc_subscrb_release_cb+0x17/0x30 net/tipc/subscr.c:316
>> tipc_close_conn+0x171/0x270 net/tipc/server.c:204
>> tipc_topsrv_kern_subscr+0x724/0x810 net/tipc/server.c:514
>> tipc_group_create+0x702/0x9c0 net/tipc/group.c:184
>> tipc_sk_join net/tipc/socket.c:2747 [inline]
>> tipc_setsockopt+0x249/0xc10 net/tipc/socket.c:2861
>> SYSC_setsockopt net/socket.c:1851 [inline]
>> SyS_setsockopt+0x189/0x360 net/socket.c:1830
>> entry_SYSCALL_64_fastpath+0x1f/0x96
>>
>> Reported-by: syzbot <syzkaller@googlegroups.com>
>> Cc: Jon Maloy <jon.maloy@ericsson.com>
>> Cc: Ying Xue <ying.xue@windriver.com>
>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>> ---
>> net/tipc/subscr.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c
>> index 251065dfd8df..2402a019b8ec 100644
>> --- a/net/tipc/subscr.c
>> +++ b/net/tipc/subscr.c
>> @@ -235,6 +235,8 @@ static struct tipc_subscriber *tipc_subscrb_create(int conid)
>>
>> static void tipc_subscrb_delete(struct tipc_subscriber *subscriber)
>> {
>> + if (!subscriber)
>> + return;
>
> The real root cause of this crash is because when it was failed to
> create a connection through s->tipc_conn_new() in
> tipc_topsrv_kern_subscr(), we just should release the connection
> instance with conn_put() rather than calling tipc_close_conn() to close
> the connection. So a proper fix might look like:
Good point! But con->sock should be already NULL since
we use kzalloc() to allocate con, so just replacing tipc_close_conn()
with con_put() should work.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-12-04 18:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-04 3:54 [Patch net] tipc: fix a null pointer deref on error path Cong Wang
2017-12-04 12:43 ` Ying Xue
2017-12-04 18:19 ` Cong Wang
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).