linux-kernel-mentees.lists.linuxfoundation.org archive mirror
 help / color / mirror / Atom feed
* [Linux-kernel-mentees] [PATCH] net: tipc: fix general protection fault in tipc_conn_delete_sub
@ 2020-07-27 13:10 B K Karthik
  2020-07-27 13:22 ` Greg KH
  2020-07-27 15:52 ` kernel test robot
  0 siblings, 2 replies; 7+ messages in thread
From: B K Karthik @ 2020-07-27 13:10 UTC (permalink / raw)
  To: Jon Maloy, Ying Xue, David S. Miller, Jakub Kicinski, netdev,
	tipc-discussion, linux-kernel, gregkh, skhan,
	linux-kernel-mentees


[-- Attachment #1.1: Type: text/plain, Size: 3492 bytes --]

fix a general protection fault in tipc_conn_delete_sub
by checking for the existance of con->server.
prevent a null-ptr-deref by returning -EINVAL when
con->server is NULL

general protection fault, probably for non-canonical address 0xdffffc0000000014: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x00000000000000a0-0x00000000000000a7]
CPU: 1 PID: 113 Comm: kworker/u4:3 Not tainted 5.6.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: tipc_send tipc_conn_send_work
RIP: 0010:tipc_conn_delete_sub+0x54/0x440 net/tipc/topsrv.c:231
Code: 48 c1 ea 03 80 3c 02 00 0f 85 f0 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b 6b 18 48 8d bd a0 00 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 c0 03 00 00 48 c7 c0 34 0b 8a 8a 4c 8b a5 a0 00
RSP: 0018:ffffc900012d7b58 EFLAGS: 00010206
RAX: dffffc0000000000 RBX: ffff8880a8269c00 RCX: ffffffff8789ca01
RDX: 0000000000000014 RSI: ffffffff8789a059 RDI: 00000000000000a0
RBP: 0000000000000000 R08: ffff8880a8d88380 R09: fffffbfff18577a8
R10: fffffbfff18577a7 R11: ffffffff8c2bbd3f R12: dffffc0000000000
R13: ffff888093d35a18 R14: ffff8880a8269c00 R15: ffff888093d35a00
FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000076c000 CR3: 000000009441d000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 tipc_conn_send_to_sock+0x380/0x560 net/tipc/topsrv.c:266
 tipc_conn_send_work+0x6f/0x90 net/tipc/topsrv.c:304
 process_one_work+0x965/0x16a0 kernel/workqueue.c:2266
 worker_thread+0x96/0xe20 kernel/workqueue.c:2412
 kthread+0x388/0x470 kernel/kthread.c:268
 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Modules linked in:
---[ end trace 2c161a84be832606 ]---
RIP: 0010:tipc_conn_delete_sub+0x54/0x440 net/tipc/topsrv.c:231
Code: 48 c1 ea 03 80 3c 02 00 0f 85 f0 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b 6b 18 48 8d bd a0 00 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 c0 03 00 00 48 c7 c0 34 0b 8a 8a 4c 8b a5 a0 00
RSP: 0018:ffffc900012d7b58 EFLAGS: 00010206
RAX: dffffc0000000000 RBX: ffff8880a8269c00 RCX: ffffffff8789ca01
RDX: 0000000000000014 RSI: ffffffff8789a059 RDI: 00000000000000a0
RBP: 0000000000000000 R08: ffff8880a8d88380 R09: fffffbfff18577a8
R10: fffffbfff18577a7 R11: ffffffff8c2bbd3f R12: dffffc0000000000
R13: ffff888093d35a18 R14: ffff8880a8269c00 R15: ffff888093d35a00
FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020800000 CR3: 0000000091b8e000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400

Reported-and-tested-by: syzbot+55a38037455d0351efd3@syzkaller.appspotmail.com
Signed-off-by: B K Karthik <bkkarthik@pesu.pes.edu>
---
 net/tipc/topsrv.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c
