* [PATCH net] tipc: Check the bearer type before calling tipc_udp_nl_bearer_add()
@ 2024-01-31 15:23 Shigeru Yoshida
2024-02-04 15:33 ` Tung Quang Nguyen
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Shigeru Yoshida @ 2024-01-31 15:23 UTC (permalink / raw)
To: jmaloy, ying.xue, davem, edumazet, kuba, pabeni
Cc: netdev, tipc-discussion, linux-kernel, Shigeru Yoshida,
syzbot+5142b87a9abc510e14fa
syzbot reported the following general protection fault [1]:
general protection fault, probably for non-canonical address 0xdffffc0000000010: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000080-0x0000000000000087]
...
RIP: 0010:tipc_udp_is_known_peer+0x9c/0x250 net/tipc/udp_media.c:291
...
Call Trace:
<TASK>
tipc_udp_nl_bearer_add+0x212/0x2f0 net/tipc/udp_media.c:646
tipc_nl_bearer_add+0x21e/0x360 net/tipc/bearer.c:1089
genl_family_rcv_msg_doit+0x1fc/0x2e0 net/netlink/genetlink.c:972
genl_family_rcv_msg net/netlink/genetlink.c:1052 [inline]
genl_rcv_msg+0x561/0x800 net/netlink/genetlink.c:1067
netlink_rcv_skb+0x16b/0x440 net/netlink/af_netlink.c:2544
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1076
netlink_unicast_kernel net/netlink/af_netlink.c:1341 [inline]
netlink_unicast+0x53b/0x810 net/netlink/af_netlink.c:1367
netlink_sendmsg+0x8b7/0xd70 net/netlink/af_netlink.c:1909
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0xd5/0x180 net/socket.c:745
____sys_sendmsg+0x6ac/0x940 net/socket.c:2584
___sys_sendmsg+0x135/0x1d0 net/socket.c:2638
__sys_sendmsg+0x117/0x1e0 net/socket.c:2667
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0x40/0x110 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x63/0x6b
The cause of this issue is that when tipc_nl_bearer_add() is called with
the TIPC_NLA_BEARER_UDP_OPTS attribute, tipc_udp_nl_bearer_add() is called
even if the bearer is not UDP.
tipc_udp_is_known_peer() called by tipc_udp_nl_bearer_add() assumes that
the media_ptr field of the tipc_bearer has an udp_bearer type object, so
the function goes crazy for non-UDP bearers.
This patch fixes the issue by checking the bearer type before calling
tipc_udp_nl_bearer_add() in tipc_nl_bearer_add().
Fixes: ef20cd4dd163 ("tipc: introduce UDP replicast")
Reported-and-tested-by: syzbot+5142b87a9abc510e14fa@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5142b87a9abc510e14fa [1]
Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
---
net/tipc/bearer.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 2cde375477e3..878415c43527 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -1086,6 +1086,12 @@ int tipc_nl_bearer_add(struct sk_buff *skb, struct genl_info *info)
#ifdef CONFIG_TIPC_MEDIA_UDP
if (attrs[TIPC_NLA_BEARER_UDP_OPTS]) {
+ if (b->media->type_id != TIPC_MEDIA_TYPE_UDP) {
+ rtnl_unlock();
+ NL_SET_ERR_MSG(info->extack, "UDP option is unsupported");
+ return -EINVAL;
+ }
+
err = tipc_udp_nl_bearer_add(b,
attrs[TIPC_NLA_BEARER_UDP_OPTS]);
if (err) {
--
2.41.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH net] tipc: Check the bearer type before calling tipc_udp_nl_bearer_add()
2024-01-31 15:23 [PATCH net] tipc: Check the bearer type before calling tipc_udp_nl_bearer_add() Shigeru Yoshida
@ 2024-02-04 15:33 ` Tung Quang Nguyen
2024-02-06 7:53 ` Paolo Abeni
2024-02-06 8:50 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 5+ messages in thread
From: Tung Quang Nguyen @ 2024-02-04 15:33 UTC (permalink / raw)
To: Shigeru Yoshida, jmaloy, ying.xue, davem, edumazet, kuba, pabeni
Cc: netdev, tipc-discussion, linux-kernel, syzbot+5142b87a9abc510e14fa
> net/tipc/bearer.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c index 2cde375477e3..878415c43527 100644
>--- a/net/tipc/bearer.c
>+++ b/net/tipc/bearer.c
>@@ -1086,6 +1086,12 @@ int tipc_nl_bearer_add(struct sk_buff *skb, struct genl_info *info)
>
> #ifdef CONFIG_TIPC_MEDIA_UDP
> if (attrs[TIPC_NLA_BEARER_UDP_OPTS]) {
>+ if (b->media->type_id != TIPC_MEDIA_TYPE_UDP) {
>+ rtnl_unlock();
>+ NL_SET_ERR_MSG(info->extack, "UDP option is unsupported");
>+ return -EINVAL;
>+ }
>+
> err = tipc_udp_nl_bearer_add(b,
> attrs[TIPC_NLA_BEARER_UDP_OPTS]);
> if (err) {
>--
>2.41.0
>
Reviewed-by: Tung Nguyen <tung.q.nguyen@dektech.com.au>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] tipc: Check the bearer type before calling tipc_udp_nl_bearer_add()
2024-01-31 15:23 [PATCH net] tipc: Check the bearer type before calling tipc_udp_nl_bearer_add() Shigeru Yoshida
2024-02-04 15:33 ` Tung Quang Nguyen
@ 2024-02-06 7:53 ` Paolo Abeni
2024-02-06 15:32 ` Shigeru Yoshida
2024-02-06 8:50 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 5+ messages in thread
From: Paolo Abeni @ 2024-02-06 7:53 UTC (permalink / raw)
To: Shigeru Yoshida, jmaloy, ying.xue, davem, edumazet, kuba
Cc: netdev, tipc-discussion, linux-kernel, syzbot+5142b87a9abc510e14fa
On Thu, 2024-02-01 at 00:23 +0900, Shigeru Yoshida wrote:
> syzbot reported the following general protection fault [1]:
>
> general protection fault, probably for non-canonical address 0xdffffc0000000010: 0000 [#1] PREEMPT SMP KASAN
> KASAN: null-ptr-deref in range [0x0000000000000080-0x0000000000000087]
> ...
> RIP: 0010:tipc_udp_is_known_peer+0x9c/0x250 net/tipc/udp_media.c:291
> ...
> Call Trace:
> <TASK>
> tipc_udp_nl_bearer_add+0x212/0x2f0 net/tipc/udp_media.c:646
> tipc_nl_bearer_add+0x21e/0x360 net/tipc/bearer.c:1089
> genl_family_rcv_msg_doit+0x1fc/0x2e0 net/netlink/genetlink.c:972
> genl_family_rcv_msg net/netlink/genetlink.c:1052 [inline]
> genl_rcv_msg+0x561/0x800 net/netlink/genetlink.c:1067
> netlink_rcv_skb+0x16b/0x440 net/netlink/af_netlink.c:2544
> genl_rcv+0x28/0x40 net/netlink/genetlink.c:1076
> netlink_unicast_kernel net/netlink/af_netlink.c:1341 [inline]
> netlink_unicast+0x53b/0x810 net/netlink/af_netlink.c:1367
> netlink_sendmsg+0x8b7/0xd70 net/netlink/af_netlink.c:1909
> sock_sendmsg_nosec net/socket.c:730 [inline]
> __sock_sendmsg+0xd5/0x180 net/socket.c:745
> ____sys_sendmsg+0x6ac/0x940 net/socket.c:2584
> ___sys_sendmsg+0x135/0x1d0 net/socket.c:2638
> __sys_sendmsg+0x117/0x1e0 net/socket.c:2667
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0x40/0x110 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x63/0x6b
>
> The cause of this issue is that when tipc_nl_bearer_add() is called with
> the TIPC_NLA_BEARER_UDP_OPTS attribute, tipc_udp_nl_bearer_add() is called
> even if the bearer is not UDP.
>
> tipc_udp_is_known_peer() called by tipc_udp_nl_bearer_add() assumes that
> the media_ptr field of the tipc_bearer has an udp_bearer type object, so
> the function goes crazy for non-UDP bearers.
>
> This patch fixes the issue by checking the bearer type before calling
> tipc_udp_nl_bearer_add() in tipc_nl_bearer_add().
>
> Fixes: ef20cd4dd163 ("tipc: introduce UDP replicast")
> Reported-and-tested-by: syzbot+5142b87a9abc510e14fa@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=5142b87a9abc510e14fa [1]
> Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
> ---
> net/tipc/bearer.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> index 2cde375477e3..878415c43527 100644
> --- a/net/tipc/bearer.c
> +++ b/net/tipc/bearer.c
> @@ -1086,6 +1086,12 @@ int tipc_nl_bearer_add(struct sk_buff *skb, struct genl_info *info)
>
> #ifdef CONFIG_TIPC_MEDIA_UDP
> if (attrs[TIPC_NLA_BEARER_UDP_OPTS]) {
> + if (b->media->type_id != TIPC_MEDIA_TYPE_UDP) {
> + rtnl_unlock();
> + NL_SET_ERR_MSG(info->extack, "UDP option is unsupported");
> + return -EINVAL;
> + }
> +
The patch LGTM, thanks!
As a side note, this function could probably deserve a net-next follow-
up consolidation the error paths under a common label.
Cheers,
Paolo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] tipc: Check the bearer type before calling tipc_udp_nl_bearer_add()
2024-01-31 15:23 [PATCH net] tipc: Check the bearer type before calling tipc_udp_nl_bearer_add() Shigeru Yoshida
2024-02-04 15:33 ` Tung Quang Nguyen
2024-02-06 7:53 ` Paolo Abeni
@ 2024-02-06 8:50 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-02-06 8:50 UTC (permalink / raw)
To: Shigeru Yoshida
Cc: jmaloy, ying.xue, davem, edumazet, kuba, pabeni, netdev,
tipc-discussion, linux-kernel, syzbot+5142b87a9abc510e14fa
Hello:
This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Thu, 1 Feb 2024 00:23:09 +0900 you wrote:
> syzbot reported the following general protection fault [1]:
>
> general protection fault, probably for non-canonical address 0xdffffc0000000010: 0000 [#1] PREEMPT SMP KASAN
> KASAN: null-ptr-deref in range [0x0000000000000080-0x0000000000000087]
> ...
> RIP: 0010:tipc_udp_is_known_peer+0x9c/0x250 net/tipc/udp_media.c:291
> ...
> Call Trace:
> <TASK>
> tipc_udp_nl_bearer_add+0x212/0x2f0 net/tipc/udp_media.c:646
> tipc_nl_bearer_add+0x21e/0x360 net/tipc/bearer.c:1089
> genl_family_rcv_msg_doit+0x1fc/0x2e0 net/netlink/genetlink.c:972
> genl_family_rcv_msg net/netlink/genetlink.c:1052 [inline]
> genl_rcv_msg+0x561/0x800 net/netlink/genetlink.c:1067
> netlink_rcv_skb+0x16b/0x440 net/netlink/af_netlink.c:2544
> genl_rcv+0x28/0x40 net/netlink/genetlink.c:1076
> netlink_unicast_kernel net/netlink/af_netlink.c:1341 [inline]
> netlink_unicast+0x53b/0x810 net/netlink/af_netlink.c:1367
> netlink_sendmsg+0x8b7/0xd70 net/netlink/af_netlink.c:1909
> sock_sendmsg_nosec net/socket.c:730 [inline]
> __sock_sendmsg+0xd5/0x180 net/socket.c:745
> ____sys_sendmsg+0x6ac/0x940 net/socket.c:2584
> ___sys_sendmsg+0x135/0x1d0 net/socket.c:2638
> __sys_sendmsg+0x117/0x1e0 net/socket.c:2667
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0x40/0x110 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x63/0x6b
>
> [...]
Here is the summary with links:
- [net] tipc: Check the bearer type before calling tipc_udp_nl_bearer_add()
https://git.kernel.org/netdev/net/c/3871aa01e1a7
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] tipc: Check the bearer type before calling tipc_udp_nl_bearer_add()
2024-02-06 7:53 ` Paolo Abeni
@ 2024-02-06 15:32 ` Shigeru Yoshida
0 siblings, 0 replies; 5+ messages in thread
From: Shigeru Yoshida @ 2024-02-06 15:32 UTC (permalink / raw)
To: pabeni
Cc: jmaloy, ying.xue, davem, edumazet, kuba, netdev, tipc-discussion,
linux-kernel, syzbot+5142b87a9abc510e14fa
On Tue, 06 Feb 2024 08:53:14 +0100, Paolo Abeni wrote:
> On Thu, 2024-02-01 at 00:23 +0900, Shigeru Yoshida wrote:
>> syzbot reported the following general protection fault [1]:
>>
>> general protection fault, probably for non-canonical address 0xdffffc0000000010: 0000 [#1] PREEMPT SMP KASAN
>> KASAN: null-ptr-deref in range [0x0000000000000080-0x0000000000000087]
>> ...
>> RIP: 0010:tipc_udp_is_known_peer+0x9c/0x250 net/tipc/udp_media.c:291
>> ...
>> Call Trace:
>> <TASK>
>> tipc_udp_nl_bearer_add+0x212/0x2f0 net/tipc/udp_media.c:646
>> tipc_nl_bearer_add+0x21e/0x360 net/tipc/bearer.c:1089
>> genl_family_rcv_msg_doit+0x1fc/0x2e0 net/netlink/genetlink.c:972
>> genl_family_rcv_msg net/netlink/genetlink.c:1052 [inline]
>> genl_rcv_msg+0x561/0x800 net/netlink/genetlink.c:1067
>> netlink_rcv_skb+0x16b/0x440 net/netlink/af_netlink.c:2544
>> genl_rcv+0x28/0x40 net/netlink/genetlink.c:1076
>> netlink_unicast_kernel net/netlink/af_netlink.c:1341 [inline]
>> netlink_unicast+0x53b/0x810 net/netlink/af_netlink.c:1367
>> netlink_sendmsg+0x8b7/0xd70 net/netlink/af_netlink.c:1909
>> sock_sendmsg_nosec net/socket.c:730 [inline]
>> __sock_sendmsg+0xd5/0x180 net/socket.c:745
>> ____sys_sendmsg+0x6ac/0x940 net/socket.c:2584
>> ___sys_sendmsg+0x135/0x1d0 net/socket.c:2638
>> __sys_sendmsg+0x117/0x1e0 net/socket.c:2667
>> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>> do_syscall_64+0x40/0x110 arch/x86/entry/common.c:83
>> entry_SYSCALL_64_after_hwframe+0x63/0x6b
>>
>> The cause of this issue is that when tipc_nl_bearer_add() is called with
>> the TIPC_NLA_BEARER_UDP_OPTS attribute, tipc_udp_nl_bearer_add() is called
>> even if the bearer is not UDP.
>>
>> tipc_udp_is_known_peer() called by tipc_udp_nl_bearer_add() assumes that
>> the media_ptr field of the tipc_bearer has an udp_bearer type object, so
>> the function goes crazy for non-UDP bearers.
>>
>> This patch fixes the issue by checking the bearer type before calling
>> tipc_udp_nl_bearer_add() in tipc_nl_bearer_add().
>>
>> Fixes: ef20cd4dd163 ("tipc: introduce UDP replicast")
>> Reported-and-tested-by: syzbot+5142b87a9abc510e14fa@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=5142b87a9abc510e14fa [1]
>> Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
>> ---
>> net/tipc/bearer.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
>> index 2cde375477e3..878415c43527 100644
>> --- a/net/tipc/bearer.c
>> +++ b/net/tipc/bearer.c
>> @@ -1086,6 +1086,12 @@ int tipc_nl_bearer_add(struct sk_buff *skb, struct genl_info *info)
>>
>> #ifdef CONFIG_TIPC_MEDIA_UDP
>> if (attrs[TIPC_NLA_BEARER_UDP_OPTS]) {
>> + if (b->media->type_id != TIPC_MEDIA_TYPE_UDP) {
>> + rtnl_unlock();
>> + NL_SET_ERR_MSG(info->extack, "UDP option is unsupported");
>> + return -EINVAL;
>> + }
>> +
>
> The patch LGTM, thanks!
>
> As a side note, this function could probably deserve a net-next follow-
> up consolidation the error paths under a common label.
Thank you for commenting. I'll make a patch to clean up the error paths.
Thanks,
Shigeru
>
> Cheers,
>
> Paolo
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-02-06 15:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-31 15:23 [PATCH net] tipc: Check the bearer type before calling tipc_udp_nl_bearer_add() Shigeru Yoshida
2024-02-04 15:33 ` Tung Quang Nguyen
2024-02-06 7:53 ` Paolo Abeni
2024-02-06 15:32 ` Shigeru Yoshida
2024-02-06 8:50 ` patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.