[PATCH net 0/2] net/iucv: give afiucv_hs_rcv() the preamble a packet_type handler needs

[PATCH net 0/2] net/iucv: give afiucv_hs_rcv() the preamble a packet_type handler needs

[Bryam Vargas via B4 Relay]

iucv_packet_type is registered with no .dev and no .af_packet_net, so
afiucv_hs_rcv() sits in the machine-global ptype_base[] and sees every frame of
its ethertype from every namespace, on any device. It then trusts the frame: no
namespace test, and no private or writable reference before it rewrites the
payload in place.

  1/2 drop frames from other namespaces -- the only one that crosses a
      privilege boundary, and the only one tagged for stable
  2/2 unshare and cow the head before the in-place rewrite

The two siblings differ in how far they go, so I will not claim more than they
do: net/x25/x25_dev.c does the namespace test and then takes a full skb_copy()
of every frame, and net/ieee802154/socket.c does the namespace test but does not
unshare at all. 2/2 takes the cheaper of the two -- skb_share_check() plus
skb_cow_head() -- which costs a refcount test and a compare when the frame is
already private, and copies only when it is not.

2/2 also covers the headroom that afiucv_swap_src_dest() pushes without
checking. That started as a third patch; it collapsed into this one once it was
clear skb_cow_head() is needed for writability anyway and asking it for ETH_HLEN
rather than nothing is free.

2/2 was surfaced by the sashiko.dev review bot on the thread of a previous af_iucv
patch, which flagged both the missing unshare and the unchecked push; the Closes:
trailer on that patch points at its report.

By inspection; not reproduced. Compile-tested for s390x.

---
Bryam Vargas (2):
      net/iucv: drop HiperSockets frames from other network namespaces
      net/iucv: take a private, writable frame before rewriting it in place

 net/iucv/af_iucv.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
---
base-commit: a59f57e2aa127c5354168d2ec4bac920df1be4f4
change-id: 20260815-b4-disp-dc82fde4-3cae5e52ae9f

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

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

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

iucv_packet_type sets neither .dev nor .af_packet_net, so it lands in the
machine-global ptype_base[] that __netif_receive_skb_core walks for every
frame in every namespace, and afiucv_hs_rcv() ignores its dev argument.
An ETH_P_AF_IUCV frame sent from any namespace holding CAP_NET_RAW is
therefore matched against the global iucv_sk_list and can move a socket
owned by the initial namespace: afiucv_hs_callback_synfin() and _fin()
overwrite its sk_state, and _syn() builds an accept-queue child.

Filter on the namespace. The core does not do it for ptype_base[] --
net/core/dev.c leaves namespace filtering to the ptype owner -- and
net/x25/x25_dev.c and net/ieee802154/socket.c both test dev_net(dev) at
exactly this point.

Fixes: 3881ac441f64 ("af_iucv: add HiperSockets transport")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 net/iucv/af_iucv.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index ea047bab65e7..e3ec965d96ca 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;

-- 
2.55.0

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

[sashiko-bot]

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

[PATCH net 2/2] net/iucv: take a private, writable frame before rewriting it in place

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

afiucv_hs_rcv() rewrites the frame in place -- EBCASC() converts four name
fields in the transport header, afiucv_swap_src_dest() swaps them and
pushes an Ethernet header back on -- without taking a private, writable
copy. It sits on the global ptype_base[], so a packet socket (tcpdump is
enough) has packet_rcv() clone every frame first, and net/core/dev.c has
warned since 1998 that such a handler "is not able to sense, that packet
is cloned and should be copied-on-write".

Unshare, then cow the head, in that order: skb_cow_head() can reach
pskb_expand_head(), which has BUG_ON(skb_shared()). Asking for ETH_HLEN
also covers the unchecked push in afiucv_swap_src_dest() --
eth_type_trans() has already pulled that much on the ordinary path, so the
call compares and returns.

Fixes: 3881ac441f64 ("af_iucv: add HiperSockets transport")
Closes: https://sashiko.dev/#/patchset/20260813-b4-disp-60433a46-v1-1-509e1200533e@proton.me?part=1
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 net/iucv/af_iucv.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index e3ec965d96ca..10cfc5e82f04 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -2069,11 +2069,20 @@ static int afiucv_hs_rcv(struct sk_buff *skb, struct net_device *dev,
 		return NET_RX_SUCCESS;
 	}
 
+	skb = skb_share_check(skb, GFP_ATOMIC);
+	if (!skb)
+		return NET_RX_SUCCESS;
+
 	if (!pskb_may_pull(skb, sizeof(*trans_hdr))) {
 		kfree_skb(skb);
 		return NET_RX_SUCCESS;
 	}
 
+	if (skb_cow_head(skb, ETH_HLEN)) {
+		kfree_skb(skb);
+		return NET_RX_SUCCESS;
+	}
+
 	trans_hdr = iucv_trans_hdr(skb);
 	EBCASC(trans_hdr->destAppName, sizeof(trans_hdr->destAppName));
 	EBCASC(trans_hdr->destUserID, sizeof(trans_hdr->destUserID));

-- 
2.55.0

Re: [PATCH net 2/2] net/iucv: take a private, writable frame before rewriting it in place

[sashiko-bot]

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

Pre-existing issues:
- [High] Unprotected socket state modification from softirq context
--

--- Patch [2]: [PATCH net 2/2] net/iucv: take a private, writable frame before rewriting it in place ---
commit 656ba922e59efdf102e7aa3950857e3c0061f9b8
Author: Bryam Vargas <hexlabsecurity@proton.me>

net/iucv: take a private, writable frame before rewriting it in place

This commit modifies afiucv_hs_rcv() to take a private, writable copy
of the socket buffer before rewriting the frame in place. It unshares
the skb and uses skb_cow_head() to ensure enough headroom.

Closes: https://sashiko.dev/#/patchset/20260813-b4-disp-60433a46-v1-1-509e1200533e@proton.me?part=1

> diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
> index e3ec965d96ca7..10cfc5e82f047 100644
> --- a/net/iucv/af_iucv.c
> +++ b/net/iucv/af_iucv.c
> @@ -2069,11 +2069,20 @@ static int afiucv_hs_rcv(struct sk_buff *skb, struct net_device *dev,
>  		return NET_RX_SUCCESS;
>  	}
>  

[ ... ]

[Severity: High]
This is a pre-existing issue, but while reviewing this file I noticed
an unprotected socket state modification:

net/iucv/af_iucv.c:afiucv_hs_callback_txnotify() {
        ...
        if (sk->sk_state == IUCV_CONNECTED) {
                sk->sk_state = IUCV_DISCONN;
                sk->sk_state_change(sk);
        }
        ...
}

Does this modification risk corrupting the socket state?

When qeth hardware issues TX completions, qeth_notify_skbs() is called from
softirq (NAPI) context and invokes afiucv_hs_callback_txnotify().

This modifies sk->sk_state directly without acquiring the socket lock
or checking sock_owned_by_user().

Could this lead to missed state transitions or concurrent execution with
process-context operations that assume exclusive access under lock_sock()?

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