2023-10-30 07:56:57

by Shigeru Yoshida

[permalink] [raw]
Subject: [PATCH net v3] tipc: Change nla_policy for bearer-related names to NLA_NUL_STRING

syzbot reported the following uninit-value access issue [1]:

=====================================================
BUG: KMSAN: uninit-value in strlen lib/string.c:418 [inline]
BUG: KMSAN: uninit-value in strstr+0xb8/0x2f0 lib/string.c:756
strlen lib/string.c:418 [inline]
strstr+0xb8/0x2f0 lib/string.c:756
tipc_nl_node_reset_link_stats+0x3ea/0xb50 net/tipc/node.c:2595
genl_family_rcv_msg_doit net/netlink/genetlink.c:971 [inline]
genl_family_rcv_msg net/netlink/genetlink.c:1051 [inline]
genl_rcv_msg+0x11ec/0x1290 net/netlink/genetlink.c:1066
netlink_rcv_skb+0x371/0x650 net/netlink/af_netlink.c:2545
genl_rcv+0x40/0x60 net/netlink/genetlink.c:1075
netlink_unicast_kernel net/netlink/af_netlink.c:1342 [inline]
netlink_unicast+0xf47/0x1250 net/netlink/af_netlink.c:1368
netlink_sendmsg+0x1238/0x13d0 net/netlink/af_netlink.c:1910
sock_sendmsg_nosec net/socket.c:730 [inline]
sock_sendmsg net/socket.c:753 [inline]
____sys_sendmsg+0x9c2/0xd60 net/socket.c:2541
___sys_sendmsg+0x28d/0x3c0 net/socket.c:2595
__sys_sendmsg net/socket.c:2624 [inline]
__do_sys_sendmsg net/socket.c:2633 [inline]
__se_sys_sendmsg net/socket.c:2631 [inline]
__x64_sys_sendmsg+0x307/0x490 net/socket.c:2631
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd

Uninit was created at:
slab_post_alloc_hook+0x12f/0xb70 mm/slab.h:767
slab_alloc_node mm/slub.c:3478 [inline]
kmem_cache_alloc_node+0x577/0xa80 mm/slub.c:3523
kmalloc_reserve+0x13d/0x4a0 net/core/skbuff.c:559
__alloc_skb+0x318/0x740 net/core/skbuff.c:650
alloc_skb include/linux/skbuff.h:1286 [inline]
netlink_alloc_large_skb net/netlink/af_netlink.c:1214 [inline]
netlink_sendmsg+0xb34/0x13d0 net/netlink/af_netlink.c:1885
sock_sendmsg_nosec net/socket.c:730 [inline]
sock_sendmsg net/socket.c:753 [inline]
____sys_sendmsg+0x9c2/0xd60 net/socket.c:2541
___sys_sendmsg+0x28d/0x3c0 net/socket.c:2595
__sys_sendmsg net/socket.c:2624 [inline]
__do_sys_sendmsg net/socket.c:2633 [inline]
__se_sys_sendmsg net/socket.c:2631 [inline]
__x64_sys_sendmsg+0x307/0x490 net/socket.c:2631
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd

TIPC bearer-related names including link names must be null-terminated
strings. If a link name which is not null-terminated is passed through
netlink, strstr() and similar functions can cause buffer overrun. This
causes the above issue.

This patch changes the nla_policy for bearer-related names from NLA_STRING
to NLA_NUL_STRING. This resolves the issue by ensuring that only
null-terminated strings are accepted as bearer-related names.

syzbot reported similar uninit-value issue related to bearer names [2]. The
root cause of this issue is that a non-null-terminated bearer name was
passed. This patch also resolved this issue.

Fixes: 7be57fc69184 ("tipc: add link get/dump to new netlink api")
Fixes: 0655f6a8635b ("tipc: add bearer disable/enable to new netlink api")
Reported-and-tested-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=5138ca807af9d2b42574 [1]
Reported-and-tested-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=9425c47dccbcb4c17d51 [2]
Signed-off-by: Shigeru Yoshida <[email protected]>
---
v2->v3:
- Change the title
- Change the nla_policy for bearer-related names instead of using nla_strscpy()
- Resolve bearer-name related issue too
https://lore.kernel.org/all/[email protected]/

