AFIUCV can currently be selected with IUCV disabled. That configuration
does not build af_iucv.o because the parent Makefile only enters
net/iucv under CONFIG_IUCV. Independently entering the directory is not
sufficient: AF_IUCV also relies on the IUCV implementation.

Restore the IUCV dependency for AFIUCV, as proposed during review. This
also constrains AFIUCV to a module when its IUCV provider is modular,
without changing the directory traversal rules.

The issue was found by our static-analysis tool.

Fixes: c69748d1c9b5 ("iucv: kernel option for z/VM IUCV and HiperSockets")
Assisted-by: gpt 5
Signed-off-by: Pengpeng Hou <hppiscas@163.com>
---
Changes since v2:
https://lore.kernel.org/all/20260815135456.53684-1-pengpeng@iscas.ac.cn/
Replace the v2 always-descend approach with the IUCV dependency proposed
by Alexandra. Do not carry the review tag for the superseded
implementation.

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

diff --git a/net/iucv/Kconfig b/net/iucv/Kconfig
index 5cfddc9c6498..2686d795cc2f 100644
--- a/net/iucv/Kconfig
+++ b/net/iucv/Kconfig
@@ -9,7 +9,7 @@ config IUCV
 	  communication link between VM guests.
 
 config AFIUCV
-	depends on S390
+	depends on IUCV
 	def_tristate m if QETH_L3 || IUCV
 	prompt "AF_IUCV Socket support (S390 - z/VM and HiperSockets transport)"
 	help

base-commit: 518e5b794c06c0f0eb40df3e202274a66202c137
-- 
2.50.1 (Apple Git-155)

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

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] net/iucv/Kconfig: replacing `depends on S390` with `depends on
  IUCV` for AFIUCV reverts, without disclosure, the design intent of the…

--

net/iucv: require IUCV for AF_IUCV sockets

This changes the AFIUCV Kconfig symbol from "depends on S390" to
"depends on IUCV" so that AFIUCV can no longer be enabled when the IUCV
core is disabled. It replaces the v2 approach of always descending into
net/iucv from net/Makefile.

Previous posting referenced in the patch:
https://lore.kernel.org/all/20260815135456.53684-1-pengpeng@iscas.ac.cn/

> diff --git a/net/iucv/Kconfig b/net/iucv/Kconfig
> index 5cfddc9c6498..2686d795cc2f 100644
> --- a/net/iucv/Kconfig
> +++ b/net/iucv/Kconfig
> @@ -9,7 +9,7 @@ config IUCV
>  	  communication link between VM guests.
>  
>  config AFIUCV
> -	depends on S390
> +	depends on IUCV

[Severity: Medium]
Is the stated rationale accurate here?  The commit message says:

  "Independently entering the directory is not sufficient: AF_IUCV also
   relies on the IUCV implementation."

Looking at afiucv_init() in net/iucv/af_iucv.c, the only reference to
the IUCV core is compile-time guarded:

	if (machine_is_vm() && IS_ENABLED(CONFIG_IUCV)) {
		...
		pr_iucv = &iucv_if;
	} else {
		memset(&iucv_userid, 0, sizeof(iucv_userid));
		pr_iucv = NULL;
	}

and iucv_sock_alloc() treats a NULL provider as a supported mode:

	if (pr_iucv)
		iucv->transport = AF_IUCV_TRANS_IUCV;
	else
		iucv->transport = AF_IUCV_TRANS_HIPER;

The other iucv_* users in af_iucv.c are iucv_path_alloc() and
iucv_path_free(), which are static inline wrappers around kzalloc_obj()
and kfree() in include/net/iucv/iucv.h.  Does af_iucv.o then have any
link-time dependency on the IUCV core at all with CONFIG_IUCV=n?

The reason af_iucv.o is not built in that configuration appears to be
only the traversal rule in net/Makefile:

	obj-$(CONFIG_IUCV)		+= iucv/

