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: [email protected]
Signed-off-by: B K Karthik <[email protected]>
---
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
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: [email protected]
> Signed-off-by: B K Karthik <[email protected]>
> ---
> 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
On Mon, Jul 27, 2020 at 6:53 PM Greg KH <[email protected]> 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: [email protected]
> > Signed-off-by: B K Karthik <[email protected]>
> > ---
> > 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
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 <[email protected]>
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/[email protected]
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 <[email protected]> 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: [email protected]
> > > Signed-off-by: B K Karthik <[email protected]>
> > > ---
> > > 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
From: kernel test robot <[email protected]>
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.
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.