index 1489cfb941d8..6c8d0c6bb112 100644
--- a/net/tipc/topsrv.c
+++ b/net/tipc/topsrv.c
@@ -255,6 +255,9 @@ static void tipc_conn_send_to_sock(struct tipc_conn *con)
 	int count = 0;
 	int ret;
 
+	if (!con->server)
+		return -EINVAL;
+
 	spin_lock_bh(&con->outqueue_lock);
 
 	while (!list_empty(queue)) {
-- 
2.20.1


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[-- Attachment #2: Type: text/plain, Size: 201 bytes --]

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] net: tipc: fix general protection fault in tipc_conn_delete_sub
  2020-07-27 13:10 [Linux-kernel-mentees] [PATCH] net: tipc: fix general protection fault in tipc_conn_delete_sub B K Karthik
@ 2020-07-27 13:22 ` Greg KH
  2020-07-27 14:16   ` B K Karthik
  2020-07-27 15:52 ` kernel test robot
  1 sibling, 1 reply; 7+ messages in thread
From: Greg KH @ 2020-07-27 13:22 UTC (permalink / raw)
  To: B K Karthik
  Cc: netdev, linux-kernel, Jon Maloy, tipc-discussion, Ying Xue,
	Jakub Kicinski, linux-kernel-mentees, David S. Miller

On Mon, Jul 27, 2020 at 06:40:57PM +0530, B K Karthik wrote:
> fix a general protection fault in tipc_conn_delete_sub
> by checking for the existance of con->server.
> prevent a null-ptr-deref by returning -EINVAL when
> con->server is NULL
> 
> general protection fault, probably for non-canonical address 0xdffffc0000000014: 0000 [#1] PREEMPT SMP KASAN
> KASAN: null-ptr-deref in range [0x00000000000000a0-0x00000000000000a7]
> CPU: 1 PID: 113 Comm: kworker/u4:3 Not tainted 5.6.0-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Workqueue: tipc_send tipc_conn_send_work
> RIP: 0010:tipc_conn_delete_sub+0x54/0x440 net/tipc/topsrv.c:231
> Code: 48 c1 ea 03 80 3c 02 00 0f 85 f0 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b 6b 18 48 8d bd a0 00 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 c0 03 00 00 48 c7 c0 34 0b 8a 8a 4c 8b a5 a0 00
> RSP: 0018:ffffc900012d7b58 EFLAGS: 00010206
> RAX: dffffc0000000000 RBX: ffff8880a8269c00 RCX: ffffffff8789ca01
> RDX: 0000000000000014 RSI: ffffffff8789a059 RDI: 00000000000000a0
> RBP: 0000000000000000 R08: ffff8880a8d88380 R09: fffffbfff18577a8
> R10: fffffbfff18577a7 R11: ffffffff8c2bbd3f R12: dffffc0000000000
> R13: ffff888093d35a18 R14: ffff8880a8269c00 R15: ffff888093d35a00
> FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 000000000076c000 CR3: 000000009441d000 CR4: 00000000001406e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>  tipc_conn_send_to_sock+0x380/0x560 net/tipc/topsrv.c:266
>  tipc_conn_send_work+0x6f/0x90 net/tipc/topsrv.c:304
>  process_one_work+0x965/0x16a0 kernel/workqueue.c:2266
>  worker_thread+0x96/0xe20 kernel/workqueue.c:2412
>  kthread+0x388/0x470 kernel/kthread.c:268
>  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
> Modules linked in:
> ---[ end trace 2c161a84be832606 ]---
> RIP: 0010:tipc_conn_delete_sub+0x54/0x440 net/tipc/topsrv.c:231
> Code: 48 c1 ea 03 80 3c 02 00 0f 85 f0 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b 6b 18 48 8d bd a0 00 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 c0 03 00 00 48 c7 c0 34 0b 8a 8a 4c 8b a5 a0 00
> RSP: 0018:ffffc900012d7b58 EFLAGS: 00010206
> RAX: dffffc0000000000 RBX: ffff8880a8269c00 RCX: ffffffff8789ca01
> RDX: 0000000000000014 RSI: ffffffff8789a059 RDI: 00000000000000a0
> RBP: 0000000000000000 R08: ffff8880a8d88380 R09: fffffbfff18577a8
> R10: fffffbfff18577a7 R11: ffffffff8c2bbd3f R12: dffffc0000000000
> R13: ffff888093d35a18 R14: ffff8880a8269c00 R15: ffff888093d35a00
> FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020800000 CR3: 0000000091b8e000 CR4: 00000000001406e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> 
> Reported-and-tested-by: syzbot+55a38037455d0351efd3@syzkaller.appspotmail.com
> Signed-off-by: B K Karthik <bkkarthik@pesu.pes.edu>
> ---
>  net/tipc/topsrv.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c
> index 1489cfb941d8..6c8d0c6bb112 100644
> --- a/net/tipc/topsrv.c
> +++ b/net/tipc/topsrv.c
> @@ -255,6 +255,9 @@ static void tipc_conn_send_to_sock(struct tipc_conn *con)
>  	int count = 0;
>  	int ret;
>  
> +	if (!con->server)
> +		return -EINVAL;

What is wrong with looking at the srv local variable instead?

And how is server getting set to NULL and this function still being
called?

thanks,

greg k-h
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] net: tipc: fix general protection fault in tipc_conn_delete_sub
  2020-07-27 13:22 ` Greg KH
@ 2020-07-27 14:16   ` B K Karthik
  2020-07-27 14:24     ` Greg KH
  0 siblings, 1 reply; 7+ messages in thread
From: B K Karthik @ 2020-07-27 14:16 UTC (permalink / raw)
  To: Greg KH
  Cc: Linux Kernel Network Developers, LKML, Jon Maloy,
	tipc-discussion, Ying Xue, Jakub Kicinski, linux-kernel-mentees,
	David S. Miller

On Mon, Jul 27, 2020 at 6:53 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Jul 27, 2020 at 06:40:57PM +0530, B K Karthik wrote:
> > fix a general protection fault in tipc_conn_delete_sub
> > by checking for the existance of con->server.
> > prevent a null-ptr-deref by returning -EINVAL when
> > con->server is NULL
> >
> > general protection fault, probably for non-canonical address 0xdffffc0000000014: 0000 [#1] PREEMPT SMP KASAN
> > KASAN: null-ptr-deref in range [0x00000000000000a0-0x00000000000000a7]
> > CPU: 1 PID: 113 Comm: kworker/u4:3 Not tainted 5.6.0-syzkaller #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> > Workqueue: tipc_send tipc_conn_send_work
> > RIP: 0010:tipc_conn_delete_sub+0x54/0x440 net/tipc/topsrv.c:231
> > Code: 48 c1 ea 03 80 3c 02 00 0f 85 f0 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b 6b 18 48 8d bd a0 00 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 c0 03 00 00 48 c7 c0 34 0b 8a 8a 4c 8b a5 a0 00
> > RSP: 0018:ffffc900012d7b58 EFLAGS: 00010206
> > RAX: dffffc0000000000 RBX: ffff8880a8269c00 RCX: ffffffff8789ca01
> > RDX: 0000000000000014 RSI: ffffffff8789a059 RDI: 00000000000000a0
> > RBP: 0000000000000000 R08: ffff8880a8d88380 R09: fffffbfff18577a8
> > R10: fffffbfff18577a7 R11: ffffffff8c2bbd3f R12: dffffc0000000000
> > R13: ffff888093d35a18 R14: ffff8880a8269c00 R15: ffff888093d35a00
> > FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 000000000076c000 CR3: 000000009441d000 CR4: 00000000001406e0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > Call Trace:
> >  tipc_conn_send_to_sock+0x380/0x560 net/tipc/topsrv.c:266
> >  tipc_conn_send_work+0x6f/0x90 net/tipc/topsrv.c:304
> >  process_one_work+0x965/0x16a0 kernel/workqueue.c:2266
> >  worker_thread+0x96/0xe20 kernel/workqueue.c:2412
> >  kthread+0x388/0x470 kernel/kthread.c:268
> >  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
> > Modules linked in:
> > ---[ end trace 2c161a84be832606 ]---
> > RIP: 0010:tipc_conn_delete_sub+0x54/0x440 net/tipc/topsrv.c:231
> > Code: 48 c1 ea 03 80 3c 02 00 0f 85 f0 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b 6b 18 48 8d bd a0 00 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 c0 03 00 00 48 c7 c0 34 0b 8a 8a 4c 8b a5 a0 00
> > RSP: 0018:ffffc900012d7b58 EFLAGS: 00010206
> > RAX: dffffc0000000000 RBX: ffff8880a8269c00 RCX: ffffffff8789ca01
> > RDX: 0000000000000014 RSI: ffffffff8789a059 RDI: 00000000000000a0
> > RBP: 0000000000000000 R08: ffff8880a8d88380 R09: fffffbfff18577a8
> > R10: fffffbfff18577a7 R11: ffffffff8c2bbd3f R12: dffffc0000000000
> > R13: ffff888093d35a18 R14: ffff8880a8269c00 R15: ffff888093d35a00
> > FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000020800000 CR3: 0000000091b8e000 CR4: 00000000001406e0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> >
> > Reported-and-tested-by: syzbot+55a38037455d0351efd3@syzkaller.appspotmail.com
> > Signed-off-by: B K Karthik <bkkarthik@pesu.pes.edu>
> > ---
> >  net/tipc/topsrv.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c
> > index 1489cfb941d8..6c8d0c6bb112 100644
> > --- a/net/tipc/topsrv.c
> > +++ b/net/tipc/topsrv.c
> > @@ -255,6 +255,9 @@ static void tipc_conn_send_to_sock(struct tipc_conn *con)
> >       int count = 0;
> >       int ret;
> >
> > +     if (!con->server)
> > +             return -EINVAL;
>
> What is wrong with looking at the srv local variable instead?
>
> And how is server getting set to NULL and this function still being
> called?

tipc_conn_send_work makes a call to connected() which just returns con
&& test_bit(CF_CONNECTED, &con->flags)
maybe we can add this check to the implementation of connection() if
you agree, but I found this solution to be fairly simpler because I'm
not sure where else connected() is being used, and I did not want to
introduce redundant function calls.

Yes we can replace con->server with the local variable srv. Extremely
sorry, I hadn't noticed it earlier.

please let me know if i've wrongly understood any of these.
thanks,

karthik
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] net: tipc: fix general protection fault in tipc_conn_delete_sub
  2020-07-27 14:16   ` B K Karthik
@ 2020-07-27 14:24     ` Greg KH
  2020-07-28 16:15       ` Ying Xue
  0 siblings, 1 reply; 7+ messages in thread
From: Greg KH @ 2020-07-27 14:24 UTC (permalink / raw)
  To: B K Karthik
  Cc: Linux Kernel Network Developers, LKML, Jon Maloy,
	tipc-discussion, Ying Xue, Jakub Kicinski, linux-kernel-mentees,
	David S. Miller

On Mon, Jul 27, 2020 at 07:46:05PM +0530, B K Karthik wrote:
> On Mon, Jul 27, 2020 at 6:53 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Jul 27, 2020 at 06:40:57PM +0530, B K Karthik wrote:
> > > fix a general protection fault in tipc_conn_delete_sub
> > > by checking for the existance of con->server.
> > > prevent a null-ptr-deref by returning -EINVAL when
> > > con->server is NULL
> > >
> > > general protection fault, probably for non-canonical address 0xdffffc0000000014: 0000 [#1] PREEMPT SMP KASAN
> > > KASAN: null-ptr-deref in range [0x00000000000000a0-0x00000000000000a7]
> > > CPU: 1 PID: 113 Comm: kworker/u4:3 Not tainted 5.6.0-syzkaller #0
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> > > Workqueue: tipc_send tipc_conn_send_work
> > > RIP: 0010:tipc_conn_delete_sub+0x54/0x440 net/tipc/topsrv.c:231
> > > Code: 48 c1 ea 03 80 3c 02 00 0f 85 f0 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b 6b 18 48 8d bd a0 00 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 c0 03 00 00 48 c7 c0 34 0b 8a 8a 4c 8b a5 a0 00
> > > RSP: 0018:ffffc900012d7b58 EFLAGS: 00010206
> > > RAX: dffffc0000000000 RBX: ffff8880a8269c00 RCX: ffffffff8789ca01
> > > RDX: 0000000000000014 RSI: ffffffff8789a059 RDI: 00000000000000a0
> > > RBP: 0000000000000000 R08: ffff8880a8d88380 R09: fffffbfff18577a8
> > > R10: fffffbfff18577a7 R11: ffffffff8c2bbd3f R12: dffffc0000000000
> > > R13: ffff888093d35a18 R14: ffff8880a8269c00 R15: ffff888093d35a00
> > > FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
> > > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 000000000076c000 CR3: 000000009441d000 CR4: 00000000001406e0
> > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > Call Trace:
> > >  tipc_conn_send_to_sock+0x380/0x560 net/tipc/topsrv.c:266
> > >  tipc_conn_send_work+0x6f/0x90 net/tipc/topsrv.c:304
> > >  process_one_work+0x965/0x16a0 kernel/workqueue.c:2266
> > >  worker_thread+0x96/0xe20 kernel/workqueue.c:2412
> > >  kthread+0x388/0x470 kernel/kthread.c:268
> > >  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
> > > Modules linked in:
> > > ---[ end trace 2c161a84be832606 ]---
> > > RIP: 0010:tipc_conn_delete_sub+0x54/0x440 net/tipc/topsrv.c:231
> > > Code: 48 c1 ea 03 80 3c 02 00 0f 85 f0 03 00 00 48 b8 00 00 00 00 00 fc ff df 48 8b 6b 18 48 8d bd a0 00 00 00 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 c0 03 00 00 48 c7 c0 34 0b 8a 8a 4c 8b a5 a0 00
> > > RSP: 0018:ffffc900012d7b58 EFLAGS: 00010206
> > > RAX: dffffc0000000000 RBX: ffff8880a8269c00 RCX: ffffffff8789ca01
> > > RDX: 0000000000000014 RSI: ffffffff8789a059 RDI: 00000000000000a0
> > > RBP: 0000000000000000 R08: ffff8880a8d88380 R09: fffffbfff18577a8
> > > R10: fffffbfff18577a7 R11: ffffffff8c2bbd3f R12: dffffc0000000000
> > > R13: ffff888093d35a18 R14: ffff8880a8269c00 R15: ffff888093d35a00
> > > FS:  0000000000000000(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
> > > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 0000000020800000 CR3: 0000000091b8e000 CR4: 00000000001406e0
> > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > >
> > > Reported-and-tested-by: syzbot+55a38037455d0351efd3@syzkaller.appspotmail.com
> > > Signed-off-by: B K Karthik <bkkarthik@pesu.pes.edu>
> > > ---
> > >  net/tipc/topsrv.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c
> > > index 1489cfb941d8..6c8d0c6bb112 100644
> > > --- a/net/tipc/topsrv.c
> > > +++ b/net/tipc/topsrv.c
> > > @@ -255,6 +255,9 @@ static void tipc_conn_send_to_sock(struct tipc_conn *con)
> > >       int count = 0;
> > >       int ret;
> > >
> > > +     if (!con->server)
> > > +             return -EINVAL;
> >
> > What is wrong with looking at the srv local variable instead?
> >
> > And how is server getting set to NULL and this function still being
> > called?
> 
> tipc_conn_send_work makes a call to connected() which just returns con
> && test_bit(CF_CONNECTED, &con->flags)
> maybe we can add this check to the implementation of connection() if
> you agree, but I found this solution to be fairly simpler because I'm
> not sure where else connected() is being used, and I did not want to
> introduce redundant function calls.

That's not what I asked here at all.

greg k-h
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] net: tipc: fix general protection fault in tipc_conn_delete_sub
  2020-07-27 13:10 [Linux-kernel-mentees] [PATCH] net: tipc: fix general protection fault in tipc_conn_delete_sub B K Karthik
  2020-07-27 13:22 ` Greg KH
@ 2020-07-27 15:52 ` kernel test robot
  2020-07-27 19:38   ` David Miller
  1 sibling, 1 reply; 7+ messages in thread
From: kernel test robot @ 2020-07-27 15:52 UTC (permalink / raw)
  To: B K Karthik, Jon Maloy, Ying Xue, David S. Miller,
	Jakub Kicinski, tipc-discussion, linux-kernel, gregkh, skhan,
	linux-kernel-mentees
  Cc: netdev, kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3273 bytes --]