Also, c69748d1c9b5, named in the Fixes: tag, deliberately changed
"depends on IUCV" to "depends on S390" together with
"def_tristate m if QETH_L3 || IUCV" so that AF_IUCV could be enabled for
the HiperSockets transport alone.  Since this patch makes the AFIUCV
symbol invisible for every CONFIG_IUCV=n configuration, is the word
"Restore" describing the right direction, and could the commit message
say explicitly that the HiperSockets-only configuration is being dropped
rather than fixed?

One further consequence worth spelling out: iucv_init() in
net/iucv/iucv.c bails out early on non-z/VM systems:

	if (!machine_is_vm()) {
		rc = -EPROTONOSUPPORT;
		goto out;
	}

so on an LPAR, does a modular IUCV still work as the provider that
AFIUCV=m now depends on, or does this effectively force CONFIG_IUCV=y
for HiperSockets AF_IUCV users?

>  	def_tristate m if QETH_L3 || IUCV
>  	prompt "AF_IUCV Socket support (S390 - z/VM and HiperSockets transport)"
>  	help

[Severity: Low]
With "depends on IUCV" in place, IUCV is always y or m whenever AFIUCV
is visible, so the QETH_L3 term of

	def_tristate m if QETH_L3 || IUCV

can never decide the default any more.  Should the default clause be
reduced to just "def_tristate m"?

Similarly, the prompt and the help text below still present z/VM and
HiperSockets as separable transports:

	  Select this option if you want to use AF_IUCV socket applications
	  based on z/VM inter-user communication vehicle or based on
	  HiperSockets.

Do these need updating too, now that the HiperSockets-only case can no
longer be configured?  The IS_ENABLED(CONFIG_IUCV) test in
afiucv_init() also becomes permanently true after this change.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260920034503.17322-1-hppiscas%40163.com

On 21.09.26 05:48, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] net/iucv/Kconfig: replacing `depends on S390` with `depends on
>   IUCV` for AFIUCV reverts, without disclosure, the design intent of the…
> 
> --

Pengpeng Hou,
thank you for your contribution.

tl,dr: I think Sashiko is right and I found another issue with this patch when testing.
Let me know, if you want me to continue with a Reported-by you, or if you want to work
on a v4. In that case see my comments below.


> 
> net/iucv: require IUCV for AF_IUCV sockets
> 
> This changes the AFIUCV Kconfig symbol from "depends on S390" to
> "depends on IUCV" so that AFIUCV can no longer be enabled when the IUCV
> core is disabled. It replaces the v2 approach of always descending into
> net/iucv from net/Makefile.
> 
> Previous posting referenced in the patch:
> https://lore.kernel.org/all/20260815135456.53684-1-pengpeng@iscas.ac.cn/
> 
>> diff --git a/net/iucv/Kconfig b/net/iucv/Kconfig
>> index 5cfddc9c6498..2686d795cc2f 100644
>> --- a/net/iucv/Kconfig
>> +++ b/net/iucv/Kconfig
>> @@ -9,7 +9,7 @@ config IUCV
>>  	  communication link between VM guests.
>>  
>>  config AFIUCV
>> -	depends on S390
>> +	depends on IUCV
> 
> [Severity: Medium]
> Is the stated rationale accurate here?  The commit message says:
> 
>   "Independently entering the directory is not sufficient: AF_IUCV also
>    relies on the IUCV implementation."
> 
> Looking at afiucv_init() in net/iucv/af_iucv.c, the only reference to
> the IUCV core is compile-time guarded:
> 
> 	if (machine_is_vm() && IS_ENABLED(CONFIG_IUCV)) {
> 		...
> 		pr_iucv = &iucv_if;
> 	} else {
> 		memset(&iucv_userid, 0, sizeof(iucv_userid));
> 		pr_iucv = NULL;
> 	}
> 
> and iucv_sock_alloc() treats a NULL provider as a supported mode:
> 
> 	if (pr_iucv)
> 		iucv->transport = AF_IUCV_TRANS_IUCV;
> 	else
> 		iucv->transport = AF_IUCV_TRANS_HIPER;
> 
> The other iucv_* users in af_iucv.c are iucv_path_alloc() and
> iucv_path_free(), which are static inline wrappers around kzalloc_obj()
> and kfree() in include/net/iucv/iucv.h.  Does af_iucv.o then have any
> link-time dependency on the IUCV core at all with CONFIG_IUCV=n?
> 

