* [PATCH net 1/3] tipc: check bearer name with right length in tipc_nl_compat_bearer_enable
2019-03-31 14:50 [PATCH net 0/3] tipc: a batch of uninit-value fixes for netlink_compat Xin Long
@ 2019-03-31 14:50 ` Xin Long
2019-03-31 14:50 ` [PATCH net 2/3] tipc: check link name with right length in tipc_nl_compat_link_set Xin Long
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Xin Long @ 2019-03-31 14:50 UTC (permalink / raw)
To: network dev; +Cc: davem, Jon Maloy, Ying Xue, tipc-discussion, syzkaller
Syzbot reported the following crash:
BUG: KMSAN: uninit-value in memchr+0xce/0x110 lib/string.c:961
memchr+0xce/0x110 lib/string.c:961
string_is_valid net/tipc/netlink_compat.c:176 [inline]
tipc_nl_compat_bearer_enable+0x2c4/0x910 net/tipc/netlink_compat.c:401
__tipc_nl_compat_doit net/tipc/netlink_compat.c:321 [inline]
tipc_nl_compat_doit+0x3aa/0xaf0 net/tipc/netlink_compat.c:354
tipc_nl_compat_handle net/tipc/netlink_compat.c:1162 [inline]
tipc_nl_compat_recv+0x1ae7/0x2750 net/tipc/netlink_compat.c:1265
genl_family_rcv_msg net/netlink/genetlink.c:601 [inline]
genl_rcv_msg+0x185f/0x1a60 net/netlink/genetlink.c:626
netlink_rcv_skb+0x431/0x620 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+0xf3e/0x1020 net/netlink/af_netlink.c:1336
netlink_sendmsg+0x127f/0x1300 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:622 [inline]
sock_sendmsg net/socket.c:632 [inline]
Uninit was created at:
__alloc_skb+0x309/0xa20 net/core/skbuff.c:208
alloc_skb include/linux/skbuff.h:1012 [inline]
netlink_alloc_large_skb net/netlink/af_netlink.c:1182 [inline]
netlink_sendmsg+0xb82/0x1300 net/netlink/af_netlink.c:1892
sock_sendmsg_nosec net/socket.c:622 [inline]
sock_sendmsg net/socket.c:632 [inline]
It was triggered when the bearer name size < TIPC_MAX_BEARER_NAME,
it would check with a wrong len/TLV_GET_DATA_LEN(msg->req), which
also includes priority and disc_domain length.
This patch is to fix it by checking it with a right length:
'TLV_GET_DATA_LEN(msg->req) - offsetof(struct tipc_bearer_config, name)'.
Reported-by: syzbot+8b707430713eb46e1e45@syzkaller.appspotmail.com
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/tipc/netlink_compat.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
index 4ad3586..5f8e53c 100644
--- a/net/tipc/netlink_compat.c
+++ b/net/tipc/netlink_compat.c
@@ -397,7 +397,12 @@ static int tipc_nl_compat_bearer_enable(struct tipc_nl_compat_cmd_doit *cmd,
if (!bearer)
return -EMSGSIZE;
- len = min_t(int, TLV_GET_DATA_LEN(msg->req), TIPC_MAX_BEARER_NAME);
+ len = TLV_GET_DATA_LEN(msg->req);
+ len -= offsetof(struct tipc_bearer_config, name);
+ if (len <= 0)
+ return -EINVAL;
+
+ len = min_t(int, len, TIPC_MAX_BEARER_NAME);
if (!string_is_valid(b->name, len))
return -EINVAL;
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net 2/3] tipc: check link name with right length in tipc_nl_compat_link_set
2019-03-31 14:50 [PATCH net 0/3] tipc: a batch of uninit-value fixes for netlink_compat Xin Long
2019-03-31 14:50 ` [PATCH net 1/3] tipc: check bearer name with right length in tipc_nl_compat_bearer_enable Xin Long
@ 2019-03-31 14:50 ` Xin Long
2019-03-31 14:50 ` [PATCH net 3/3] tipc: handle the err returned from cmd header function Xin Long
2019-03-31 22:27 ` [PATCH net 0/3] tipc: a batch of uninit-value fixes for netlink_compat Jon Maloy
3 siblings, 0 replies; 6+ messages in thread
From: Xin Long @ 2019-03-31 14:50 UTC (permalink / raw)
To: network dev; +Cc: davem, Jon Maloy, Ying Xue, tipc-discussion, syzkaller
A similar issue as fixed by Patch "tipc: check bearer name with right
length in tipc_nl_compat_bearer_enable" was also found by syzbot in
tipc_nl_compat_link_set().
The length to check with should be 'TLV_GET_DATA_LEN(msg->req) -
offsetof(struct tipc_link_config, name)'.
Reported-by: syzbot+de00a87b8644a582ae79@syzkaller.appspotmail.com
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/tipc/netlink_compat.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
index 5f8e53c..0bfd03d6 100644
--- a/net/tipc/netlink_compat.c
+++ b/net/tipc/netlink_compat.c
@@ -771,7 +771,12 @@ static int tipc_nl_compat_link_set(struct tipc_nl_compat_cmd_doit *cmd,
lc = (struct tipc_link_config *)TLV_DATA(msg->req);
- len = min_t(int, TLV_GET_DATA_LEN(msg->req), TIPC_MAX_LINK_NAME);
+ len = TLV_GET_DATA_LEN(msg->req);
+ len -= offsetof(struct tipc_link_config, name);
+ if (len <= 0)
+ return -EINVAL;
+
+ len = min_t(int, len, TIPC_MAX_LINK_NAME);
if (!string_is_valid(lc->name, len))
return -EINVAL;
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 3/3] tipc: handle the err returned from cmd header function
2019-03-31 14:50 [PATCH net 0/3] tipc: a batch of uninit-value fixes for netlink_compat Xin Long
2019-03-31 14:50 ` [PATCH net 1/3] tipc: check bearer name with right length in tipc_nl_compat_bearer_enable Xin Long
2019-03-31 14:50 ` [PATCH net 2/3] tipc: check link name with right length in tipc_nl_compat_link_set Xin Long
@ 2019-03-31 14:50 ` Xin Long
2019-03-31 22:27 ` [PATCH net 0/3] tipc: a batch of uninit-value fixes for netlink_compat Jon Maloy
3 siblings, 0 replies; 6+ messages in thread
From: Xin Long @ 2019-03-31 14:50 UTC (permalink / raw)
To: network dev; +Cc: davem, Jon Maloy, Ying Xue, tipc-discussion, syzkaller
Syzbot found a crash:
BUG: KMSAN: uninit-value in tipc_nl_compat_name_table_dump+0x54f/0xcd0 net/tipc/netlink_compat.c:872
Call Trace:
tipc_nl_compat_name_table_dump+0x54f/0xcd0 net/tipc/netlink_compat.c:872
__tipc_nl_compat_dumpit+0x59e/0xda0 net/tipc/netlink_compat.c:215
tipc_nl_compat_dumpit+0x63a/0x820 net/tipc/netlink_compat.c:280
tipc_nl_compat_handle net/tipc/netlink_compat.c:1226 [inline]
tipc_nl_compat_recv+0x1b5f/0x2750 net/tipc/netlink_compat.c:1265
genl_family_rcv_msg net/netlink/genetlink.c:601 [inline]
genl_rcv_msg+0x185f/0x1a60 net/netlink/genetlink.c:626
netlink_rcv_skb+0x431/0x620 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+0xf3e/0x1020 net/netlink/af_netlink.c:1336
netlink_sendmsg+0x127f/0x1300 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:622 [inline]
sock_sendmsg net/socket.c:632 [inline]
Uninit was created at:
__alloc_skb+0x309/0xa20 net/core/skbuff.c:208
alloc_skb include/linux/skbuff.h:1012 [inline]
netlink_alloc_large_skb net/netlink/af_netlink.c:1182 [inline]
netlink_sendmsg+0xb82/0x1300 net/netlink/af_netlink.c:1892
sock_sendmsg_nosec net/socket.c:622 [inline]
sock_sendmsg net/socket.c:632 [inline]
It was supposed to be fixed on commit 974cb0e3e7c9 ("tipc: fix uninit-value
in tipc_nl_compat_name_table_dump") by checking TLV_GET_DATA_LEN(msg->req)
in cmd->header()/tipc_nl_compat_name_table_dump_header(), which is called
ahead of tipc_nl_compat_name_table_dump().
However, tipc_nl_compat_dumpit() doesn't handle the error returned from cmd
header function. It means even when the check added in that fix fails, it
won't stop calling tipc_nl_compat_name_table_dump(), and the issue will be
triggered again.
So this patch is to add the process for the err returned from cmd header
function in tipc_nl_compat_dumpit().
Reported-by: syzbot+3ce8520484b0d4e260a5@syzkaller.appspotmail.com
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/tipc/netlink_compat.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
index 0bfd03d6..340a6e7 100644
--- a/net/tipc/netlink_compat.c
+++ b/net/tipc/netlink_compat.c
@@ -267,8 +267,14 @@ static int tipc_nl_compat_dumpit(struct tipc_nl_compat_cmd_dump *cmd,
if (msg->rep_type)
tipc_tlv_init(msg->rep, msg->rep_type);
- if (cmd->header)
- (*cmd->header)(msg);
+ if (cmd->header) {
+ err = (*cmd->header)(msg);
+ if (err) {
+ kfree_skb(msg->rep);
+ msg->rep = NULL;
+ return err;
+ }
+ }
arg = nlmsg_new(0, GFP_KERNEL);
if (!arg) {
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* RE: [PATCH net 0/3] tipc: a batch of uninit-value fixes for netlink_compat
2019-03-31 14:50 [PATCH net 0/3] tipc: a batch of uninit-value fixes for netlink_compat Xin Long
` (2 preceding siblings ...)
2019-03-31 14:50 ` [PATCH net 3/3] tipc: handle the err returned from cmd header function Xin Long
@ 2019-03-31 22:27 ` Jon Maloy
2019-03-31 23:46 ` David Miller
3 siblings, 1 reply; 6+ messages in thread
From: Jon Maloy @ 2019-03-31 22:27 UTC (permalink / raw)
To: Xin Long, network dev
Cc: davem@davemloft.net, Ying Xue,
tipc-discussion@lists.sourceforge.net, syzkaller@googlegroups.com
All three acked by me.
///jon
> -----Original Message-----
> From: Xin Long <lucien.xin@gmail.com>
> Sent: 31-Mar-19 10:50
> To: network dev <netdev@vger.kernel.org>
> Cc: davem@davemloft.net; Jon Maloy <jon.maloy@ericsson.com>; Ying Xue
> <ying.xue@windriver.com>; tipc-discussion@lists.sourceforge.net;
> syzkaller@googlegroups.com
> Subject: [PATCH net 0/3] tipc: a batch of uninit-value fixes for
> netlink_compat
>
> These issues were all reported by syzbot, and exist since very beginning.
> See the details on each patch.
>
> Xin Long (3):
> tipc: check bearer name with right length in
> tipc_nl_compat_bearer_enable
> tipc: check link name with right length in tipc_nl_compat_link_set
> tipc: handle the err returned from cmd header function
>
> net/tipc/netlink_compat.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> --
> 2.1.0
^ permalink raw reply [flat|nested] 6+ messages in thread