Hi K,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.8-rc7 next-20200724]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/B-K-Karthik/net-tipc-fix-general-protection-fault-in-tipc_conn_delete_sub/20200727-211330
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 92ed301919932f777713b9172e525674157e983d
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   net/tipc/topsrv.c: In function 'tipc_conn_send_to_sock':
>> net/tipc/topsrv.c:259:10: warning: 'return' with a value, in function returning void [-Wreturn-type]
     259 |   return -EINVAL;
         |          ^
   net/tipc/topsrv.c:247:13: note: declared here
     247 | static void tipc_conn_send_to_sock(struct tipc_conn *con)
         |             ^~~~~~~~~~~~~~~~~~~~~~

vim +/return +259 net/tipc/topsrv.c

   246	
   247	static void tipc_conn_send_to_sock(struct tipc_conn *con)
   248	{
   249		struct list_head *queue = &con->outqueue;
   250		struct tipc_topsrv *srv = con->server;
   251		struct outqueue_entry *e;
   252		struct tipc_event *evt;
   253		struct msghdr msg;
   254		struct kvec iov;
   255		int count = 0;
   256		int ret;
   257	
   258		if (!con->server)
 > 259			return -EINVAL;
   260	
   261		spin_lock_bh(&con->outqueue_lock);
   262	
   263		while (!list_empty(queue)) {
   264			e = list_first_entry(queue, struct outqueue_entry, list);
   265			evt = &e->evt;
   266			spin_unlock_bh(&con->outqueue_lock);
   267	
   268			if (e->inactive)
   269				tipc_conn_delete_sub(con, &evt->s);
   270	
   271			memset(&msg, 0, sizeof(msg));
   272			msg.msg_flags = MSG_DONTWAIT;
   273			iov.iov_base = evt;
   274			iov.iov_len = sizeof(*evt);
   275			msg.msg_name = NULL;
   276	
   277			if (con->sock) {
   278				ret = kernel_sendmsg(con->sock, &msg, &iov,
   279						     1, sizeof(*evt));
   280				if (ret == -EWOULDBLOCK || ret == 0) {
   281					cond_resched();
   282					return;
   283				} else if (ret < 0) {
   284					return tipc_conn_close(con);
   285				}
   286			} else {
   287				tipc_topsrv_kern_evt(srv->net, evt);
   288			}
   289	
   290			/* Don't starve users filling buffers */
   291			if (++count >= MAX_SEND_MSG_COUNT) {
   292				cond_resched();
   293				count = 0;
   294			}
   295			spin_lock_bh(&con->outqueue_lock);
   296			list_del(&e->list);
   297			kfree(e);
   298		}
   299		spin_unlock_bh(&con->outqueue_lock);
   300	}
   301	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 61092 bytes --]

