[PATCH net v2] net/iucv: descend into net/iucv independently of IUCV

[PATCH net v2] net/iucv: descend into net/iucv independently of IUCV

[Pengpeng Hou]

AFIUCV can be enabled as either built-in or a module through QETH_L3 even
when IUCV is disabled. The top-level net Makefile currently descends into
net/iucv/ only under CONFIG_IUCV, so the valid configurations
CONFIG_AFIUCV=y/m with CONFIG_IUCV=n never consider af_iucv.o.

Always descend into net/iucv/. Its Makefile already gates iucv.o and
af_iucv.o on their own Kconfig symbols, so this does not build either
object unless selected.

Fixes: c69748d1c9b5 ("iucv: kernel option for z/VM IUCV and HiperSockets")
Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>

Assisted-by: Codex:gpt-5
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since RFC:
https://lore.kernel.org/all/20260625061303.36326-1-pengpeng@iscas.ac.cn/
- submit this as a regular fix rather than an RFC
- describe both AFIUCV=y and AFIUCV=m with IUCV=n
- add Alexandra's Reviewed-by

 net/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/Makefile b/net/Makefile
index 5b2dd7f07a85..c0da946b281e 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -45,7 +45,7 @@ obj-$(CONFIG_WIRELESS)		+= wireless/
 obj-$(CONFIG_MAC80211)		+= mac80211/
 obj-$(CONFIG_TIPC)		+= tipc/
 obj-$(CONFIG_NETLABEL)		+= netlabel/
-obj-$(CONFIG_IUCV)		+= iucv/
+obj-y				+= iucv/
 obj-$(CONFIG_SMC)		+= smc/
 obj-$(CONFIG_RFKILL)		+= rfkill/
 obj-$(CONFIG_NET_9P)		+= 9p/
-- 
2.50.1 (Apple Git-155)

Re: [PATCH net v2] net/iucv: descend into net/iucv independently of IUCV

[sashiko-bot]

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

New issues:
- [High] Unconditional descent into net/iucv/ causes a vmlinux linker error when CONFIG_AFIUCV=y and CONFIG_IUCV=m.

Pre-existing issues:
- [High] Packet handler modifies and queues shared SKBs without ensuring exclusive ownership, causing list corruption and data corruption.
--

--- Patch [1]: [PATCH net v2] net/iucv: descend into net/iucv independently of IUCV ---
commit e7b1f87fb6bb063631fc940969ffda9c69434171
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

net/iucv: descend into net/iucv independently of IUCV

AFIUCV can be enabled as either built-in or a module through QETH_L3 even
when IUCV is disabled. The top-level net Makefile currently descends into
net/iucv/ only under CONFIG_IUCV, so the valid configurations
CONFIG_AFIUCV=y/m with CONFIG_IUCV=n never consider af_iucv.o.

> diff --git a/net/Makefile b/net/Makefile
> index 5b2dd7f07a85e..c0da946b281e1 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -45,7 +45,7 @@ obj-$(CONFIG_WIRELESS)		+= wireless/
>  obj-$(CONFIG_MAC80211)		+= mac80211/
>  obj-$(CONFIG_TIPC)		+= tipc/
>  obj-$(CONFIG_NETLABEL)		+= netlabel/
> -obj-$(CONFIG_IUCV)		+= iucv/
> +obj-y				+= iucv/

[Severity: High]
Does unconditionally descending into iucv/ cause a vmlinux linker error when
CONFIG_AFIUCV=y and CONFIG_IUCV=m?

Before this patch, Kbuild descended into net/iucv/ as a module (obj-m),
compiling af_iucv.o as a module. With this change, af_iucv.o is compiled
into vmlinux when CONFIG_AFIUCV=y.

However, af_iucv.c contains:

net/iucv/af_iucv.c:afiucv_init() {
    ...
    if (machine_is_vm() && IS_ENABLED(CONFIG_IUCV)) {
        cpcmd("QUERY USERID", iucv_userid, sizeof(iucv_userid), &err);
        ...
        pr_iucv = &iucv_if;
    }

Because IS_ENABLED(CONFIG_IUCV) evaluates to 1 when CONFIG_IUCV=m, the
built-in afiucv_init() will emit an external reference to iucv_if (which
resides in the iucv.ko module), leading to an undefined reference linker
error.

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the IUCV
code, I noticed a pre-existing issue in the AF_IUCV packet handlers.

Could the packet handler corrupt memory by modifying and queuing shared SKBs
without ensuring exclusive ownership?

When a HiperSockets AF_IUCV packet is received by afiucv_hs_rcv() while a
packet socket like tcpdump is concurrently active on the same interface,
the network core passes a shared or cloned SKB to the handlers.

afiucv_hs_rcv() and its callbacks modify the packet data in-place without
calling skb_share_check() or skb_unshare():

net/iucv/af_iucv.c:afiucv_hs_rcv() {
    ...
    EBCASC(trans_hdr->destAppName, sizeof(trans_hdr->destAppName));
    EBCASC(trans_hdr->destUserID, sizeof(trans_hdr->destUserID));
    ...
}

The same happens in the callbacks before transmission:

net/iucv/af_iucv.c:afiucv_hs_callback_syn() {
    ...
    skb_push(skb, ETH_HLEN);
    memset(skb->data, 0, ETH_HLEN);
    ...
}

Furthermore, it enqueues the shared SKB directly into the backlog:

net/iucv/af_iucv.c:afiucv_hs_callback_rx() {
    ...
    skb_queue_tail(&iucv->backlog_skb_q, skb);
    ...
}

If the packet socket also enqueues the SKB, the SKB's next and prev
pointers are concurrently overwritten, which can lead to list corruption
and a kernel panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815135456.53684-1-pengpeng@iscas.ac.cn?part=1

Re: [PATCH net v2] net/iucv: descend into net/iucv independently of IUCV

[Alexandra Winter]

On 16.08.26 15:55, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] Unconditional descent into net/iucv/ causes a vmlinux linker error when CONFIG_AFIUCV=y and CONFIG_IUCV=m.

@Pengpeng Hou,
if it easier, you can also fix this issue by making CONFIG_AFIUCV depend on CONFIG_IUCV.
I see no practical scenario where we would want AFIUCV without IUCV.

Re: [PATCH net v2] net/iucv: descend into net/iucv independently of IUCV

[Alexandra Winter]

On 17.08.26 14:06, Alexandra Winter wrote:
> 
> 
> On 16.08.26 15:55, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>>
>> New issues:
>> - [High] Unconditional descent into net/iucv/ causes a vmlinux linker error when CONFIG_AFIUCV=y and CONFIG_IUCV=m.
> 
> @Pengpeng Hou,
> if it easier, you can also fix this issue by making CONFIG_AFIUCV depend on CONFIG_IUCV.
> I see no practical scenario where we would want AFIUCV without IUCV.
> 


And I agree with Sashiko. I did not verify this combination with your initial RFC, sorry.
So I propose to depend AFIUCV on IUCV instead of your patch.
Do you want to send such a patch, or do you want me to do that?