[PATCH net] net/iucv: take a reference on the socket found in afiucv_hs_rcv()

[PATCH net] net/iucv: take a reference on the socket found in afiucv_hs_rcv()

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

afiucv_hs_rcv() looks up the destination socket under iucv_sk_list.lock,
drops the lock, and then passes the socket to the afiucv_hs_callback_*()
handlers without holding a reference. AF_IUCV sockets are not
RCU-protected and are freed synchronously by iucv_sock_kill() ->
sock_put(), so a concurrent close can free the socket in the window
between read_unlock() and the handler, which then dereferences freed
memory (for example sk->sk_data_ready() in afiucv_hs_callback_syn()).

Take a reference with sock_hold() while the socket is still on the list
and release it with sock_put() once the handler has run.

Fixes: 3881ac441f64 ("af_iucv: add HiperSockets transport")
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
afiucv_netdev_event() has the same iucv_sk_list use-after-free and is
being fixed separately by Nagamani PV:
  https://lore.kernel.org/all/20260508170534.2208812-1-nagamani@linux.ibm.com/
This patch covers the receive path, afiucv_hs_rcv(), which that fix does
not touch.

Verified with an LKMM/herd7 litmus (the missing sock_hold() is the only
delta between a flagged data race and none) and with an in-kernel KASAN
model that reproduces the afiucv_hs_rcv() lookup-without-hold path (the
freed socket's sk_data_ready pointer is read after the callback runs);
adding the reference clears it. af_iucv is s390-only, so this is the
model rather than the driver. Reproducer available on request.
---
 net/iucv/af_iucv.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index fed240b453bd..b85fb9767dec 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -2089,6 +2089,8 @@ static int afiucv_hs_rcv(struct sk_buff *skb, struct net_device *dev,
 			}
 		}
 	}
+	if (sk)
+		sock_hold(sk);
 	read_unlock(&iucv_sk_list.lock);
 	if (!iucv)
 		sk = NULL;
@@ -2138,6 +2140,8 @@ static int afiucv_hs_rcv(struct sk_buff *skb, struct net_device *dev,
 		kfree_skb(skb);
 	}
 
+	if (sk)
+		sock_put(sk);
 	return err;
 }
 

---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260705-b4-disp-fc79c0dc-019670262472

Best regards,
-- 
Bryam Vargas <hexlabsecurity@proton.me>

Re: [PATCH net] net/iucv: take a reference on the socket found in afiucv_hs_rcv()

[Hidayathulla Khan I]

Hi Bryam,

Addressing Sashiko's findings on this patch: [High] NULL Pointer Dereference in `afiucv_hs_callback_syn` error path.

This is already fixed by my patch currently under review.

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

The fix guards iucv_sock_kill(nsk) with if (nsk).

Regards,
Hidayath Khan

On 06/07/26 8:54 am, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <hexlabsecurity@proton.me>
>
> afiucv_hs_rcv() looks up the destination socket under iucv_sk_list.lock,
> drops the lock, and then passes the socket to the afiucv_hs_callback_*()
> handlers without holding a reference. AF_IUCV sockets are not
> RCU-protected and are freed synchronously by iucv_sock_kill() ->
> sock_put(), so a concurrent close can free the socket in the window
> between read_unlock() and the handler, which then dereferences freed
> memory (for example sk->sk_data_ready() in afiucv_hs_callback_syn()).
>
> Take a reference with sock_hold() while the socket is still on the list
> and release it with sock_put() once the handler has run.
>
> Fixes: 3881ac441f64 ("af_iucv: add HiperSockets transport")
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
> afiucv_netdev_event() has the same iucv_sk_list use-after-free and is
> being fixed separately by Nagamani PV:
>    https://lore.kernel.org/all/20260508170534.2208812-1-nagamani@linux.ibm.com/
> This patch covers the receive path, afiucv_hs_rcv(), which that fix does
> not touch.
>
> Verified with an LKMM/herd7 litmus (the missing sock_hold() is the only
> delta between a flagged data race and none) and with an in-kernel KASAN
> model that reproduces the afiucv_hs_rcv() lookup-without-hold path (the
> freed socket's sk_data_ready pointer is read after the callback runs);
> adding the reference clears it. af_iucv is s390-only, so this is the
> model rather than the driver. Reproducer available on request.
> ---
>   net/iucv/af_iucv.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
> index fed240b453bd..b85fb9767dec 100644
> --- a/net/iucv/af_iucv.c
> +++ b/net/iucv/af_iucv.c
> @@ -2089,6 +2089,8 @@ static int afiucv_hs_rcv(struct sk_buff *skb, struct net_device *dev,
>   			}
>   		}
>   	}
> +	if (sk)
> +		sock_hold(sk);
>   	read_unlock(&iucv_sk_list.lock);
>   	if (!iucv)
>   		sk = NULL;
> @@ -2138,6 +2140,8 @@ static int afiucv_hs_rcv(struct sk_buff *skb, struct net_device *dev,
>   		kfree_skb(skb);
>   	}
>   
> +	if (sk)
> +		sock_put(sk);
>   	return err;
>   }
>   
>
> ---
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
> change-id: 20260705-b4-disp-fc79c0dc-019670262472
>
> Best regards,