[-- Attachment #3: Type: text/plain, Size: 201 bytes --]

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] net: tipc: fix general protection fault in tipc_conn_delete_sub
  2020-07-27 15:52 ` kernel test robot
@ 2020-07-27 19:38   ` David Miller
  0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2020-07-27 19:38 UTC (permalink / raw)
  To: lkp
  Cc: kbuild-all, bkkarthik, linux-kernel, jmaloy, netdev,
	tipc-discussion, ying.xue, kuba, linux-kernel-mentees

From: kernel test robot <lkp@intel.com>
Date: Mon, 27 Jul 2020 23:52:50 +0800

> All warnings (new ones prefixed by >>):
> 
>    net/tipc/topsrv.c: In function 'tipc_conn_send_to_sock':
>>> net/tipc/topsrv.c:259:10: warning: 'return' with a value, in function returning void [-Wreturn-type]
>      259 |   return -EINVAL;
>          |          ^
>    net/tipc/topsrv.c:247:13: note: declared here
>      247 | static void tipc_conn_send_to_sock(struct tipc_conn *con)
>          |             ^~~~~~~~~~~~~~~~~~~~~~

Please look at the compiler output when you submit changes.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] net: tipc: fix general protection fault in tipc_conn_delete_sub
  2020-07-27 14:24     ` Greg KH
@ 2020-07-28 16:15       ` Ying Xue
  0 siblings, 0 replies; 7+ messages in thread
