linux-kernel - Re: [PATCH 2/3] module: make structure definitions always visible

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7ccb0a97-fc20-4493-8187-48ecfd07bac2@kernel.org>
Date: Fri, 11 Jul 2025 15:24:25 +0200
From: Daniel Gomez <da.gomez@...nel.org>
To: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
Cc: Luis Chamberlain <mcgrof@...nel.org>, Petr Pavlu <petr.pavlu@...e.com>,
 Sami Tolvanen <samitolvanen@...gle.com>, Daniel Gomez
 <da.gomez@...sung.com>, Brendan Higgins <brendan.higgins@...ux.dev>,
 David Gow <davidgow@...gle.com>, Rae Moar <rmoar@...gle.com>,
 linux-modules@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com
Subject: Re: [PATCH 2/3] module: make structure definitions always visible

On 11/07/2025 08.29, Thomas WeiÃschuh wrote:
> On Mon, Jul 07, 2025 at 09:11:05PM +0200, Daniel Gomez wrote:
>> On 12/06/2025 16.53, Thomas WeiÃschuh wrote:
>>> To write code that works with both CONFIG_MODULES=y and CONFIG_MODULES=n
>>> it is convenient to use "if (IS_ENABLED(CONFIG_MODULES))" over raw #ifdef.
>>> The code will still fully typechecked but the unreachable parts are
>>> discarded by the compiler. This prevents accidental breakage when a certain
>>> kconfig combination was not specifically tested by the developer.
>>> This pattern is already supported to some extend by module.h defining
>>> empty stub functions if CONFIG_MODULES=n.
>>> However some users of module.h work on the structured defined by module.h.
>>>
>>> Therefore these structure definitions need to be visible, too.
>>
>> We are missing here which structures are needed. + we are making more things
>> visible than what we actually need.
>>
>>>
>>> Many structure members are still gated by specific configuration settings.
>>> The assumption for those is that the code using them will be gated behind
>>> the same configuration setting anyways.
>>
>> I think code and kconfig need to reflect the actual dependencies. For example,
>> if CONFIG_LIVEPATCH depends on CONFIG_MODULES, we need to specify that in
>> Kconfig with depends on, as well as keep the code gated by these 2 configs with
>> ifdef/IS_ENABLED.
> 
> If CONFIG_LIVEPATCH depends on CONFIG_MODULES in kconfig then
> IS_ENABLED(CONFIG_LIVEPATCH) will depend on CONFIG_MODULES automatically.
> There is no need for another explicit IS_ENABLED(CONFIG_MODULES).

This makes sense to me. My assessment before to reflect in code what we have in
kconfig does not scale. Thanks.

> 
>>>
>>> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
>>> ---
>>>  include/linux/module.h | 23 ++++++++++++-----------
>>>  1 file changed, 12 insertions(+), 11 deletions(-)

{...}

>>
>> After the patch, mod_tree_node is not needed externally.
> 
> Can you explain what you mean with "not needed externally"?
> 'struct mod_tree_node' is only ever used by core module code.
> It is only public because it is embedded in the public 'struct module'

But only when MODULES_TREE_LOOKUP is enabled. Now, all kernels (regardless of
that config) will define mod_tree_node data structure.

However, Petr already stated that is harmless to do so. I was trying here to
not be useless.

With that, changes look good to me:

Reviewed-by: Daniel Gomez <da.gomez@...sung.com>