| Message ID | 20240131152310.4089541-1-syoshida@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3871aa01e1a779d866fa9dfdd5a836f342f4eb87 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] tipc: Check the bearer type before calling tipc_udp_nl_bearer_add() | expand |
> 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>
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
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!
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 --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) {
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(+)