Re: [tipc-discussion] [PATCH net] tipc: fix uninit-value in tipc_nl_compat_link_set

2019-01-07 Thread Ying Xue 
On 1/7/19 9:38 PM, David Miller wrote:
> From: Ying Xue 
> Date: Mon, 7 Jan 2019 19:29:52 +0800
> 
>> This is because lc->name string is not validated before it's used.
> 
> It looks like we have several situations like this, not just this one.
> 
> For example, tipc_nl_compat_bearer_{enable,disable}() with b->name.
> 
> Next, tipc_nl_compat_media_set() and tipc_nl_compat_bearer_set().
> 
> On input, tipc_nl_compat_link_stat_dump() blindly does a strcmp()
> on one of these strings.
> 
> In fact, this entire file is full of errors of this sort.
> 
> Can you please address all of them, perhaps using a helper of
> some kind to consolidate the logic?
> 

Thank you for your good suggestions. I will solve them as soon as possible.

Regards,
Ying

> Thank you.
> 


___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Re: [tipc-discussion] [PATCH net] tipc: fix uninit-value in tipc_nl_compat_link_set

2019-01-07 Thread David Miller 
From: Ying Xue 
Date: Mon, 7 Jan 2019 19:29:52 +0800

> This is because lc->name string is not validated before it's used.

It looks like we have several situations like this, not just this one.

For example, tipc_nl_compat_bearer_{enable,disable}() with b->name.

Next, tipc_nl_compat_media_set() and tipc_nl_compat_bearer_set().

On input, tipc_nl_compat_link_stat_dump() blindly does a strcmp()
on one of these strings.

In fact, this entire file is full of errors of this sort.

Can you please address all of them, perhaps using a helper of
some kind to consolidate the logic?

Thank you.


___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

[tipc-discussion] [PATCH net] tipc: fix uninit-value in tipc_nl_compat_link_set

2019-01-07 Thread Ying Xue 
syzbot reports following splat:

BUG: KMSAN: uninit-value in strlen+0x3b/0xa0 lib/string.c:486
CPU: 1 PID: 9306 Comm: syz-executor172 Not tainted 4.20.0-rc7+ #2
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x173/0x1d0 lib/dump_stack.c:113
  kmsan_report+0x12e/0x2a0 mm/kmsan/kmsan.c:613
  __msan_warning+0x82/0xf0 mm/kmsan/kmsan_instr.c:313
  strlen+0x3b/0xa0 lib/string.c:486
  nla_put_string include/net/netlink.h:1154 [inline]
  __tipc_nl_compat_link_set net/tipc/netlink_compat.c:708 [inline]
  tipc_nl_compat_link_set+0x929/0x1220 net/tipc/netlink_compat.c:744
  __tipc_nl_compat_doit net/tipc/netlink_compat.c:311 [inline]
  tipc_nl_compat_doit+0x3aa/0xaf0 net/tipc/netlink_compat.c:344
  tipc_nl_compat_handle net/tipc/netlink_compat.c:1107 [inline]
  tipc_nl_compat_recv+0x14d7/0x2760 net/tipc/netlink_compat.c:1210
  genl_family_rcv_msg net/netlink/genetlink.c:601 [inline]
  genl_rcv_msg+0x185f/0x1a60 net/netlink/genetlink.c:626
  netlink_rcv_skb+0x444/0x640 net/netlink/af_netlink.c:2477
  genl_rcv+0x63/0x80 net/netlink/genetlink.c:637
  netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
  netlink_unicast+0xf40/0x1020 net/netlink/af_netlink.c:1336
  netlink_sendmsg+0x127f/0x1300 net/netlink/af_netlink.c:1917
  sock_sendmsg_nosec net/socket.c:621 [inline]
  sock_sendmsg net/socket.c:631 [inline]
  ___sys_sendmsg+0xdb9/0x11b0 net/socket.c:2116
  __sys_sendmsg net/socket.c:2154 [inline]
  __do_sys_sendmsg net/socket.c:2163 [inline]
  __se_sys_sendmsg+0x305/0x460 net/socket.c:2161
  __x64_sys_sendmsg+0x4a/0x70 net/socket.c:2161
  do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:291
  entry_SYSCALL_64_after_hwframe+0x63/0xe7

The uninitialised access happened in
nla_put_string(skb, TIPC_NLA_LINK_NAME, lc->name)

This is because lc->name string is not validated before it's used.

Reported-by: syzbot+d78b8a29241a195ae...@syzkaller.appspotmail.com
Signed-off-by: Ying Xue 
---
 net/tipc/netlink_compat.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
index 21f6ccc..bbf3f5a 100644
--- a/net/tipc/netlink_compat.c
+++ b/net/tipc/netlink_compat.c
@@ -705,6 +705,9 @@ static int __tipc_nl_compat_link_set(struct sk_buff *skb,
if (!link)
return -EMSGSIZE;
 
+   if (!memchr(lc->name, '\0', TIPC_MAX_LINK_NAME))
+   return -EINVAL;
+
if (nla_put_string(skb, TIPC_NLA_LINK_NAME, lc->name))
return -EMSGSIZE;
 
-- 
2.7.4



___
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion