* [PATCH v2 net] net/handshake: fix null-ptr-deref in handshake_nl_done_doit()
@ 2023-08-31 8:45 Eric Dumazet
2023-08-31 14:20 ` Michal Kubiak
2023-09-01 6:30 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Eric Dumazet @ 2023-08-31 8:45 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: netdev, eric.dumazet, Eric Dumazet, syzbot, Chuck Lever
We should not call trace_handshake_cmd_done_err() if socket lookup has failed.
Also we should call trace_handshake_cmd_done_err() before releasing the file,
otherwise dereferencing sock->sk can return garbage.
This also reverts 7afc6d0a107f ("net/handshake: Fix uninitialized local variable")
Unable to handle kernel paging request at virtual address dfff800000000003
KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f]
Mem abort info:
ESR = 0x0000000096000005
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x05: level 1 translation fault
Data abort info:
ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
CM = 0, WnR = 0, TnD = 0, TagAccess = 0
GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[dfff800000000003] address between user and kernel address ranges
Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
Modules linked in:
CPU: 1 PID: 5986 Comm: syz-executor292 Not tainted 6.5.0-rc7-syzkaller-gfe4469582053 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/26/2023
pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : handshake_nl_done_doit+0x198/0x9c8 net/handshake/netlink.c:193
lr : handshake_nl_done_doit+0x180/0x9c8
sp : ffff800096e37180
x29: ffff800096e37200 x28: 1ffff00012dc6e34 x27: dfff800000000000
x26: ffff800096e373d0 x25: 0000000000000000 x24: 00000000ffffffa8
x23: ffff800096e373f0 x22: 1ffff00012dc6e38 x21: 0000000000000000
x20: ffff800096e371c0 x19: 0000000000000018 x18: 0000000000000000
x17: 0000000000000000 x16: ffff800080516cc4 x15: 0000000000000001
x14: 1fffe0001b14aa3b x13: 0000000000000000 x12: 0000000000000000
x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000003
x8 : 0000000000000003 x7 : ffff800080afe47c x6 : 0000000000000000
x5 : 0000000000000000 x4 : 0000000000000000 x3 : ffff800080a88078
x2 : 0000000000000001 x1 : 00000000ffffffa8 x0 : 0000000000000000
Call trace:
handshake_nl_done_doit+0x198/0x9c8 net/handshake/netlink.c:193
genl_family_rcv_msg_doit net/netlink/genetlink.c:970 [inline]
genl_family_rcv_msg net/netlink/genetlink.c:1050 [inline]
genl_rcv_msg+0x96c/0xc50 net/netlink/genetlink.c:1067
netlink_rcv_skb+0x214/0x3c4 net/netlink/af_netlink.c:2549
genl_rcv+0x38/0x50 net/netlink/genetlink.c:1078
netlink_unicast_kernel net/netlink/af_netlink.c:1339 [inline]
netlink_unicast+0x660/0x8d4 net/netlink/af_netlink.c:1365
netlink_sendmsg+0x834/0xb18 net/netlink/af_netlink.c:1914
sock_sendmsg_nosec net/socket.c:725 [inline]
sock_sendmsg net/socket.c:748 [inline]
____sys_sendmsg+0x56c/0x840 net/socket.c:2494
___sys_sendmsg net/socket.c:2548 [inline]
__sys_sendmsg+0x26c/0x33c net/socket.c:2577
__do_sys_sendmsg net/socket.c:2586 [inline]
__se_sys_sendmsg net/socket.c:2584 [inline]
__arm64_sys_sendmsg+0x80/0x94 net/socket.c:2584
__invoke_syscall arch/arm64/kernel/syscall.c:37 [inline]
invoke_syscall+0x98/0x2b8 arch/arm64/kernel/syscall.c:51
el0_svc_common+0x130/0x23c arch/arm64/kernel/syscall.c:136
do_el0_svc+0x48/0x58 arch/arm64/kernel/syscall.c:155
el0_svc+0x58/0x16c arch/arm64/kernel/entry-common.c:678
el0t_64_sync_handler+0x84/0xfc arch/arm64/kernel/entry-common.c:696
el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:591
Code: 12800108 b90043e8 910062b3 d343fe68 (387b6908)
Fixes: 3b3009ea8abb ("net/handshake: Create a NETLINK service for handling handshake requests")
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Chuck Lever <chuck.lever@oracle.com>
---
v2: remove req=NULL as claimed in changelog (Paolo)
net/handshake/netlink.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/net/handshake/netlink.c b/net/handshake/netlink.c
index 1086653e1fada1724f98ccbc81fbcf7741ef9bc9..d0bc1dd8e65a8201751fddcc2356da89cd2c65e7 100644
--- a/net/handshake/netlink.c
+++ b/net/handshake/netlink.c
@@ -157,26 +157,24 @@ int handshake_nl_accept_doit(struct sk_buff *skb, struct genl_info *info)
int handshake_nl_done_doit(struct sk_buff *skb, struct genl_info *info)
{
struct net *net = sock_net(skb->sk);
- struct handshake_req *req = NULL;
- struct socket *sock = NULL;
+ struct handshake_req *req;
+ struct socket *sock;
int fd, status, err;
if (GENL_REQ_ATTR_CHECK(info, HANDSHAKE_A_DONE_SOCKFD))
return -EINVAL;
fd = nla_get_u32(info->attrs[HANDSHAKE_A_DONE_SOCKFD]);
- err = 0;
sock = sockfd_lookup(fd, &err);
- if (err) {
- err = -EBADF;
- goto out_status;
- }
+ if (!sock)
+ return err;
req = handshake_req_hash_lookup(sock->sk);
if (!req) {
err = -EBUSY;
+ trace_handshake_cmd_done_err(net, req, sock->sk, err);
fput(sock->file);
- goto out_status;
+ return err;
}
trace_handshake_cmd_done(net, req, sock->sk, fd);
@@ -188,10 +186,6 @@ int handshake_nl_done_doit(struct sk_buff *skb, struct genl_info *info)
handshake_complete(req, status, info);
fput(sock->file);
return 0;
-
-out_status:
- trace_handshake_cmd_done_err(net, req, sock->sk, err);
- return err;
}
static unsigned int handshake_net_id;
--
2.42.0.rc2.253.gd59a3bf2b4-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 net] net/handshake: fix null-ptr-deref in handshake_nl_done_doit()
2023-08-31 8:45 [PATCH v2 net] net/handshake: fix null-ptr-deref in handshake_nl_done_doit() Eric Dumazet
@ 2023-08-31 14:20 ` Michal Kubiak
2023-09-01 6:30 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Michal Kubiak @ 2023-08-31 14:20 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, netdev,
eric.dumazet, syzbot, Chuck Lever
On Thu, Aug 31, 2023 at 08:45:09AM +0000, Eric Dumazet wrote:
> We should not call trace_handshake_cmd_done_err() if socket lookup has failed.
>
> Also we should call trace_handshake_cmd_done_err() before releasing the file,
> otherwise dereferencing sock->sk can return garbage.
>
> This also reverts 7afc6d0a107f ("net/handshake: Fix uninitialized local variable")
>
> Unable to handle kernel paging request at virtual address dfff800000000003
> KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f]
> Mem abort info:
> ESR = 0x0000000096000005
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x05: level 1 translation fault
> Data abort info:
> ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
> CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [dfff800000000003] address between user and kernel address ranges
> Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 1 PID: 5986 Comm: syz-executor292 Not tainted 6.5.0-rc7-syzkaller-gfe4469582053 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/26/2023
> pstate: 80400005 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : handshake_nl_done_doit+0x198/0x9c8 net/handshake/netlink.c:193
> lr : handshake_nl_done_doit+0x180/0x9c8
> sp : ffff800096e37180
> x29: ffff800096e37200 x28: 1ffff00012dc6e34 x27: dfff800000000000
> x26: ffff800096e373d0 x25: 0000000000000000 x24: 00000000ffffffa8
> x23: ffff800096e373f0 x22: 1ffff00012dc6e38 x21: 0000000000000000
> x20: ffff800096e371c0 x19: 0000000000000018 x18: 0000000000000000
> x17: 0000000000000000 x16: ffff800080516cc4 x15: 0000000000000001
> x14: 1fffe0001b14aa3b x13: 0000000000000000 x12: 0000000000000000
> x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000003
> x8 : 0000000000000003 x7 : ffff800080afe47c x6 : 0000000000000000
> x5 : 0000000000000000 x4 : 0000000000000000 x3 : ffff800080a88078
> x2 : 0000000000000001 x1 : 00000000ffffffa8 x0 : 0000000000000000
> Call trace:
> handshake_nl_done_doit+0x198/0x9c8 net/handshake/netlink.c:193
> genl_family_rcv_msg_doit net/netlink/genetlink.c:970 [inline]
> genl_family_rcv_msg net/netlink/genetlink.c:1050 [inline]
> genl_rcv_msg+0x96c/0xc50 net/netlink/genetlink.c:1067
> netlink_rcv_skb+0x214/0x3c4 net/netlink/af_netlink.c:2549
> genl_rcv+0x38/0x50 net/netlink/genetlink.c:1078
> netlink_unicast_kernel net/netlink/af_netlink.c:1339 [inline]
> netlink_unicast+0x660/0x8d4 net/netlink/af_netlink.c:1365
> netlink_sendmsg+0x834/0xb18 net/netlink/af_netlink.c:1914
> sock_sendmsg_nosec net/socket.c:725 [inline]
> sock_sendmsg net/socket.c:748 [inline]
> ____sys_sendmsg+0x56c/0x840 net/socket.c:2494
> ___sys_sendmsg net/socket.c:2548 [inline]
> __sys_sendmsg+0x26c/0x33c net/socket.c:2577
> __do_sys_sendmsg net/socket.c:2586 [inline]
> __se_sys_sendmsg net/socket.c:2584 [inline]
> __arm64_sys_sendmsg+0x80/0x94 net/socket.c:2584
> __invoke_syscall arch/arm64/kernel/syscall.c:37 [inline]
> invoke_syscall+0x98/0x2b8 arch/arm64/kernel/syscall.c:51
> el0_svc_common+0x130/0x23c arch/arm64/kernel/syscall.c:136
> do_el0_svc+0x48/0x58 arch/arm64/kernel/syscall.c:155
> el0_svc+0x58/0x16c arch/arm64/kernel/entry-common.c:678
> el0t_64_sync_handler+0x84/0xfc arch/arm64/kernel/entry-common.c:696
> el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:591
> Code: 12800108 b90043e8 910062b3 d343fe68 (387b6908)
>
> Fixes: 3b3009ea8abb ("net/handshake: Create a NETLINK service for handling handshake requests")
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Chuck Lever <chuck.lever@oracle.com>
> ---
> v2: remove req=NULL as claimed in changelog (Paolo)
>
> net/handshake/netlink.c | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/net/handshake/netlink.c b/net/handshake/netlink.c
> index 1086653e1fada1724f98ccbc81fbcf7741ef9bc9..d0bc1dd8e65a8201751fddcc2356da89cd2c65e7 100644
> --- a/net/handshake/netlink.c
> +++ b/net/handshake/netlink.c
> @@ -157,26 +157,24 @@ int handshake_nl_accept_doit(struct sk_buff *skb, struct genl_info *info)
> int handshake_nl_done_doit(struct sk_buff *skb, struct genl_info *info)
> {
> struct net *net = sock_net(skb->sk);
> - struct handshake_req *req = NULL;
> - struct socket *sock = NULL;
> + struct handshake_req *req;
> + struct socket *sock;
> int fd, status, err;
>
> if (GENL_REQ_ATTR_CHECK(info, HANDSHAKE_A_DONE_SOCKFD))
> return -EINVAL;
> fd = nla_get_u32(info->attrs[HANDSHAKE_A_DONE_SOCKFD]);
>
> - err = 0;
> sock = sockfd_lookup(fd, &err);
> - if (err) {
> - err = -EBADF;
> - goto out_status;
> - }
> + if (!sock)
> + return err;
>
> req = handshake_req_hash_lookup(sock->sk);
> if (!req) {
> err = -EBUSY;
> + trace_handshake_cmd_done_err(net, req, sock->sk, err);
> fput(sock->file);
> - goto out_status;
> + return err;
> }
>
> trace_handshake_cmd_done(net, req, sock->sk, fd);
> @@ -188,10 +186,6 @@ int handshake_nl_done_doit(struct sk_buff *skb, struct genl_info *info)
> handshake_complete(req, status, info);
> fput(sock->file);
> return 0;
> -
> -out_status:
> - trace_handshake_cmd_done_err(net, req, sock->sk, err);
> - return err;
> }
>
> static unsigned int handshake_net_id;
> --
> 2.42.0.rc2.253.gd59a3bf2b4-goog
>
>
LGTM
Thanks,
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2 net] net/handshake: fix null-ptr-deref in handshake_nl_done_doit()
2023-08-31 8:45 [PATCH v2 net] net/handshake: fix null-ptr-deref in handshake_nl_done_doit() Eric Dumazet
2023-08-31 14:20 ` Michal Kubiak
@ 2023-09-01 6:30 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-09-01 6:30 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, kuba, pabeni, netdev, eric.dumazet, syzkaller, chuck.lever
Hello:
This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:
On Thu, 31 Aug 2023 08:45:09 +0000 you wrote:
> We should not call trace_handshake_cmd_done_err() if socket lookup has failed.
>
> Also we should call trace_handshake_cmd_done_err() before releasing the file,
> otherwise dereferencing sock->sk can return garbage.
>
> This also reverts 7afc6d0a107f ("net/handshake: Fix uninitialized local variable")
>
> [...]
Here is the summary with links:
- [v2,net] net/handshake: fix null-ptr-deref in handshake_nl_done_doit()
https://git.kernel.org/netdev/net/c/82ba0ff7bf04
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] 3+ messages in thread
end of thread, other threads:[~2023-09-01 6:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-31 8:45 [PATCH v2 net] net/handshake: fix null-ptr-deref in handshake_nl_done_doit() Eric Dumazet
2023-08-31 14:20 ` Michal Kubiak
2023-09-01 6:30 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).