Re: [PATCH net] net/handshake: Fix null-ptr-deref in handshake_complete()
From: Wang Liang
Date: Tue Dec 09 2025 - 21:51:26 EST
在 2025/12/10 2:06, Simon Horman 写道:
On Tue, Dec 09, 2025 at 07:58:52PM +0800, Wang Liang wrote:
A null pointer dereference in handshake_complete() was observed [1].
When handshake_req_next() return NULL in handshake_nl_accept_doit(),
function handshake_complete() will be called unexpectedly which triggers
this crash. Fix it by goto out_status when req is NULL.
[1]
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000005: 0000 [#1] SMP KASAN PTI
RIP: 0010:handshake_complete+0x36/0x2b0 net/handshake/request.c:288
Call Trace:
<TASK>
handshake_nl_accept_doit+0x32d/0x7e0 net/handshake/netlink.c:129
genl_family_rcv_msg_doit+0x204/0x300 net/netlink/genetlink.c:1115
genl_family_rcv_msg+0x436/0x670 net/netlink/genetlink.c:1195
genl_rcv_msg+0xcc/0x170 net/netlink/genetlink.c:1210
netlink_rcv_skb+0x14c/0x430 net/netlink/af_netlink.c:2550
genl_rcv+0x2d/0x40 net/netlink/genetlink.c:1219
netlink_unicast_kernel net/netlink/af_netlink.c:1318 [inline]
netlink_unicast+0x878/0xb20 net/netlink/af_netlink.c:1344
netlink_sendmsg+0x897/0xd70 net/netlink/af_netlink.c:1894
sock_sendmsg_nosec net/socket.c:727 [inline]
__sock_sendmsg net/socket.c:742 [inline]
____sys_sendmsg+0xa39/0xbf0 net/socket.c:2592
___sys_sendmsg+0x121/0x1c0 net/socket.c:2646
__sys_sendmsg+0x155/0x200 net/socket.c:2678
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x5f/0x350 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x76/0x7e
</TASK>
Fixes: fe67b063f687 ("net/handshake: convert handshake_nl_accept_doit() to FD_PREPARE()")
Signed-off-by: Wang Liang <wangliang74@xxxxxxxxxx>
---
net/handshake/netlink.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/net/handshake/netlink.c b/net/handshake/netlink.c
index 1d33a4675a48..cdaea8b8d004 100644
--- a/net/handshake/netlink.c
+++ b/net/handshake/netlink.c
@@ -106,25 +106,26 @@ int handshake_nl_accept_doit(struct sk_buff *skb, struct genl_info *info)
err = -EAGAIN;
req = handshake_req_next(hn, class);
- if (req) {
- sock = req->hr_sk->sk_socket;
-
- FD_PREPARE(fdf, O_CLOEXEC, sock->file);
- if (fdf.err) {
- err = fdf.err;
- goto out_complete;
- }
-
- get_file(sock->file); /* FD_PREPARE() consumes a reference. */
- err = req->hr_proto->hp_accept(req, info, fd_prepare_fd(fdf));
- if (err)
- goto out_complete; /* Automatic cleanup handles fput */
-
- trace_handshake_cmd_accept(net, req, req->hr_sk, fd_prepare_fd(fdf));
- fd_publish(fdf);
- return 0;
+ if (!req)
+ goto out_status;
+
+ sock = req->hr_sk->sk_socket;
+
+ FD_PREPARE(fdf, O_CLOEXEC, sock->file);
+ if (fdf.err) {
+ err = fdf.err;
+ goto out_complete;
}
+ get_file(sock->file); /* FD_PREPARE() consumes a reference. */
+ err = req->hr_proto->hp_accept(req, info, fd_prepare_fd(fdf));
+ if (err)
+ goto out_complete; /* Automatic cleanup handles fput */
+
+ trace_handshake_cmd_accept(net, req, req->hr_sk, fd_prepare_fd(fdf));
+ fd_publish(fdf);
+ return 0;
+
out_complete:
handshake_complete(req, -EIO, NULL);
out_status:
Hi,
Clang 21.1.7 W=1 builds are rather unhappy about this:
net/handshake/netlink.c:110:3: error: cannot jump from this goto statement to its label
110 | goto out_status;
| ^
net/handshake/netlink.c:114:13: note: jump bypasses initialization of variable with __attribute__((cleanup))
114 | FD_PREPARE(fdf, O_CLOEXEC, sock->file);
| ^
net/handshake/netlink.c:104:3: error: cannot jump from this goto statement to its label
104 | goto out_status;
| ^
net/handshake/netlink.c:114:13: note: jump bypasses initialization of variable with __attribute__((cleanup))
114 | FD_PREPARE(fdf, O_CLOEXEC, sock->file);
| ^
net/handshake/netlink.c:100:3: error: cannot jump from this goto statement to its label
100 | goto out_status;
| ^
net/handshake/netlink.c:114:13: note: jump bypasses initialization of variable with __attribute__((cleanup))
114 | FD_PREPARE(fdf, O_CLOEXEC, sock->file);
| ^
My undersatnding of the problem is as follows:
FD_PREPARE uses __cleanup to call class_fd_prepare_destructor when
resources when fdf goes out of scope.
Prior to this patch this was when the if (req) block was existed.
Either via return or a goto due to an error.
Now it is when handshake_nl_accept_doit() itself is exited.
Again via a return or a goto due to error.
But, importantly, such a goto can now occur before fdf is initialised.
Boom!
Thanks for your analysis, you are right!
How about adding a null check before calling handshake_complete()?
Like:
diff --git a/net/handshake/netlink.c b/net/handshake/netlink.c
index 1d33a4675a48..b989456fc4c5 100644
--- a/net/handshake/netlink.c
+++ b/net/handshake/netlink.c
@@ -126,7 +126,8 @@ int handshake_nl_accept_doit(struct sk_buff *skb,
struct genl_info *info)
}
out_complete:
- handshake_complete(req, -EIO, NULL);
+ if (req)
+ handshake_complete(req, -EIO, NULL);
out_status:
trace_handshake_cmd_accept_err(net, req, NULL, err);
return err;
------
Best regards
Wang Liang