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

[Jon M. <jm...@re...>]

On 7/1/22 02:16, Hoang Le wrote:
> syzbot found the following issue on:
> ==================================================================
> BUG: KMSAN: uninit-value in strlen lib/string.c:495 [inline]
> BUG: KMSAN: uninit-value in strstr+0xb4/0x2e0 lib/string.c:840
>   strlen lib/string.c:495 [inline]
>   strstr+0xb4/0x2e0 lib/string.c:840
>   tipc_nl_node_reset_link_stats+0x41e/0xba0 net/tipc/node.c:2582
>   genl_family_rcv_msg_doit net/netlink/genetlink.c:731 [inline]
>   genl_family_rcv_msg net/netlink/genetlink.c:775 [inline]
>   genl_rcv_msg+0x103f/0x1260 net/netlink/genetlink.c:792
>   netlink_rcv_skb+0x3a5/0x6c0 net/netlink/af_netlink.c:2501
>   genl_rcv+0x3c/0x50 net/netlink/genetlink.c:803
>   netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline]
>   netlink_unicast+0xf3b/0x1270 net/netlink/af_netlink.c:1345
>   netlink_sendmsg+0x1288/0x1440 net/netlink/af_netlink.c:1921
>   sock_sendmsg_nosec net/socket.c:714 [inline]
>   sock_sendmsg net/socket.c:734 [inline]
>   ____sys_sendmsg+0xabc/0xe90 net/socket.c:2492
>   ___sys_sendmsg+0x2a5/0x350 net/socket.c:2546
>   __sys_sendmsg net/socket.c:2575 [inline]
>   __do_sys_sendmsg net/socket.c:2584 [inline]
>   __se_sys_sendmsg net/socket.c:2582 [inline]
>   __x64_sys_sendmsg+0x367/0x540 net/socket.c:2582
>   do_syscall_x64 arch/x86/entry/common.c:50 [inline]
>   do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80
>   entry_SYSCALL_64_after_hwframe+0x46/0xb0
> ==================================================================
>
> This is because link name string is not validated before it's used
> in calling strstr() and strlen().
>
> Reported-by: syz...@sy...
> Signed-off-by: Hoang Le <hoa...@de...>
> ---
>   net/tipc/node.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/net/tipc/node.c b/net/tipc/node.c
> index b48d97cbbe29..23419a599471 100644
> --- a/net/tipc/node.c
> +++ b/net/tipc/node.c
> @@ -2561,6 +2561,7 @@ int tipc_nl_node_reset_link_stats(struct sk_buff *skb, struct genl_info *info)
>   	struct net *net = sock_net(skb->sk);
>   	struct tipc_net *tn = tipc_net(net);
>   	struct tipc_link_entry *le;
> +	int len;
>   
>   	if (!info->attrs[TIPC_NLA_LINK])
>   		return -EINVAL;
> @@ -2574,7 +2575,14 @@ int tipc_nl_node_reset_link_stats(struct sk_buff *skb, struct genl_info *info)
>   	if (!attrs[TIPC_NLA_LINK_NAME])
>   		return -EINVAL;
>   
> +	len = nla_len(attrs[TIPC_NLA_LINK_NAME]);
> +	if (len <= 0)
> +		return -EINVAL;
> +
>   	link_name = nla_data(attrs[TIPC_NLA_LINK_NAME]);
> +	len = min_t(int, len, TIPC_MAX_LINK_NAME);
> +	if (!memchr(link_name, '\0', len))
> +		return -EINVAL;
>   
>   	err = -EINVAL;
>   	if (!strcmp(link_name, tipc_bclink_name)) {
Acked-by: Jon Maloy <jm...@re...>