[PATCH net-next] net/iucv: fix UAF in afiucv_netdev_event()

[PATCH net-next] net/iucv: fix UAF in afiucv_netdev_event()

[Nagamani PV]

afiucv_netdev_event() traverses iucv_sk_list without holding
iucv_sk_list.lock.

A concurrent socket teardown can unlink and free an af_iucv socket via
iucv_sock_kill() while the netdevice notifier path is still traversing
the list, resulting in a use-after-free when dereferencing the socket.

syzbot reported a KASAN slab-use-after-free triggered from the netdevice
notifier path:

  BUG: KASAN: slab-use-after-free in afiucv_netdev_event+0x166/0x570
  Read of size 8 at addr 001a54009dcda368 by task syz.3.24/516

  Call Trace:
    afiucv_netdev_event+0x166/0x570 net/iucv/af_iucv.c:2193
    notifier_call_chain+0x18e/0x510 kernel/notifier.c:85
    call_netdevice_notifiers net/core/dev.c:2301 [inline]
    __dev_close_many+0x1c6/0x680 net/core/dev.c:1747
    unregister_netdevice_many_notify+0x9b6/0x2650 net/core/dev.c:12388
    unregister_netdevice_queue+0x392/0x3d0 net/core/dev.c:12291
    __tun_detach+0xa50/0x1990 drivers/net/tun.c:621

  Allocated by task 521:
    sk_prot_alloc+0xd2/0x230 net/core/sock.c:2247
    sk_alloc+0x48/0x680 net/core/sock.c:2303
    iucv_sock_alloc+0x4e/0x710 net/iucv/af_iucv.c:456
    iucv_sock_create+0x114/0x180 net/iucv/af_iucv.c:2254

  Freed by task 519:
    kfree+0x164/0x500 mm/slub.c:6561
    iucv_sock_release+0x12e/0x150 net/iucv/af_iucv.c:1482
    sock_close+0xa4/0x230 net/socket.c:1514

Protect the list traversal with the existing read-side lock.
Use read_lock_bh()/read_unlock_bh() to synchronize with
write_lock_bh()-protected updates to iucv_sk_list.

Fixes: 9fbd87d41392 ("af_iucv: handle netdev events")
Cc: stable@vger.kernel.org
Suggested-by: Hidayath Khan <hidayath@linux.ibm.com>
Signed-off-by: Nagamani PV <nagamani@linux.ibm.com>

