Re: [PATCH net 1/2] net/iucv: drop HiperSockets frames from other network namespaces

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] AF_IUCV allows unprivileged users in non-initial network namespaces to create sockets in the initial network namespace, bypassing namespace isolation.
- [High] Netdevice reference count leak during NETDEV_UNREGISTER leads to kernel hangs.
- [High] Bypass of `lock_sock()` mutual exclusion in softirq receive path leads to data corruption.
- [High] TOCTOU race in `afiucv_hs_callback_rx` leads to silent SKB memory leak.
- [High] Newly allocated IUCV sockets leak memory and references when an error occurs during connection request processing.
- [High] Incoming packets bypass `sk_rcvbuf` limits via unbounded queuing to `backlog_skb_q`, leading to OOM.
--

--- Patch [1]: [PATCH net 1/2] net/iucv: drop HiperSockets frames from other network namespaces ---
commit f9050fb5eddf9fc6d107045a06d53a731b334db3
Author: Bryam Vargas <hexlabsecurity@proton.me>

net/iucv: drop HiperSockets frames from other network namespaces

This commit adds a namespace check to afiucv_hs_rcv() to drop HiperSockets
frames from other network namespaces. Because iucv_packet_type lacks specific
namespace bindings, it previously landed in the global ptype_base[], allowing
frames from namespaces holding CAP_NET_RAW to interact with sockets owned by
the initial namespace.

> diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
> index ea047bab65e7c..e3ec965d96ca7 100644
> --- a/net/iucv/af_iucv.c
> +++ b/net/iucv/af_iucv.c
> @@ -2064,6 +2064,11 @@ static int afiucv_hs_rcv(struct sk_buff *skb, struct net_device *dev,
>  	int err = NET_RX_SUCCESS;
>  	char nullstring[8];
>  
> +	if (!net_eq(dev_net(dev), &init_net)) {
> +		kfree_skb(skb);
> +		return NET_RX_SUCCESS;
> +	}
> +
>  	if (!pskb_may_pull(skb, sizeof(*trans_hdr))) {
>  		kfree_skb(skb);
>  		return NET_RX_SUCCESS;

[Severity: Critical]
This is a pre-existing issue, but does iucv_sock_create() bypass namespace
isolation?

A process in a non-initial network namespace calling socket(AF_IUCV, ...)
ignores the passed network namespace and unconditionally allocates the socket
in init_net:

net/iucv/af_iucv.c:iucv_sock_create() {
    ...
    sk = iucv_sock_alloc(sock, protocol, GFP_KERNEL, kern);
    // inside iucv_sock_alloc:
    sk = sk_alloc(&init_net, PF_IUCV, prio, &iucv_proto, kern);
    ...
}

Subsequently, iucv_sock_bind() iterates through init_net devices and binds the
socket to them. This appears to allow local users in containers with socket
creation privileges to bypass namespace isolation and communicate over init_net
devices.

[Severity: High]
This isn't a bug introduced by this patch, but is there a netdevice reference
count leak during NETDEV_UNREGISTER?

When an AF_IUCV socket binds to a HiperSockets device via iucv_sock_bind(),
it takes a reference to the device using dev_hold(dev). The netdevice
notifier explicitly ignores the NETDEV_UNREGISTER event:

net/iucv/af_iucv.c:afiucv_netdev_event() {
    ...
    case NETDEV_DOWN:
    case NETDEV_UNREGISTER:
    default:
        break;
    ...
}

If a device is unregistered while a socket remains open, it seems the
unreleased reference could prevent the device from being fully removed,
causing the kernel to hang indefinitely in netdev_wait_allrefs().

[Severity: High]
This is a pre-existing issue, but does afiucv_hs_callback_syn() bypass
lock_sock() mutual exclusion?

The HiperSockets receive path (afiucv_hs_rcv) executes in softirq context.
Callbacks acquire bh_lock_sock(sk) but fail to check sock_owned_by_user(sk):

net/iucv/af_iucv.c:afiucv_hs_callback_syn() {
    ...
    bh_lock_sock(sk);
    if ((sk->sk_state != IUCV_LISTEN) || ...
    ...
}

If user context is currently holding lock_sock() (e.g., executing close()),
sk->sk_lock.owned is 1, but the softirq ignores this and modifies the socket
state concurrently. Could this lead to data races and socket state corruption?

[Severity: High]
This isn't a bug introduced by this patch, but is there a TOCTOU race in
afiucv_hs_callback_rx() leading to a silent SKB memory leak?

The code locklessly checks sk->sk_state:

net/iucv/af_iucv.c:afiucv_hs_callback_rx() {
    ...
    if (sk->sk_state != IUCV_CONNECTED) {
        kfree_skb(skb);
        return NET_RX_SUCCESS;
    }
    ...
}

Concurrently, iucv_sock_close() can execute, set the state to IUCV_CLOSED,
and purge backlog_skb_q. If the softirq reads the state as IUCV_CONNECTED
before it is updated, but queues the SKB after iucv_sock_close() has purged
the queue, the SKB lands in backlog_skb_q of a closing socket.

Since iucv_sock_destruct() fails to purge backlog_skb_q, is the SKB
permanently leaked?

[Severity: High]
This is a pre-existing issue, but do newly allocated IUCV sockets leak memory
and references when an error occurs during connection request processing?

In afiucv_hs_callback_syn(), a new socket is allocated via iucv_sock_alloc(),
which clears its SOCK_ZAPPED flag. On the error path, it calls
iucv_sock_kill(nsk):

net/iucv/af_iucv.c:afiucv_hs_callback_syn() {
    ...
    err = dev_queue_xmit(skb);
    if (nsk)
        iucv_sock_kill(nsk);
    ...
}

However, iucv_sock_kill() returns immediately if !sock_flag(sk, SOCK_ZAPPED):

net/iucv/af_iucv.c:iucv_sock_kill() {
    ...
    if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
        return;
    ...
}

Since the socket is neither unlinked nor freed, can a remote sender exploit
this by spamming AF_IUCV SYN packets to exhaust kernel memory?

[Severity: High]
This isn't a bug introduced by this patch, but do incoming packets bypass
sk_rcvbuf limits?

Incoming data packets are appended to the socket's receive queue via
__sock_queue_rcv_skb(). If this fails because the receive buffer limit is
exceeded, the packet is queued to iucv->backlog_skb_q:

net/iucv/af_iucv.c:afiucv_hs_callback_rx() {
    ...
    if (__sock_queue_rcv_skb(sk, skb))
        /* handle rcv queue full */
        skb_queue_tail(&iucv->backlog_skb_q, skb);
    ...
}

Since backlog_skb_q lacks length limits, could a remote attacker sending a
continuous stream of packets force the kernel to continuously allocate SKBs
and bypass flow control limits?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815-b4-disp-dc82fde4-v1-0-e83b10b22ce9@proton.me?part=1