[PATCH net] net/af_iucv: fix NULL deref in afiucv_hs_callback_syn()

[PATCH net] net/af_iucv: fix NULL deref in afiucv_hs_callback_syn()

[Hidayath Khan]

afiucv_hs_callback_syn() allocates the child socket with GFP_ATOMIC.
If the allocation fails, nsk is NULL.

The connection-refused path is entered when the listen state check
fails, the accept backlog is full, or nsk is NULL. The code
unconditionally calls iucv_sock_kill(nsk) in that path.

iucv_sock_kill() does not accept a NULL socket pointer and immediately
dereferences sk via sock_flag(sk, SOCK_ZAPPED). When nsk is NULL,
calling iucv_sock_kill(nsk) results in a NULL pointer dereference.

Only call iucv_sock_kill() when a child socket was successfully
allocated.

Fixes: 3881ac441f64 ("af_iucv: add HiperSockets transport")
Cc: stable@vger.kernel.org
Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>
---
 net/iucv/af_iucv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index fed240b453bd..f5b1ec44b6ae 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -1872,7 +1872,8 @@ static int afiucv_hs_callback_syn(struct sock *sk, struct sk_buff *skb)
 		afiucv_swap_src_dest(skb);
 		trans_hdr->flags = AF_IUCV_FLAG_SYN | AF_IUCV_FLAG_FIN;
 		err = dev_queue_xmit(skb);
-		iucv_sock_kill(nsk);
+		if (nsk)
+			iucv_sock_kill(nsk);
 		bh_unlock_sock(sk);
 		goto out;
 	}

base-commit: 262b2eac463d880a664cf92af1107b4f9d84ad37
-- 
2.52.0

RE: [PATCH net] net/af_iucv: fix NULL deref in afiucv_hs_callback_syn()

[Jagielski, Jedrzej]

From: Hidayath Khan <hidayath@linux.ibm.com> 
Sent: Thursday, July 9, 2026 9:18 PM

>afiucv_hs_callback_syn() allocates the child socket with GFP_ATOMIC.
>If the allocation fails, nsk is NULL.
>
>The connection-refused path is entered when the listen state check
>fails, the accept backlog is full, or nsk is NULL. The code
>unconditionally calls iucv_sock_kill(nsk) in that path.
>
>iucv_sock_kill() does not accept a NULL socket pointer and immediately
>dereferences sk via sock_flag(sk, SOCK_ZAPPED). When nsk is NULL,
>calling iucv_sock_kill(nsk) results in a NULL pointer dereference.
>
>Only call iucv_sock_kill() when a child socket was successfully
>allocated.
>
>Fixes: 3881ac441f64 ("af_iucv: add HiperSockets transport")
>Cc: stable@vger.kernel.org
>Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
>Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>
>---
> net/iucv/af_iucv.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
>index fed240b453bd..f5b1ec44b6ae 100644
>--- a/net/iucv/af_iucv.c
>+++ b/net/iucv/af_iucv.c
>@@ -1872,7 +1872,8 @@ static int afiucv_hs_callback_syn(struct sock *sk, struct sk_buff *skb)
> 		afiucv_swap_src_dest(skb);
> 		trans_hdr->flags = AF_IUCV_FLAG_SYN | AF_IUCV_FLAG_FIN;
> 		err = dev_queue_xmit(skb);
>-		iucv_sock_kill(nsk);
>+		if (nsk)

Hi Hidayath

why not to move this check into iucv_sock_kill()?
would prevent from potential similar issues in the future

>+			iucv_sock_kill(nsk);
> 		bh_unlock_sock(sk);
> 		goto out;
> 	}
>
>base-commit: 262b2eac463d880a664cf92af1107b4f9d84ad37
>-- 
>2.52.0

Re: [PATCH net] net/af_iucv: fix NULL deref in afiucv_hs_callback_syn()

[Hidayathulla Khan I]