v1->v2:
- Use nla_strscpy()
- Fix similar bugs in other functions other than syzbot reported
https://lore.kernel.org/all/[email protected]/
---
net/tipc/netlink.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/tipc/netlink.c b/net/tipc/netlink.c
index e8fd257c0e68..1a9a5bdaccf4 100644
--- a/net/tipc/netlink.c
+++ b/net/tipc/netlink.c
@@ -88,7 +88,7 @@ const struct nla_policy tipc_nl_net_policy[TIPC_NLA_NET_MAX + 1] = {

const struct nla_policy tipc_nl_link_policy[TIPC_NLA_LINK_MAX + 1] = {
[TIPC_NLA_LINK_UNSPEC] = { .type = NLA_UNSPEC },
- [TIPC_NLA_LINK_NAME] = { .type = NLA_STRING,
+ [TIPC_NLA_LINK_NAME] = { .type = NLA_NUL_STRING,
.len = TIPC_MAX_LINK_NAME },
[TIPC_NLA_LINK_MTU] = { .type = NLA_U32 },
[TIPC_NLA_LINK_BROADCAST] = { .type = NLA_FLAG },
@@ -125,7 +125,7 @@ const struct nla_policy tipc_nl_prop_policy[TIPC_NLA_PROP_MAX + 1] = {

const struct nla_policy tipc_nl_bearer_policy[TIPC_NLA_BEARER_MAX + 1] = {
[TIPC_NLA_BEARER_UNSPEC] = { .type = NLA_UNSPEC },
- [TIPC_NLA_BEARER_NAME] = { .type = NLA_STRING,
+ [TIPC_NLA_BEARER_NAME] = { .type = NLA_NUL_STRING,
.len = TIPC_MAX_BEARER_NAME },
[TIPC_NLA_BEARER_PROP] = { .type = NLA_NESTED },
[TIPC_NLA_BEARER_DOMAIN] = { .type = NLA_U32 }
--
2.41.0


2023-10-30 12:01:24

by Jiri Pirko

[permalink] [raw]
Subject: Re: [PATCH net v3] tipc: Change nla_policy for bearer-related names to NLA_NUL_STRING

Mon, Oct 30, 2023 at 08:55:40AM CET, [email protected] wrote:
>syzbot reported the following uninit-value access issue [1]:
>
>=====================================================
>BUG: KMSAN: uninit-value in strlen lib/string.c:418 [inline]
>BUG: KMSAN: uninit-value in strstr+0xb8/0x2f0 lib/string.c:756
> strlen lib/string.c:418 [inline]
> strstr+0xb8/0x2f0 lib/string.c:756
> tipc_nl_node_reset_link_stats+0x3ea/0xb50 net/tipc/node.c:2595
> genl_family_rcv_msg_doit net/netlink/genetlink.c:971 [inline]
> genl_family_rcv_msg net/netlink/genetlink.c:1051 [inline]
> genl_rcv_msg+0x11ec/0x1290 net/netlink/genetlink.c:1066
> netlink_rcv_skb+0x371/0x650 net/netlink/af_netlink.c:2545
> genl_rcv+0x40/0x60 net/netlink/genetlink.c:1075
> netlink_unicast_kernel net/netlink/af_netlink.c:1342 [inline]
> netlink_unicast+0xf47/0x1250 net/netlink/af_netlink.c:1368
> netlink_sendmsg+0x1238/0x13d0 net/netlink/af_netlink.c:1910
> sock_sendmsg_nosec net/socket.c:730 [inline]
> sock_sendmsg net/socket.c:753 [inline]
> ____sys_sendmsg+0x9c2/0xd60 net/socket.c:2541
> ___sys_sendmsg+0x28d/0x3c0 net/socket.c:2595
> __sys_sendmsg net/socket.c:2624 [inline]
> __do_sys_sendmsg net/socket.c:2633 [inline]
> __se_sys_sendmsg net/socket.c:2631 [inline]
> __x64_sys_sendmsg+0x307/0x490 net/socket.c:2631
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
>Uninit was created at:
> slab_post_alloc_hook+0x12f/0xb70 mm/slab.h:767
> slab_alloc_node mm/slub.c:3478 [inline]
> kmem_cache_alloc_node+0x577/0xa80 mm/slub.c:3523
> kmalloc_reserve+0x13d/0x4a0 net/core/skbuff.c:559
> __alloc_skb+0x318/0x740 net/core/skbuff.c:650
> alloc_skb include/linux/skbuff.h:1286 [inline]
> netlink_alloc_large_skb net/netlink/af_netlink.c:1214 [inline]
> netlink_sendmsg+0xb34/0x13d0 net/netlink/af_netlink.c:1885
> sock_sendmsg_nosec net/socket.c:730 [inline]
> sock_sendmsg net/socket.c:753 [inline]
> ____sys_sendmsg+0x9c2/0xd60 net/socket.c:2541
> ___sys_sendmsg+0x28d/0x3c0 net/socket.c:2595
> __sys_sendmsg net/socket.c:2624 [inline]
> __do_sys_sendmsg net/socket.c:2633 [inline]
> __se_sys_sendmsg net/socket.c:2631 [inline]
> __x64_sys_sendmsg+0x307/0x490 net/socket.c:2631
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
>TIPC bearer-related names including link names must be null-terminated
>strings. If a link name which is not null-terminated is passed through
>netlink, strstr() and similar functions can cause buffer overrun. This
>causes the above issue.
>
>This patch changes the nla_policy for bearer-related names from NLA_STRING
>to NLA_NUL_STRING. This resolves the issue by ensuring that only
>null-terminated strings are accepted as bearer-related names.
>
>syzbot reported similar uninit-value issue related to bearer names [2]. The
>root cause of this issue is that a non-null-terminated bearer name was
>passed. This patch also resolved this issue.
>
>Fixes: 7be57fc69184 ("tipc: add link get/dump to new netlink api")
>Fixes: 0655f6a8635b ("tipc: add bearer disable/enable to new netlink api")
>Reported-and-tested-by: [email protected]
>Closes: https://syzkaller.appspot.com/bug?extid=5138ca807af9d2b42574 [1]
>Reported-and-tested-by: [email protected]
>Closes: https://syzkaller.appspot.com/bug?extid=9425c47dccbcb4c17d51 [2]
>Signed-off-by: Shigeru Yoshida <[email protected]>

Reviewed-by: Jiri Pirko <[email protected]>

2023-11-02 05:51:42

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net v3] tipc: Change nla_policy for bearer-related names to NLA_NUL_STRING

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

On Mon, 30 Oct 2023 16:55:40 +0900 you wrote:
> syzbot reported the following uninit-value access issue [1]:
>
> =====================================================
> BUG: KMSAN: uninit-value in strlen lib/string.c:418 [inline]
> BUG: KMSAN: uninit-value in strstr+0xb8/0x2f0 lib/string.c:756
> strlen lib/string.c:418 [inline]
> strstr+0xb8/0x2f0 lib/string.c:756
> tipc_nl_node_reset_link_stats+0x3ea/0xb50 net/tipc/node.c:2595
> genl_family_rcv_msg_doit net/netlink/genetlink.c:971 [inline]
> genl_family_rcv_msg net/netlink/genetlink.c:1051 [inline]
> genl_rcv_msg+0x11ec/0x1290 net/netlink/genetlink.c:1066
> netlink_rcv_skb+0x371/0x650 net/netlink/af_netlink.c:2545
> genl_rcv+0x40/0x60 net/netlink/genetlink.c:1075
> netlink_unicast_kernel net/netlink/af_netlink.c:1342 [inline]
> netlink_unicast+0xf47/0x1250 net/netlink/af_netlink.c:1368
> netlink_sendmsg+0x1238/0x13d0 net/netlink/af_netlink.c:1910
> sock_sendmsg_nosec net/socket.c:730 [inline]
> sock_sendmsg net/socket.c:753 [inline]
> ____sys_sendmsg+0x9c2/0xd60 net/socket.c:2541
> ___sys_sendmsg+0x28d/0x3c0 net/socket.c:2595
> __sys_sendmsg net/socket.c:2624 [inline]
> __do_sys_sendmsg net/socket.c:2633 [inline]
> __se_sys_sendmsg net/socket.c:2631 [inline]
> __x64_sys_sendmsg+0x307/0x490 net/socket.c:2631
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> [...]

Here is the summary with links:
- [net,v3] tipc: Change nla_policy for bearer-related names to NLA_NUL_STRING
https://git.kernel.org/netdev/net/c/19b3f72a41a8

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html