[PATCH net] net/handshake: Fix null-ptr-deref in handshake_complete()

[PATCH net] net/handshake: Fix null-ptr-deref in handshake_complete()

[Wang Liang]

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@huawei.com>
---
 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:
-- 
2.34.1

Re: [PATCH net] net/handshake: Fix null-ptr-deref in handshake_complete()

[Chuck Lever]

On Tue, Dec 9, 2025, at 6:58 AM, 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@huawei.com>
> ---
>  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:
> -- 
> 2.34.1

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/kernel-tls-handshake/aScekpuOYHRM9uOd@morisot.1015granger.net/T/#m7cfa5c11efc626d77622b2981591197a2acdd65e


-- 
Chuck Lever

Re: [PATCH net] net/handshake: Fix null-ptr-deref in handshake_complete()

[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@huawei.com>
> ---
>  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!

-- 
pw-bot: changes-requested

Re: [PATCH net] net/handshake: Fix null-ptr-deref in handshake_complete()

[kernel test robot]

Hi Wang,

kernel test robot noticed the following build errors:

[auto build test ERROR on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Wang-Liang/net-handshake-Fix-null-ptr-deref-in-handshake_complete/20251209-194006
base:   net/main
patch link:    https://lore.kernel.org/r/20251209115852.3827876-1-wangliang74%40huawei.com
patch subject: [PATCH net] net/handshake: Fix null-ptr-deref in handshake_complete()
config: arm-mps2_defconfig (https://download.01.org/0day-ci/archive/20251210/202512100952.cr9q1lGr-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 6ec8c4351cfc1d0627d1633b02ea787bd29c77d8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251210/202512100952.cr9q1lGr-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512100952.cr9q1lGr-lkp@intel.com/

All errors (new ones prefixed by >>):

>> 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);
         |                    ^
   3 errors generated.


vim +110 net/handshake/netlink.c

    89	
    90	int handshake_nl_accept_doit(struct sk_buff *skb, struct genl_info *info)
    91	{
    92		struct net *net = sock_net(skb->sk);
    93		struct handshake_net *hn = handshake_pernet(net);
    94		struct handshake_req *req = NULL;
    95		struct socket *sock;
    96		int class, err;
    97	
    98		err = -EOPNOTSUPP;
    99		if (!hn)
   100			goto out_status;
   101	
   102		err = -EINVAL;
   103		if (GENL_REQ_ATTR_CHECK(info, HANDSHAKE_A_ACCEPT_HANDLER_CLASS))
   104			goto out_status;
   105		class = nla_get_u32(info->attrs[HANDSHAKE_A_ACCEPT_HANDLER_CLASS]);
   106	
   107		err = -EAGAIN;
   108		req = handshake_req_next(hn, class);
   109		if (!req)
 > 110			goto out_status;
   111	
   112		sock = req->hr_sk->sk_socket;
   113	
   114		FD_PREPARE(fdf, O_CLOEXEC, sock->file);
   115		if (fdf.err) {
   116			err = fdf.err;
   117			goto out_complete;
   118		}
   119	
   120		get_file(sock->file); /* FD_PREPARE() consumes a reference. */
   121		err = req->hr_proto->hp_accept(req, info, fd_prepare_fd(fdf));
   122		if (err)
   123			goto out_complete; /* Automatic cleanup handles fput */
   124	
   125		trace_handshake_cmd_accept(net, req, req->hr_sk, fd_prepare_fd(fdf));
   126		fd_publish(fdf);
   127		return 0;
   128	
   129	out_complete:
   130		handshake_complete(req, -EIO, NULL);
   131	out_status:
   132		trace_handshake_cmd_accept_err(net, req, NULL, err);
   133		return err;
   134	}
   135	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net] net/handshake: Fix null-ptr-deref in handshake_complete()

[Wang Liang]

在 2025/12/9 22:39, Chuck Lever 写道:
>
> On Tue, Dec 9, 2025, at 6:58 AM, 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@huawei.com>
>> ---
>>   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:
>> -- 
>> 2.34.1
> Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/kernel-tls-handshake/aScekpuOYHRM9uOd@morisot.1015granger.net/T/#m7cfa5c11efc626d77622b2981591197a2acdd65e


Thanks for the reminder. I hadn't noticed this email before.

I will add this in v2 patch.

------
Best regards
Wang Liang

>

Re: [PATCH net] net/handshake: Fix null-ptr-deref in handshake_complete()

[Wang Liang]

在 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@huawei.com>
>> ---
>>   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

Re: [PATCH net] net/handshake: Fix null-ptr-deref in handshake_complete()

[Simon Horman]

On Wed, Dec 10, 2025 at 10:51:11AM +0800, Wang Liang wrote:

...

> > 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()?

I assumed the problem lies around the initialisation of fdf
and class_fd_prepare_destructor being called regardless of that
having occurred. But maybe I miss your point.

In any case, I would advocate an approach that left FD_PREPARE in a scope
where it is always called before the scope is exited.

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