On 10/07/26 3:04 pm, Jagielski, Jedrzej wrote:
> From: Hidayath Khan <hidayath@linux.ibm.com>
> Sent: Thursday, July 9, 2026 9:18 PM
>
>> afiucv_hs_callback_syn() allocates the child socket with GFP_ATOMIC.
>> If the allocation fails, nsk is NULL.
>>
>> The connection-refused path is entered when the listen state check
>> fails, the accept backlog is full, or nsk is NULL. The code
>> unconditionally calls iucv_sock_kill(nsk) in that path.
>>
>> iucv_sock_kill() does not accept a NULL socket pointer and immediately
>> dereferences sk via sock_flag(sk, SOCK_ZAPPED). When nsk is NULL,
>> calling iucv_sock_kill(nsk) results in a NULL pointer dereference.
>>
>> Only call iucv_sock_kill() when a child socket was successfully
>> allocated.
>>
>> Fixes: 3881ac441f64 ("af_iucv: add HiperSockets transport")
>> Cc: stable@vger.kernel.org
>> Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
>> Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>
>> ---
>> net/iucv/af_iucv.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
>> index fed240b453bd..f5b1ec44b6ae 100644
>> --- a/net/iucv/af_iucv.c
>> +++ b/net/iucv/af_iucv.c
>> @@ -1872,7 +1872,8 @@ static int afiucv_hs_callback_syn(struct sock *sk, struct sk_buff *skb)
>> 		afiucv_swap_src_dest(skb);
>> 		trans_hdr->flags = AF_IUCV_FLAG_SYN | AF_IUCV_FLAG_FIN;
>> 		err = dev_queue_xmit(skb);
>> -		iucv_sock_kill(nsk);
>> +		if (nsk)
> Hi Hidayath
>
> why not to move this check into iucv_sock_kill()?
> would prevent from potential similar issues in the future
Hi Jedrzej,

Every other call to iucv_sock_kill() passes a non-NULL socket by 
construction.

If iucv_sock_kill() silently accepted NULL, a future caller wrongly
passing NULL would go unnoticed instead of being caught.

>
>> +			iucv_sock_kill(nsk);
>> 		bh_unlock_sock(sk);
>> 		goto out;
>> 	}
>>
>> base-commit: 262b2eac463d880a664cf92af1107b4f9d84ad37
>> -- 
>> 2.52.0
>
>

Re: [PATCH net] net/af_iucv: fix NULL deref in afiucv_hs_callback_syn()

[Hidayathulla Khan I]

Thanks for the Sashiko AI review: On the findings it raised.

Finding 1: iucv_sock_kill() returns early unless SOCK_ZAPPED is set,
and the flag is never set on a freshly allocated child socket, so the
child socket and its pinned net_device leak on the error paths. I had 
already
spotted this leak (in afiucv_hs_callback_syn() and iucv_callback_connreq())
and Alexandra Winter and I are looking into it.

Both NULL deref and child sock leak come from the same root cause,
the child socket is allocated before the listen-state and accept-queue 
checks.
I will address them together in v2 by allocating the child socket only 
after the
listen-state and accept-queue checks, so the refused path has nothing to
release (no NULL to guard and no child socket to free).

And on the transmit-failure path release the already-constructed
child socket directly (dev_put, unlink, put the last reference) instead of
relying on iucv_sock_kill().

Finding 2: missing sock_hold on the afiucv_hs_rcv() lookup. Agreed.
Bryam Vargas has already submitted a patch for this.

The other findings look valid too. I will follow up on them separately.

Thanks,
Hidayath Khan

On 10/07/26 12:47 am, Hidayath Khan wrote:
> afiucv_hs_callback_syn() allocates the child socket with GFP_ATOMIC.
> If the allocation fails, nsk is NULL.
>
> The connection-refused path is entered when the listen state check
> fails, the accept backlog is full, or nsk is NULL. The code
> unconditionally calls iucv_sock_kill(nsk) in that path.
>
> iucv_sock_kill() does not accept a NULL socket pointer and immediately
> dereferences sk via sock_flag(sk, SOCK_ZAPPED). When nsk is NULL,
> calling iucv_sock_kill(nsk) results in a NULL pointer dereference.
>
> Only call iucv_sock_kill() when a child socket was successfully
> allocated.
>
> Fixes: 3881ac441f64 ("af_iucv: add HiperSockets transport")
> Cc: stable@vger.kernel.org
> Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
> Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>
> ---
>   net/iucv/af_iucv.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
> index fed240b453bd..f5b1ec44b6ae 100644
> --- a/net/iucv/af_iucv.c
> +++ b/net/iucv/af_iucv.c
> @@ -1872,7 +1872,8 @@ static int afiucv_hs_callback_syn(struct sock *sk, struct sk_buff *skb)
>   		afiucv_swap_src_dest(skb);
>   		trans_hdr->flags = AF_IUCV_FLAG_SYN | AF_IUCV_FLAG_FIN;
>   		err = dev_queue_xmit(skb);
> -		iucv_sock_kill(nsk);
> +		if (nsk)
> +			iucv_sock_kill(nsk);
>   		bh_unlock_sock(sk);
>   		goto out;
>   	}
>
> base-commit: 262b2eac463d880a664cf92af1107b4f9d84ad37

Re: [PATCH net] net/af_iucv: fix NULL deref in afiucv_hs_callback_syn()

[Alexandra Winter]