I actually tested IUCV=N, AF_IUCV=M with your v2 and it worked fine.
So it's not that "AF_IUCV also relies on the IUCV implementation";

In case of IUCV=M, AF_IUCV=Y
it's that 'IS_ENABLED(CONFIG_IUCV)' causes the issue of
	undefined reference to `iucv_if'
when IUCV is a module and AF_IUCV is built in.



> The reason af_iucv.o is not built in that configuration appears to be
> only the traversal rule in net/Makefile:
> 
> 	obj-$(CONFIG_IUCV)		+= iucv/
> 
> Also, c69748d1c9b5, named in the Fixes: tag, deliberately changed
> "depends on IUCV" to "depends on S390" together with
> "def_tristate m if QETH_L3 || IUCV" so that AF_IUCV could be enabled for
> the HiperSockets transport alone.  Since this patch makes the AFIUCV
> symbol invisible for every CONFIG_IUCV=n configuration, is the word
> "Restore" describing the right direction, and could the commit message
> say explicitly that the HiperSockets-only configuration is being dropped
> rather than fixed?


I agree it seems, it was the intention, that AF_IUCV over HiperSockets,
could be used independently of IUCV over z/VM's TRANS_IUCV.
However I never heard of any usecase where such a kernel was built,
typically you want both on s390.
That's why I proposed the shortcut.


> 
> One further consequence worth spelling out: iucv_init() in
> net/iucv/iucv.c bails out early on non-z/VM systems:
> 
> 	if (!machine_is_vm()) {
> 		rc = -EPROTONOSUPPORT;
> 		goto out;
> 	}
> 
> so on an LPAR, does a modular IUCV still work as the provider that
> AFIUCV=m now depends on, or does this effectively force CONFIG_IUCV=y
> for HiperSockets AF_IUCV users?
> 


Actually even with your v2 IUCV=M, AF_IUCV=M builds fine in an LPAR,
but modprobe returns with
modprobe: ERROR: could not insert 'af_iucv': Protocol not supported	(not expected)
modprobe: ERROR: could not insert 'iucv': Protocol not supported   	(expected)

Is this a result of
> 	if (machine_is_vm() && ...
>		err = -EPROTONOSUPPORT;

I don't fully understand it yet.

But that combination can be defined, even with your v3, so we have another open issue here.

I definitively need to test all possible combinations! (Should have done that earlier, I admit)



>>  	def_tristate m if QETH_L3 || IUCV
>>  	prompt "AF_IUCV Socket support (S390 - z/VM and HiperSockets transport)"
>>  	help
> 
> [Severity: Low]
> With "depends on IUCV" in place, IUCV is always y or m whenever AFIUCV
> is visible, so the QETH_L3 term of
> 
> 	def_tristate m if QETH_L3 || IUCV
> 
> can never decide the default any more.  Should the default clause be
> reduced to just "def_tristate m"?
> 
> Similarly, the prompt and the help text below still present z/VM and
> HiperSockets as separable transports:
> 
> 	  Select this option if you want to use AF_IUCV socket applications
> 	  based on z/VM inter-user communication vehicle or based on
> 	  HiperSockets.
> 
> Do these need updating too, now that the HiperSockets-only case can no
> longer be configured?  The IS_ENABLED(CONFIG_IUCV) test in
> afiucv_init() also becomes permanently true after this change.

Valid thoughts, in case we go with the 'depends' option.