Re: [PATCH net] net/iucv: take a reference on the socket found in afiucv_hs_rcv()

[Hidayathulla Khan I]

Hi Bryam,

Patch looks correct. The fix properly pins the socket found in 
afiucv_hs_rcv()
before dropping iucv_sk_list.lock, preventing a concurrent close from 
freeing
it before the handlers run.

The sock_hold() and sock_put() placement and guards are correct.

Reviewed-by: Hidayath Khan <hidayath@linux.ibm.com>

On 06/07/26 8:54 am, Bryam Vargas via B4 Relay wrote:
> From: Bryam Vargas <hexlabsecurity@proton.me>
>
> afiucv_hs_rcv() looks up the destination socket under iucv_sk_list.lock,
> drops the lock, and then passes the socket to the afiucv_hs_callback_*()
> handlers without holding a reference. AF_IUCV sockets are not
> RCU-protected and are freed synchronously by iucv_sock_kill() ->
> sock_put(), so a concurrent close can free the socket in the window
> between read_unlock() and the handler, which then dereferences freed
> memory (for example sk->sk_data_ready() in afiucv_hs_callback_syn()).
>
> Take a reference with sock_hold() while the socket is still on the list
> and release it with sock_put() once the handler has run.
>
> Fixes: 3881ac441f64 ("af_iucv: add HiperSockets transport")
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
> afiucv_netdev_event() has the same iucv_sk_list use-after-free and is
> being fixed separately by Nagamani PV:
>    https://lore.kernel.org/all/20260508170534.2208812-1-nagamani@linux.ibm.com/
> This patch covers the receive path, afiucv_hs_rcv(), which that fix does
> not touch.
>
> Verified with an LKMM/herd7 litmus (the missing sock_hold() is the only
> delta between a flagged data race and none) and with an in-kernel KASAN
> model that reproduces the afiucv_hs_rcv() lookup-without-hold path (the
> freed socket's sk_data_ready pointer is read after the callback runs);
> adding the reference clears it. af_iucv is s390-only, so this is the
> model rather than the driver. Reproducer available on request.
> ---
>   net/iucv/af_iucv.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
> index fed240b453bd..b85fb9767dec 100644
> --- a/net/iucv/af_iucv.c
> +++ b/net/iucv/af_iucv.c
> @@ -2089,6 +2089,8 @@ static int afiucv_hs_rcv(struct sk_buff *skb, struct net_device *dev,
>   			}
>   		}
>   	}
> +	if (sk)
> +		sock_hold(sk);
>   	read_unlock(&iucv_sk_list.lock);
>   	if (!iucv)
>   		sk = NULL;
> @@ -2138,6 +2140,8 @@ static int afiucv_hs_rcv(struct sk_buff *skb, struct net_device *dev,
>   		kfree_skb(skb);
>   	}
>   
> +	if (sk)
> +		sock_put(sk);
>   	return err;
>   }
>   
>
> ---
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
> change-id: 20260705-b4-disp-fc79c0dc-019670262472
>
> Best regards,

Re: [PATCH net] net/iucv: take a reference on the socket found in afiucv_hs_rcv()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Sun, 05 Jul 2026 22:24:36 -0500 you wrote:
> From: Bryam Vargas <hexlabsecurity@proton.me>
> 
> afiucv_hs_rcv() looks up the destination socket under iucv_sk_list.lock,
> drops the lock, and then passes the socket to the afiucv_hs_callback_*()
> handlers without holding a reference. AF_IUCV sockets are not
> RCU-protected and are freed synchronously by iucv_sock_kill() ->
> sock_put(), so a concurrent close can free the socket in the window
> between read_unlock() and the handler, which then dereferences freed
> memory (for example sk->sk_data_ready() in afiucv_hs_callback_syn()).
> 
> [...]

Here is the summary with links:
  - [net] net/iucv: take a reference on the socket found in afiucv_hs_rcv()
    https://git.kernel.org/netdev/net/c/4fa349156043

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