On 09.07.26 21:17, Hidayath Khan wrote:
> afiucv_hs_callback_syn() allocates the child socket with GFP_ATOMIC.
> If the allocation fails, nsk is NULL.
> 
> The connection-refused path is entered when the listen state check
> fails, the accept backlog is full, or nsk is NULL. The code
> unconditionally calls iucv_sock_kill(nsk) in that path.
> 
> iucv_sock_kill() does not accept a NULL socket pointer and immediately
> dereferences sk via sock_flag(sk, SOCK_ZAPPED). When nsk is NULL,
> calling iucv_sock_kill(nsk) results in a NULL pointer dereference.
> 
> Only call iucv_sock_kill() when a child socket was successfully
> allocated.
> 
> Fixes: 3881ac441f64 ("af_iucv: add HiperSockets transport")
> Cc: stable@vger.kernel.org
> Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
> Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>
> ---
>  net/iucv/af_iucv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
> index fed240b453bd..f5b1ec44b6ae 100644
> --- a/net/iucv/af_iucv.c
> +++ b/net/iucv/af_iucv.c
> @@ -1872,7 +1872,8 @@ static int afiucv_hs_callback_syn(struct sock *sk, struct sk_buff *skb)
>  		afiucv_swap_src_dest(skb);
>  		trans_hdr->flags = AF_IUCV_FLAG_SYN | AF_IUCV_FLAG_FIN;
>  		err = dev_queue_xmit(skb);
> -		iucv_sock_kill(nsk);
> +		if (nsk)
> +			iucv_sock_kill(nsk);
>  		bh_unlock_sock(sk);
>  		goto out;
>  	}
> 
> base-commit: 262b2eac463d880a664cf92af1107b4f9d84ad37


Gentle ping to netdev maintainers:
Did this one get lost in the overflow?
It is all green in patchwork. Is there something you need us to do?
Should we re-send it?
I don't see this as urgent or especially dangerous.

Kind regards
Alexandra

Re: [PATCH net] net/af_iucv: fix NULL deref in afiucv_hs_callback_syn()

[Paolo Abeni]

On 7/21/26 3:54 PM, Alexandra Winter wrote:
> On 09.07.26 21:17, Hidayath Khan wrote:
>> afiucv_hs_callback_syn() allocates the child socket with GFP_ATOMIC.
>> If the allocation fails, nsk is NULL.
>>
>> The connection-refused path is entered when the listen state check
>> fails, the accept backlog is full, or nsk is NULL. The code
>> unconditionally calls iucv_sock_kill(nsk) in that path.
>>
>> iucv_sock_kill() does not accept a NULL socket pointer and immediately
>> dereferences sk via sock_flag(sk, SOCK_ZAPPED). When nsk is NULL,
>> calling iucv_sock_kill(nsk) results in a NULL pointer dereference.
>>
>> Only call iucv_sock_kill() when a child socket was successfully
>> allocated.
>>
>> Fixes: 3881ac441f64 ("af_iucv: add HiperSockets transport")
>> Cc: stable@vger.kernel.org
>> Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
>> Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>
>> ---
>>  net/iucv/af_iucv.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
>> index fed240b453bd..f5b1ec44b6ae 100644
>> --- a/net/iucv/af_iucv.c
>> +++ b/net/iucv/af_iucv.c
>> @@ -1872,7 +1872,8 @@ static int afiucv_hs_callback_syn(struct sock *sk, struct sk_buff *skb)
>>  		afiucv_swap_src_dest(skb);
>>  		trans_hdr->flags = AF_IUCV_FLAG_SYN | AF_IUCV_FLAG_FIN;
>>  		err = dev_queue_xmit(skb);
>> -		iucv_sock_kill(nsk);
>> +		if (nsk)
>> +			iucv_sock_kill(nsk);
>>  		bh_unlock_sock(sk);
>>  		goto out;
>>  	}
>>
>> base-commit: 262b2eac463d880a664cf92af1107b4f9d84ad37
> 
> 
> Gentle ping to netdev maintainers:
> Did this one get lost in the overflow?
> It is all green in patchwork. Is there something you need us to do?
> Should we re-send it?
> I don't see this as urgent or especially dangerous.
It's still alive in PW. Our backlog is unusually huge due to an
unfortunate sequence of season holidays and conferences, but hopefully
it should get back to normality someday in the future :)

/P

Re: [PATCH net] net/af_iucv: fix NULL deref in afiucv_hs_callback_syn()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu,  9 Jul 2026 21:17:32 +0200 you wrote:
> afiucv_hs_callback_syn() allocates the child socket with GFP_ATOMIC.
> If the allocation fails, nsk is NULL.
> 
> The connection-refused path is entered when the listen state check
> fails, the accept backlog is full, or nsk is NULL. The code
> unconditionally calls iucv_sock_kill(nsk) in that path.
> 
> [...]

Here is the summary with links:
  - [net] net/af_iucv: fix NULL deref in afiucv_hs_callback_syn()
    https://git.kernel.org/netdev/net/c/47a5116e56a6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html