From: Ying Xue @ 2020-07-28 16:15 UTC (permalink / raw)
  To: Greg KH, B K Karthik
  Cc: Linux Kernel Network Developers, LKML, Jon Maloy,
	tipc-discussion, Jakub Kicinski, linux-kernel-mentees,
	David S. Miller

On 7/27/20 10:24 PM, Greg KH wrote:
>>>> diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c
>>>> index 1489cfb941d8..6c8d0c6bb112 100644
>>>> --- a/net/tipc/topsrv.c
>>>> +++ b/net/tipc/topsrv.c
>>>> @@ -255,6 +255,9 @@ static void tipc_conn_send_to_sock(struct tipc_conn *con)
>>>>       int count = 0;
>>>>       int ret;
>>>>
>>>> +     if (!con->server)
>>>> +             return -EINVAL;
>>> What is wrong with looking at the srv local variable instead?
>>>
>>> And how is server getting set to NULL and this function still being
>>> called?
>> tipc_conn_send_work makes a call to connected() which just returns con
>> && test_bit(CF_CONNECTED, &con->flags)
>> maybe we can add this check to the implementation of connection() if
>> you agree, but I found this solution to be fairly simpler because I'm
>> not sure where else connected() is being used, and I did not want to
>> introduce redundant function calls.
> That's not what I asked here at all
I agreed with Greg. The key problem is that we need to understand why
con->server got NULL, otherwise, we probably just hide the issue by
checking if it's NULL.

The topology server is created in kernel space as an internal TIPC
server and its life cycle is the same as TIPC network namespace.
Whenever the topology server accepts a connection from its client, it
will create a "con" which will be used to talk to the client and the
topology server instance (ie, "topsrv") will be attached to
"con->server". In theory, "con" cannot be died before its server, as a
result, con->server cannot become NULL in tipc_conn_send_to_sock().

So I suspect there is other potential issues which caused this problem,
for example, the refcount of "con" is not properly taken or put and this
case is triggered before of use-after-free, or race condition etc.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

end of thread, other threads:[~2020-07-28 19:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-27 13:10 [Linux-kernel-mentees] [PATCH] net: tipc: fix general protection fault in tipc_conn_delete_sub B K Karthik
2020-07-27 13:22 ` Greg KH
2020-07-27 14:16   ` B K Karthik
2020-07-27 14:24     ` Greg KH
2020-07-28 16:15       ` Ying Xue
2020-07-27 15:52 ` kernel test robot
2020-07-27 19:38   ` David Miller

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).