---
 net/iucv/af_iucv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index ea047bab65e7..034039c50941 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -2213,6 +2213,7 @@ static int afiucv_netdev_event(struct notifier_block *this,
 	switch (event) {
 	case NETDEV_REBOOT:
 	case NETDEV_GOING_DOWN:
+		read_lock_bh(&iucv_sk_list.lock);
 		sk_for_each(sk, &iucv_sk_list.head) {
 			iucv = iucv_sk(sk);
 			if ((iucv->hs_dev == event_dev) &&
@@ -2223,6 +2224,7 @@ static int afiucv_netdev_event(struct notifier_block *this,
 				sk->sk_state_change(sk);
 			}
 		}
+		read_unlock_bh(&iucv_sk_list.lock);
 		break;
 	case NETDEV_DOWN:
 	case NETDEV_UNREGISTER:
-- 
2.53.0

Re: [PATCH net-next] net/iucv: fix UAF in afiucv_netdev_event()

[Alexandra Winter]

On 03.08.26 20:20, Nagamani PV wrote:
> afiucv_netdev_event() traverses iucv_sk_list without holding
> iucv_sk_list.lock.
> 
> A concurrent socket teardown can unlink and free an af_iucv socket via
> iucv_sock_kill() while the netdevice notifier path is still traversing
> the list, resulting in a use-after-free when dereferencing the socket.
> 
> syzbot reported a KASAN slab-use-after-free triggered from the netdevice
> notifier path:
> 
>   BUG: KASAN: slab-use-after-free in afiucv_netdev_event+0x166/0x570
>   Read of size 8 at addr 001a54009dcda368 by task syz.3.24/516
> 
>   Call Trace:
>     afiucv_netdev_event+0x166/0x570 net/iucv/af_iucv.c:2193
>     notifier_call_chain+0x18e/0x510 kernel/notifier.c:85
>     call_netdevice_notifiers net/core/dev.c:2301 [inline]
>     __dev_close_many+0x1c6/0x680 net/core/dev.c:1747
>     unregister_netdevice_many_notify+0x9b6/0x2650 net/core/dev.c:12388
>     unregister_netdevice_queue+0x392/0x3d0 net/core/dev.c:12291
>     __tun_detach+0xa50/0x1990 drivers/net/tun.c:621
> 
>   Allocated by task 521:
>     sk_prot_alloc+0xd2/0x230 net/core/sock.c:2247
>     sk_alloc+0x48/0x680 net/core/sock.c:2303
>     iucv_sock_alloc+0x4e/0x710 net/iucv/af_iucv.c:456
>     iucv_sock_create+0x114/0x180 net/iucv/af_iucv.c:2254
> 
>   Freed by task 519:
>     kfree+0x164/0x500 mm/slub.c:6561
>     iucv_sock_release+0x12e/0x150 net/iucv/af_iucv.c:1482
>     sock_close+0xa4/0x230 net/socket.c:1514
> 
> Protect the list traversal with the existing read-side lock.
> Use read_lock_bh()/read_unlock_bh() to synchronize with
> write_lock_bh()-protected updates to iucv_sk_list.
> 
> Fixes: 9fbd87d41392 ("af_iucv: handle netdev events")
> Cc: stable@vger.kernel.org
> Suggested-by: Hidayath Khan <hidayath@linux.ibm.com>
> Signed-off-by: Nagamani PV <nagamani@linux.ibm.com>
> 
> ---
>  net/iucv/af_iucv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
> index ea047bab65e7..034039c50941 100644
> --- a/net/iucv/af_iucv.c
> +++ b/net/iucv/af_iucv.c
> @@ -2213,6 +2213,7 @@ static int afiucv_netdev_event(struct notifier_block *this,
>  	switch (event) {
>  	case NETDEV_REBOOT:
>  	case NETDEV_GOING_DOWN:
> +		read_lock_bh(&iucv_sk_list.lock);
>  		sk_for_each(sk, &iucv_sk_list.head) {
>  			iucv = iucv_sk(sk);
>  			if ((iucv->hs_dev == event_dev) &&
> @@ -2223,6 +2224,7 @@ static int afiucv_netdev_event(struct notifier_block *this,
>  				sk->sk_state_change(sk);
>  			}
>  		}
> +		read_unlock_bh(&iucv_sk_list.lock);
>  		break;
>  	case NETDEV_DOWN:
>  	case NETDEV_UNREGISTER:

Nagmani,
As this is a fix, it should have been prefixed with net and not net-next.


See the discussions with Bryam Vargas [1]:
It is correct that afiucv_netdev_event() is missing lock protection.

However for a complete solution it should call lock_sock()
(because netdev events are called in process context) and handle
owned_by_user and a backlog queue.

So this patch improves the situation, but is not the complete solution.
I'd rather continue to work on a more complete fix than take this one,
but both ways are possible.

Kind regards
Alexandra

[1] https://lore.kernel.org/netdev/20260724222917.134769-1-hexlabsecurity@proton.me/

Re: [PATCH net-next] net/iucv: fix UAF in afiucv_netdev_event()

[Nagamani PV]

On 04/08/26 2:03 PM, Alexandra Winter wrote:
> 
> As this is a fix, it should have been prefixed with net and not net-next.
> 

Apologies for that — you had already corrected this in the internal
review. v2 will use [PATCH net] and include your Reviewed-by.

> 
> See the discussions with Bryam Vargas [1]:
> It is correct that afiucv_netdev_event() is missing lock protection.
> 
> However for a complete solution it should call lock_sock()
> (because netdev events are called in process context) and handle
> owned_by_user and a backlog queue.
> 
> So this patch improves the situation, but is not the complete solution.
> I'd rather continue to work on a more complete fix than take this one,
> but both ways are possible.
> 

Understood. This patch addresses the UAF on iucv_sk_list traversal
specifically. The lock_sock() concern is at a different level and
could be a follow-up.

Could you clarify your preference — take this as an incremental fix
with a follow-up for lock_sock(), or hold and work toward a combined
solution?

Thanks,
Nagamani

> Kind regards
> Alexandra
> 
> [1] https://lore.kernel.org/netdev/20260724222917.134769-1-hexlabsecurity@proton.me/
> 
> 
> 
> 

Re: [PATCH net-next] net/iucv: fix UAF in afiucv_netdev_event()

[Alexandra Winter]

On 04.08.26 14:52, Nagamani PV wrote:
>> I'd rather continue to work on a more complete fix than take this one,
>> but both ways are possible.
>>
> Understood. This patch addresses the UAF on iucv_sk_list traversal
> specifically. The lock_sock() concern is at a different level and
> could be a follow-up.
> 
> Could you clarify your preference — take this as an incremental fix
> with a follow-up for lock_sock(), or hold and work toward a combined
> solution?

I prefer to work on the combined solution

Re: [PATCH net-next] net/iucv: fix UAF in afiucv_netdev_event()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The patch introduces a sleep-in-atomic-context bug by calling a sleepable memory allocation function while holding a spinlock (`read_lock_bh`) that disables bottom halves.

Pre-existing issues:
- [High] Socket state, shutdown flags, and transmissions are manipulated locklessly, leading to data races, lost state updates, and a potential NULL pointer dereference if the socket is orphaned concurrently.
--

--- Patch [1]: [PATCH net-next] net/iucv: fix UAF in afiucv_netdev_event() ---
commit 295060a75254f1d540b1b59d065d45ebee09c8e8
Author: Nagamani PV <nagamani@linux.ibm.com>
Subject: net/iucv: fix UAF in afiucv_netdev_event()

This commit aims to fix a KASAN slab-use-after-free in afiucv_netdev_event() by
protecting the socket list traversal with read_lock_bh() / read_unlock_bh().
This prevents concurrent socket teardown from freeing the socket during
iteration.

> diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
> index ea047bab65e7c..034039c509418 100644
> --- a/net/iucv/af_iucv.c
> +++ b/net/iucv/af_iucv.c

[ ... ]

> @@ -2213,6 +2213,7 @@ static int afiucv_netdev_event(struct notifier_block *this,
>  	switch (event) {
>  	case NETDEV_REBOOT:
>  	case NETDEV_GOING_DOWN:
> +		read_lock_bh(&iucv_sk_list.lock);
>  		sk_for_each(sk, &iucv_sk_list.head) {
>  			iucv = iucv_sk(sk);
>  			if ((iucv->hs_dev == event_dev) &&
>  			    (sk->sk_state == IUCV_CONNECTED)) {

[Severity: High]
This isn't a bug introduced by this patch, but does checking and modifying
the socket state here without taking the socket lock create a data race?

If a concurrent iucv_sock_release() sets the state to closed and calls
sock_orphan(sk) (which sets sk->sk_socket = NULL) before waiting on
iucv_sk_list.lock, could this event loop read the old state, get delayed,
and invoke iucv_send_ctrl() after the socket is orphaned?

When sock_alloc_send_pskb() is subsequently called inside iucv_send_ctrl(),
if the socket's write buffer is full, it tries to set SOCK_NOSPACE via
set_bit(SOCK_NOSPACE, &sk->sk_socket->flags). Would this result in a NULL
pointer dereference since sk->sk_socket was cleared?

The iucv_send_ctrl() function also directly manipulates sk->sk_shutdown
temporarily without locking the socket:

iucv_send_ctrl() {
    if (sk->sk_shutdown & SEND_SHUTDOWN) {
        shutdown = sk->sk_shutdown;
        sk->sk_shutdown &= RCV_SHUTDOWN;
    }
    ...
}

Does this create a race window that loses concurrent shutdown updates?

>  				if (event == NETDEV_GOING_DOWN)
>  					iucv_send_ctrl(sk, AF_IUCV_FLAG_FIN);

[Severity: High]
Does this introduce a sleep-in-atomic-context bug?

By acquiring read_lock_bh() earlier in afiucv_netdev_event(), the code now
executes in an atomic context. However, iucv_send_ctrl() performs a memory
allocation that can block:

afiucv_netdev_event()
   read_lock_bh(&iucv_sk_list.lock)
   iucv_send_ctrl()
      sock_alloc_send_skb()

The allocation relies on sk->sk_allocation (which defaults to
GFP_KERNEL | GFP_DMA for AF_IUCV sockets). Since these flags contain
__GFP_DIRECT_RECLAIM, will this allow the allocator to sleep and trigger
a "scheduling while atomic" panic?

>  				sk->sk_state = IUCV_DISCONN;
>  				sk->sk_state_change(sk);
>  			}
>  		}
> +		read_unlock_bh(&iucv_sk_list.lock);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803182053.2355882-1-nagamani@linux.ibm.com?part=1