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

[Simon Horman]

On Fri, Dec 12, 2025 at 09:27:23AM +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()")
> Reviewed-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Closes: https://lore.kernel.org/kernel-tls-handshake/aScekpuOYHRM9uOd@xxxxxxxxxxxxxxxxxxxxxxx/T/#m7cfa5c11efc626d77622b2981591197a2acdd65e
> Signed-off-by: Wang Liang <wangliang74@xxxxxxxxxx>
> ---
>  net/handshake/netlink.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Thanks for the update and for addressing my review of v1.

Considering that this function combines __cleanup (via FD_PREPARE) with
goto labels, which IMHO leads to an awkward arrangement of the code, I
think this approach is a good one for a bug fix.

Reviewed-by: Simon Horman <horms@xxxxxxxxxx>