diff mbox series

[net] tipc: Check the bearer type before calling tipc_udp_nl_bearer_add()

Message ID 20240131152310.4089541-1-syoshida@redhat.com
State New
Headers
Series [net] tipc: Check the bearer type before calling tipc_udp_nl_bearer_add() |

Commit Message

Shigeru Yoshida Jan. 31, 2024, 3:23 p.m. UTC 
  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(+)

Comments

Tung Quang Nguyen Feb. 4, 2024, 3:33 p.m. UTC | #1 
> 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>
Paolo Abeni Feb. 6, 2024, 7:53 a.m. UTC | #2 
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.appspotmailcom
> 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
patchwork-bot+netdevbpf@kernel.org Feb. 6, 2024, 8:50 a.m. UTC | #3 
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!
Shigeru Yoshida Feb. 6, 2024, 3:32 p.m. UTC | #4 
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
>
diff mbox series

Patch

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) {