* Re: [PATCH v4 0/7] Add generated modalias to modules.builtin.modinfo
From: Alexey Gladkov @ 2025-07-22 10:05 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Petr Pavlu, Luis Chamberlain, Sami Tolvanen, Daniel Gomez,
Nathan Chancellor, Nicolas Schier, linux-kernel, linux-modules,
linux-kbuild, linux-scsi
In-Reply-To: <CAK7LNARjC_FCam14RXfTVTQ4_jtXuBKfDsdyG84_k9L1x5zJyg@mail.gmail.com>
On Wed, Jul 16, 2025 at 01:23:26AM +0900, Masahiro Yamada wrote:
> Hi, sorry for the delay.
>
> On Mon, Jul 14, 2025 at 10:41 PM Alexey Gladkov <legion@kernel.org> wrote:
> >
> > On Sat, Jun 21, 2025 at 03:57:12PM +0200, Alexey Gladkov wrote:
> > > The modules.builtin.modinfo file is used by userspace (kmod to be specific) to
> > > get information about builtin modules. Among other information about the module,
> > > information about module aliases is stored. This is very important to determine
> > > that a particular modalias will be handled by a module that is inside the
> > > kernel.
> > >
> > > There are several mechanisms for creating modalias for modules:
> > >
> > > The first is to explicitly specify the MODULE_ALIAS of the macro. In this case,
> > > the aliases go into the '.modinfo' section of the module if it is compiled
> > > separately or into vmlinux.o if it is builtin into the kernel.
> > >
> > > The second is the use of MODULE_DEVICE_TABLE followed by the use of the
> > > modpost utility. In this case, vmlinux.o no longer has this information and
> > > does not get it into modules.builtin.modinfo.
> > >
> > > For example:
> > >
> > > $ modinfo pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30
> > > modinfo: ERROR: Module pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30 not found.
> > >
> > > $ modinfo xhci_pci
> > > name: xhci_pci
> > > filename: (builtin)
> > > license: GPL
> > > file: drivers/usb/host/xhci-pci
> > > description: xHCI PCI Host Controller Driver
> > >
> > > The builtin module is missing alias "pci:v*d*sv*sd*bc0Csc03i30*" which will be
> > > generated by modpost if the module is built separately.
> > >
> > > To fix this it is necessary to add the generated by modpost modalias to
> > > modules.builtin.modinfo.
> > >
> > > Fortunately modpost already generates .vmlinux.export.c for exported symbols. It
> > > is possible to use this file to create a '.modinfo' section for builtin modules.
> > > The modules.builtin.modinfo file becomes a composite file. One part is extracted
> > > from vmlinux.o, the other part from .vmlinux.export.o.
> >
> > Masahiro Yamada, does this version of the patchset look better to you ?
>
>
> Looks better, but this may break s390 build:
>
> https://lore.kernel.org/linux-kbuild/202506062053.zbkFBEnJ-lkp@intel.com/
>
> I have not taken a close look at it.
> If we do not find how to fix the warning, we would
> end up with the original solution.
I think I found a problem. I just pushed fix to my branch. I'll make a new
version of the patchset in a few days. I want to test it a bit longer.
> > > Notes:
> > > - v4:
> > > * Rework the patchset based on top of Masahiro Yamada's patches.
> > > * Add removal of unnecessary __mod_device_table__* symbols to avoid symbol
> > > table growth in vmlinux.
> > > * rust code takes into account changes in __mod_device_table__*.
> > > * v3: https://lore.kernel.org/all/cover.1748335606.git.legion@kernel.org/
> > >
> > > - v3:
> > > * Add `Reviewed-by` tag to patches from Petr Pavlu.
> > > * Rebase to v6.15.
> > > * v2: https://lore.kernel.org/all/20250509164237.2886508-1-legion@kernel.org/
> > >
> > > - v2:
> > > * Drop patch for mfd because it was already applied and is in linux-next.
> > > * The generation of aliases for builtin modules has been redone as
> > > suggested by Masahiro Yamada.
> > > * Rebase to v6.15-rc5-136-g9c69f8884904
> > > * v1: https://lore.kernel.org/all/cover.1745591072.git.legion@kernel.org/
> > >
> > >
> > > Alexey Gladkov (3):
> > > scsi: Always define blogic_pci_tbl structure
> > > modpost: Add modname to mod_device_table alias
> > > modpost: Create modalias for builtin modules
> > >
> > > Masahiro Yamada (4):
> > > module: remove meaningless 'name' parameter from __MODULE_INFO()
> > > kbuild: always create intermediate vmlinux.unstripped
> > > kbuild: keep .modinfo section in vmlinux.unstripped
> > > kbuild: extract modules.builtin.modinfo from vmlinux.unstripped
> > >
> > > drivers/scsi/BusLogic.c | 2 -
> > > include/asm-generic/vmlinux.lds.h | 2 +-
> > > include/crypto/algapi.h | 4 +-
> > > include/linux/module.h | 21 ++++-----
> > > include/linux/moduleparam.h | 9 ++--
> > > include/net/tcp.h | 4 +-
> > > rust/kernel/device_id.rs | 8 ++--
> > > scripts/Makefile.vmlinux | 74 +++++++++++++++++++++----------
> > > scripts/Makefile.vmlinux_o | 26 +----------
> > > scripts/mksysmap | 6 +++
> > > scripts/mod/file2alias.c | 34 ++++++++++++--
> > > scripts/mod/modpost.c | 17 ++++++-
> > > scripts/mod/modpost.h | 2 +
> > > 13 files changed, 131 insertions(+), 78 deletions(-)
> > >
> > > --
> > > 2.49.0
> > >
> >
> > --
> > Rgrds, legion
> >
>
>
> --
> Best Regards
> Masahiro Yamada
>
--
Rgrds, legion
^ permalink raw reply
* Re: [PATCH v3] module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES
From: Daniel Gomez @ 2025-07-22 9:08 UTC (permalink / raw)
To: Greg Kroah-Hartman, Vlastimil Babka, Christian Brauner
Cc: Jiri Slaby (SUSE), Stephen Rothwell, Daniel Gomez,
Matthias Maennich, Jonathan Corbet, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Viro, Jan Kara, Christoph Hellwig, Peter Zijlstra,
David Hildenbrand, Shivank Garg, linux-doc, linux-kernel,
linux-modules, linux-kbuild, linux-fsdevel
In-Reply-To: <2025072246-unexpired-deletion-a0f8@gregkh>
On 22/07/2025 10.53, Greg Kroah-Hartman wrote:
> On Tue, Jul 22, 2025 at 10:49:48AM +0200, Vlastimil Babka wrote:
>> On 7/22/25 10:46, Greg Kroah-Hartman wrote:
>>> On Tue, Jul 22, 2025 at 10:26:43AM +0200, Daniel Gomez wrote:
>>>>>
>>>>> Maybe with no reply, you can queue it then?
>>>>
>>>> + Jiri, Stephen and Greg, added to the To: list.
>>>>
>>>> EXPORT_SYMBOL_GPL_FOR_MODULES macro was merged [1] through Masahiro's
>>>> pull request in v6.16-rc1. This patch from Vlastimil renames the macro to
>>>> EXPORT_SYMBOL_FOR_MODULES. This means Jiri's patch b20d6576cdb3 "serial: 8250:
>>>> export RSA functions" will need to be updated accordingly. I'd like like to
>>>> know how you prefer to proceed, since it was requested to have this merged as a
>>>> fix before Linus releases a new kernel with the former name.
>>>
>>> So you want this in 6.16-final? Ok, do so and then someone needs to fix
>>> up the build breakage in linux-next and in all of the pull requests to
>>> Linus for 6.17-rc1 :)
>>>
I see... that doesn't sound like it was the right approach. I didn't expect
follow-up fixes to be needed for the next merge window. Thanks for the heads-up.
I'll hold off on merging this for now.
>>>> Link: https://lore.kernel.org/all/CAK7LNAQunzxOHR+vMZLf8kqxyRtLx-Z2G2VZquJmndrT9TZjiQ@mail.gmail.com/ [1]
>>>>
>>>>
>>>> Masahiro, just a heads-up that I plan to merge this through the linux-modules
>>>> tree unless you advise otherwise.
>>>
>>> Why not just do the rename after 6.17-rc1 is out? That way all new
>>> users will be able to be caught at that point in time. There's no issue
>>
>> Hm there might be people basing their new exports for 6.18 on 6.17-rc1. They
>> would have to be told to use rc2 then.
>
> Yes, that's normal, nothing wrong with that at all, we make api name
> changes across the tree quite often (i.e. almost every-other release.)
>
>> Maybe the best way would be if Linus
>> did this just before tagging rc1, while fixing up all users merged during
>> the merge window?
>
> Again, what's wrong with -rc2? Anyone caught using this on only -rc1
> will get a quick "this broke the build" report in linux-next so it's not
> like this is going to be unnoticed at all.
I think Christian had some renaming candidates. Christian, does this work for
you?
^ permalink raw reply
* Re: [PATCH v3] module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES
From: Greg Kroah-Hartman @ 2025-07-22 8:53 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Daniel Gomez, Jiri Slaby (SUSE), Stephen Rothwell, Daniel Gomez,
Matthias Maennich, Jonathan Corbet, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Viro, Christian Brauner, Jan Kara, Christoph Hellwig,
Peter Zijlstra, David Hildenbrand, Shivank Garg, linux-doc,
linux-kernel, linux-modules, linux-kbuild, linux-fsdevel
In-Reply-To: <9d61a747-2655-4f4c-a8fe-5db51ff33ff7@suse.cz>
On Tue, Jul 22, 2025 at 10:49:48AM +0200, Vlastimil Babka wrote:
> On 7/22/25 10:46, Greg Kroah-Hartman wrote:
> > On Tue, Jul 22, 2025 at 10:26:43AM +0200, Daniel Gomez wrote:
> >> >
> >> > Maybe with no reply, you can queue it then?
> >>
> >> + Jiri, Stephen and Greg, added to the To: list.
> >>
> >> EXPORT_SYMBOL_GPL_FOR_MODULES macro was merged [1] through Masahiro's
> >> pull request in v6.16-rc1. This patch from Vlastimil renames the macro to
> >> EXPORT_SYMBOL_FOR_MODULES. This means Jiri's patch b20d6576cdb3 "serial: 8250:
> >> export RSA functions" will need to be updated accordingly. I'd like like to
> >> know how you prefer to proceed, since it was requested to have this merged as a
> >> fix before Linus releases a new kernel with the former name.
> >
> > So you want this in 6.16-final? Ok, do so and then someone needs to fix
> > up the build breakage in linux-next and in all of the pull requests to
> > Linus for 6.17-rc1 :)
> >
> >> Link: https://lore.kernel.org/all/CAK7LNAQunzxOHR+vMZLf8kqxyRtLx-Z2G2VZquJmndrT9TZjiQ@mail.gmail.com/ [1]
> >>
> >>
> >> Masahiro, just a heads-up that I plan to merge this through the linux-modules
> >> tree unless you advise otherwise.
> >
> > Why not just do the rename after 6.17-rc1 is out? That way all new
> > users will be able to be caught at that point in time. There's no issue
>
> Hm there might be people basing their new exports for 6.18 on 6.17-rc1. They
> would have to be told to use rc2 then.
Yes, that's normal, nothing wrong with that at all, we make api name
changes across the tree quite often (i.e. almost every-other release.)
> Maybe the best way would be if Linus
> did this just before tagging rc1, while fixing up all users merged during
> the merge window?
Again, what's wrong with -rc2? Anyone caught using this on only -rc1
will get a quick "this broke the build" report in linux-next so it's not
like this is going to be unnoticed at all.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3] module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES
From: Vlastimil Babka @ 2025-07-22 8:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Daniel Gomez
Cc: Jiri Slaby (SUSE), Stephen Rothwell, Daniel Gomez,
Matthias Maennich, Jonathan Corbet, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Viro, Christian Brauner, Jan Kara, Christoph Hellwig,
Peter Zijlstra, David Hildenbrand, Shivank Garg, linux-doc,
linux-kernel, linux-modules, linux-kbuild, linux-fsdevel
In-Reply-To: <2025072219-dollhouse-margarita-de67@gregkh>
On 7/22/25 10:46, Greg Kroah-Hartman wrote:
> On Tue, Jul 22, 2025 at 10:26:43AM +0200, Daniel Gomez wrote:
>> >
>> > Maybe with no reply, you can queue it then?
>>
>> + Jiri, Stephen and Greg, added to the To: list.
>>
>> EXPORT_SYMBOL_GPL_FOR_MODULES macro was merged [1] through Masahiro's
>> pull request in v6.16-rc1. This patch from Vlastimil renames the macro to
>> EXPORT_SYMBOL_FOR_MODULES. This means Jiri's patch b20d6576cdb3 "serial: 8250:
>> export RSA functions" will need to be updated accordingly. I'd like like to
>> know how you prefer to proceed, since it was requested to have this merged as a
>> fix before Linus releases a new kernel with the former name.
>
> So you want this in 6.16-final? Ok, do so and then someone needs to fix
> up the build breakage in linux-next and in all of the pull requests to
> Linus for 6.17-rc1 :)
>
>> Link: https://lore.kernel.org/all/CAK7LNAQunzxOHR+vMZLf8kqxyRtLx-Z2G2VZquJmndrT9TZjiQ@mail.gmail.com/ [1]
>>
>>
>> Masahiro, just a heads-up that I plan to merge this through the linux-modules
>> tree unless you advise otherwise.
>
> Why not just do the rename after 6.17-rc1 is out? That way all new
> users will be able to be caught at that point in time. There's no issue
Hm there might be people basing their new exports for 6.18 on 6.17-rc1. They
would have to be told to use rc2 then. Maybe the best way would be if Linus
did this just before tagging rc1, while fixing up all users merged during
the merge window?
> with the name being as it is for 6.16-final that I can determine, right?
>
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [PATCH v3] module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES
From: Greg Kroah-Hartman @ 2025-07-22 8:46 UTC (permalink / raw)
To: Daniel Gomez
Cc: Jiri Slaby (SUSE), Stephen Rothwell, Vlastimil Babka,
Daniel Gomez, Matthias Maennich, Jonathan Corbet,
Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier, Alexander Viro,
Christian Brauner, Jan Kara, Christoph Hellwig, Peter Zijlstra,
David Hildenbrand, Shivank Garg, linux-doc, linux-kernel,
linux-modules, linux-kbuild, linux-fsdevel
In-Reply-To: <49eeff09-993f-42a0-8e3b-b3f95b41dbcf@kernel.org>
On Tue, Jul 22, 2025 at 10:26:43AM +0200, Daniel Gomez wrote:
> On 21/07/2025 12.40, Vlastimil Babka wrote:
> > On 7/15/25 20:58, Daniel Gomez wrote:
> >> On 15/07/2025 10.43, Vlastimil Babka wrote:
> >>> Christoph suggested that the explicit _GPL_ can be dropped from the
> >>> module namespace export macro, as it's intended for in-tree modules
> >>> only. It would be possible to restrict it technically, but it was
> >>> pointed out [2] that some cases of using an out-of-tree build of an
> >>> in-tree module with the same name are legitimate. But in that case those
> >>> also have to be GPL anyway so it's unnecessary to spell it out in the
> >>> macro name.
> >>>
> >>> Link: https://lore.kernel.org/all/aFleJN_fE-RbSoFD@infradead.org/ [1]
> >>> Link: https://lore.kernel.org/all/CAK7LNATRkZHwJGpojCnvdiaoDnP%2BaeUXgdey5sb_8muzdWTMkA@mail.gmail.com/ [2]
> >>> Suggested-by: Christoph Hellwig <hch@infradead.org>
> >>> Reviewed-by: Shivank Garg <shivankg@amd.com>
> >>> Acked-by: David Hildenbrand <david@redhat.com>
> >>> Acked-by: Nicolas Schier <n.schier@avm.de>
> >>> Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
> >>> Reviewed-by: Christian Brauner <brauner@kernel.org>
> >>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> >>> ---
> >>> Daniel, please clarify if you'll take this via module tree or Christian
> >>> can take it via vfs tree?
> >>
> >> Patch 707f853d7fa3 ("module: Provide EXPORT_SYMBOL_GPL_FOR_MODULES() helper")
> >> from Peter was merged through Masahiro in v6.16-rc1. Since this is a related
> >> fix/rename/cleanup, it'd make sense for it to go through his kbuild tree as
> >> well. Masahiro, please let me know if you'd prefer otherwise. If not, I'll queue
> >> it up in the modules tree.
> >
> > Maybe with no reply, you can queue it then?
>
> + Jiri, Stephen and Greg, added to the To: list.
>
> EXPORT_SYMBOL_GPL_FOR_MODULES macro was merged [1] through Masahiro's
> pull request in v6.16-rc1. This patch from Vlastimil renames the macro to
> EXPORT_SYMBOL_FOR_MODULES. This means Jiri's patch b20d6576cdb3 "serial: 8250:
> export RSA functions" will need to be updated accordingly. I'd like like to
> know how you prefer to proceed, since it was requested to have this merged as a
> fix before Linus releases a new kernel with the former name.
So you want this in 6.16-final? Ok, do so and then someone needs to fix
up the build breakage in linux-next and in all of the pull requests to
Linus for 6.17-rc1 :)
> Link: https://lore.kernel.org/all/CAK7LNAQunzxOHR+vMZLf8kqxyRtLx-Z2G2VZquJmndrT9TZjiQ@mail.gmail.com/ [1]
>
>
> Masahiro, just a heads-up that I plan to merge this through the linux-modules
> tree unless you advise otherwise.
Why not just do the rename after 6.17-rc1 is out? That way all new
users will be able to be caught at that point in time. There's no issue
with the name being as it is for 6.16-final that I can determine, right?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3] module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES
From: Daniel Gomez @ 2025-07-22 8:26 UTC (permalink / raw)
To: Jiri Slaby (SUSE), Stephen Rothwell, Greg Kroah-Hartman,
Vlastimil Babka, Daniel Gomez, Matthias Maennich, Jonathan Corbet,
Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier, Alexander Viro,
Christian Brauner, Jan Kara
Cc: Christoph Hellwig, Peter Zijlstra, David Hildenbrand,
Shivank Garg, linux-doc, linux-kernel, linux-modules,
linux-kbuild, linux-fsdevel
In-Reply-To: <24f995fe-df76-4495-b9c6-9339b6afa6be@suse.cz>
On 21/07/2025 12.40, Vlastimil Babka wrote:
> On 7/15/25 20:58, Daniel Gomez wrote:
>> On 15/07/2025 10.43, Vlastimil Babka wrote:
>>> Christoph suggested that the explicit _GPL_ can be dropped from the
>>> module namespace export macro, as it's intended for in-tree modules
>>> only. It would be possible to restrict it technically, but it was
>>> pointed out [2] that some cases of using an out-of-tree build of an
>>> in-tree module with the same name are legitimate. But in that case those
>>> also have to be GPL anyway so it's unnecessary to spell it out in the
>>> macro name.
>>>
>>> Link: https://lore.kernel.org/all/aFleJN_fE-RbSoFD@infradead.org/ [1]
>>> Link: https://lore.kernel.org/all/CAK7LNATRkZHwJGpojCnvdiaoDnP%2BaeUXgdey5sb_8muzdWTMkA@mail.gmail.com/ [2]
>>> Suggested-by: Christoph Hellwig <hch@infradead.org>
>>> Reviewed-by: Shivank Garg <shivankg@amd.com>
>>> Acked-by: David Hildenbrand <david@redhat.com>
>>> Acked-by: Nicolas Schier <n.schier@avm.de>
>>> Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
>>> Reviewed-by: Christian Brauner <brauner@kernel.org>
>>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>>> ---
>>> Daniel, please clarify if you'll take this via module tree or Christian
>>> can take it via vfs tree?
>>
>> Patch 707f853d7fa3 ("module: Provide EXPORT_SYMBOL_GPL_FOR_MODULES() helper")
>> from Peter was merged through Masahiro in v6.16-rc1. Since this is a related
>> fix/rename/cleanup, it'd make sense for it to go through his kbuild tree as
>> well. Masahiro, please let me know if you'd prefer otherwise. If not, I'll queue
>> it up in the modules tree.
>
> Maybe with no reply, you can queue it then?
+ Jiri, Stephen and Greg, added to the To: list.
EXPORT_SYMBOL_GPL_FOR_MODULES macro was merged [1] through Masahiro's
pull request in v6.16-rc1. This patch from Vlastimil renames the macro to
EXPORT_SYMBOL_FOR_MODULES. This means Jiri's patch b20d6576cdb3 "serial: 8250:
export RSA functions" will need to be updated accordingly. I'd like like to
know how you prefer to proceed, since it was requested to have this merged as a
fix before Linus releases a new kernel with the former name.
Link: https://lore.kernel.org/all/CAK7LNAQunzxOHR+vMZLf8kqxyRtLx-Z2G2VZquJmndrT9TZjiQ@mail.gmail.com/ [1]
Masahiro, just a heads-up that I plan to merge this through the linux-modules
tree unless you advise otherwise.
^ permalink raw reply
* Re: [PATCH] module: pr_debug when there is no version info
From: Petr Pavlu @ 2025-07-22 8:25 UTC (permalink / raw)
To: Wang Jinchao
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-kernel,
linux-modules
In-Reply-To: <86062810-ff6b-4181-83b7-dfe443ff4012@gmail.com>
On 7/22/25 5:08 AM, Wang Jinchao wrote:
> On 7/21/25 22:40, Petr Pavlu wrote:
>> On 7/21/25 6:52 AM, Wang Jinchao wrote:
>>> When there is no version information, modprobe and insmod only
>>> report "invalid format".
>>> Print the actual cause to make it easier to diagnose the issue.
>>> This helps developers quickly identify version-related module
>>> loading failures.
>>> Signed-off-by: Wang Jinchao <wangjinchao600@gmail.com>
>>> ---
>>> kernel/module/version.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/kernel/module/version.c b/kernel/module/version.c
>>> index 2beefeba82d9..bc28c697ff3a 100644
>>> --- a/kernel/module/version.c
>>> +++ b/kernel/module/version.c
>>> @@ -42,8 +42,10 @@ int check_version(const struct load_info *info,
>>> }
>>> /* No versions at all? modprobe --force does this. */
>>> - if (versindex == 0)
>>> + if (versindex == 0) {
>>> + pr_debug("No version info for module %s\n", info->name);
>>> return try_to_force_load(mod, symname) == 0;
>>> + }
>>> versions = (void *)sechdrs[versindex].sh_addr;
>>> num_versions = sechdrs[versindex].sh_size
>>
>> I think it would be better to instead improve the behavior of
>> try_to_force_load(). The function should print the error reason prior to
>> returning -ENOEXEC. This would also help its two other callers,
>> check_modinfo() and check_export_symbol_versions().
>>
>> Additionally, I suggest moving the check to ensure version information
>> is available for imported symbols earlier in the loading process.
>> A suitable place might be check_modstruct_version(). This way the check
>> is performed only once per module.
>>
>> The following is a prototype patch:
>>
>> diff --git a/kernel/module/main.c b/kernel/module/main.c
>> index c2c08007029d..c1ccd343e8c3 100644
>> --- a/kernel/module/main.c
>> +++ b/kernel/module/main.c
>> @@ -1053,6 +1053,7 @@ int try_to_force_load(struct module *mod, const char *reason)
>> add_taint_module(mod, TAINT_FORCED_MODULE, LOCKDEP_NOW_UNRELIABLE);
>> return 0;
>> #else
>> + pr_err("%s: %s\n", mod->name, reason);
>> return -ENOEXEC;
>> #endif
>> }
>> diff --git a/kernel/module/version.c b/kernel/module/version.c
>> index 2beefeba82d9..4d9ebf0834de 100644
>> --- a/kernel/module/version.c
>> +++ b/kernel/module/version.c
>> @@ -41,9 +41,9 @@ int check_version(const struct load_info *info,
>> return 1;
>> }
>> - /* No versions at all? modprobe --force does this. */
>> + /* No versions? Ok, already tainted in check_modstruct_version(). */
>> if (versindex == 0)
>> - return try_to_force_load(mod, symname) == 0;
>> + return 1;
>> versions = (void *)sechdrs[versindex].sh_addr;
>> num_versions = sechdrs[versindex].sh_size
>> @@ -90,6 +90,11 @@ int check_modstruct_version(const struct load_info *info,
>> have_symbol = find_symbol(&fsa);
>> BUG_ON(!have_symbol);
>> + /* No versions at all? modprobe --force does this. */
>> + if (!info->index.vers && !info->index.vers_ext_crc)
>> + return try_to_force_load(
>> + mod, "no versions for imported symbols") == 0;
>> +
>> return check_version(info, "module_layout", mod, fsa.crc);
>> }
>>
>> As a side note, I'm confused why with CONFIG_MODULE_FORCE_LOAD=y, the
>> code treats missing modversions for imported symbols as ok, even without
>> MODULE_INIT_IGNORE_MODVERSIONS. This is at least consistent with the
>> handling of missing vermagic, but it seems this behavior should be
>> stricter.
>>
> When debugging syzkaller, I noticed that the insmod command always reports errors. However, to get the exact information, I need to trace the kernel, so I casually submitted this patch.
>
> Based on your response, I also feel that the meaning of force_load here is somewhat unclear. It would be better to create a mask or a clear list to indicate which fields can be forced and which cannot. Once this is clear, we can create a function named may_force_check().
I cannot find an explicit reason in the Git history why a missing
vermagic is treated as if the module was loaded with
MODULE_INIT_IGNORE_VERMAGIC, and similarly why missing modversions data
is treated as if the module was loaded with
MODULE_INIT_IGNORE_MODVERSIONS.
I would argue that a more sensible behavior would be to consider
a missing vermagic as an error and allow loading the module only if
MODULE_INIT_IGNORE_VERMAGIC is explicitly specified. And analogously for
missing modversions and MODULE_INIT_IGNORE_MODVERSIONS.
Nonetheless, if I understand correctly, this should be mostly separate
from your issue.
>
> In addition, check_modstruct_version also calls check_version to handle tainting. So there's a minor issue with the logic in your example patch.
>
I'm not sure I follow here. My example lifts the try_to_force_load()
call from check_version() to check_modstruct_version(), and should still
result in tainting the kernel.
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH] module: pr_debug when there is no version info
From: Wang Jinchao @ 2025-07-22 3:08 UTC (permalink / raw)
To: Petr Pavlu
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-kernel,
linux-modules
In-Reply-To: <3992b57d-3d8b-4d60-bc4a-f227f712dcca@suse.com>
On 7/21/25 22:40, Petr Pavlu wrote:
> On 7/21/25 6:52 AM, Wang Jinchao wrote:
>> When there is no version information, modprobe and insmod only
>> report "invalid format".
>> Print the actual cause to make it easier to diagnose the issue.
>> This helps developers quickly identify version-related module
>> loading failures.
>> Signed-off-by: Wang Jinchao <wangjinchao600@gmail.com>
>> ---
>> kernel/module/version.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/module/version.c b/kernel/module/version.c
>> index 2beefeba82d9..bc28c697ff3a 100644
>> --- a/kernel/module/version.c
>> +++ b/kernel/module/version.c
>> @@ -42,8 +42,10 @@ int check_version(const struct load_info *info,
>> }
>>
>> /* No versions at all? modprobe --force does this. */
>> - if (versindex == 0)
>> + if (versindex == 0) {
>> + pr_debug("No version info for module %s\n", info->name);
>> return try_to_force_load(mod, symname) == 0;
>> + }
>>
>> versions = (void *)sechdrs[versindex].sh_addr;
>> num_versions = sechdrs[versindex].sh_size
>
> I think it would be better to instead improve the behavior of
> try_to_force_load(). The function should print the error reason prior to
> returning -ENOEXEC. This would also help its two other callers,
> check_modinfo() and check_export_symbol_versions().
>
> Additionally, I suggest moving the check to ensure version information
> is available for imported symbols earlier in the loading process.
> A suitable place might be check_modstruct_version(). This way the check
> is performed only once per module.
>
> The following is a prototype patch:
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c2c08007029d..c1ccd343e8c3 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -1053,6 +1053,7 @@ int try_to_force_load(struct module *mod, const char *reason)
> add_taint_module(mod, TAINT_FORCED_MODULE, LOCKDEP_NOW_UNRELIABLE);
> return 0;
> #else
> + pr_err("%s: %s\n", mod->name, reason);
> return -ENOEXEC;
> #endif
> }
> diff --git a/kernel/module/version.c b/kernel/module/version.c
> index 2beefeba82d9..4d9ebf0834de 100644
> --- a/kernel/module/version.c
> +++ b/kernel/module/version.c
> @@ -41,9 +41,9 @@ int check_version(const struct load_info *info,
> return 1;
> }
>
> - /* No versions at all? modprobe --force does this. */
> + /* No versions? Ok, already tainted in check_modstruct_version(). */
> if (versindex == 0)
> - return try_to_force_load(mod, symname) == 0;
> + return 1;
>
> versions = (void *)sechdrs[versindex].sh_addr;
> num_versions = sechdrs[versindex].sh_size
> @@ -90,6 +90,11 @@ int check_modstruct_version(const struct load_info *info,
> have_symbol = find_symbol(&fsa);
> BUG_ON(!have_symbol);
>
> + /* No versions at all? modprobe --force does this. */
> + if (!info->index.vers && !info->index.vers_ext_crc)
> + return try_to_force_load(
> + mod, "no versions for imported symbols") == 0;
> +
> return check_version(info, "module_layout", mod, fsa.crc);
> }
>
>
> As a side note, I'm confused why with CONFIG_MODULE_FORCE_LOAD=y, the
> code treats missing modversions for imported symbols as ok, even without
> MODULE_INIT_IGNORE_MODVERSIONS. This is at least consistent with the
> handling of missing vermagic, but it seems this behavior should be
> stricter.
>
When debugging syzkaller, I noticed that the insmod command always
reports errors. However, to get the exact information, I need to trace
the kernel, so I casually submitted this patch.
Based on your response, I also feel that the meaning of force_load here
is somewhat unclear. It would be better to create a mask or a clear list
to indicate which fields can be forced and which cannot. Once this is
clear, we can create a function named may_force_check().
In addition, check_modstruct_version also calls check_version to handle
tainting. So there's a minor issue with the logic in your example patch.
--
Best regards,
Jinchao
^ permalink raw reply
* Re: [PATCH] module: pr_debug when there is no version info
From: Petr Pavlu @ 2025-07-21 14:40 UTC (permalink / raw)
To: Wang Jinchao
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-kernel,
linux-modules
In-Reply-To: <20250721045224.391745-1-wangjinchao600@gmail.com>
On 7/21/25 6:52 AM, Wang Jinchao wrote:
> When there is no version information, modprobe and insmod only
> report "invalid format".
> Print the actual cause to make it easier to diagnose the issue.
> This helps developers quickly identify version-related module
> loading failures.
> Signed-off-by: Wang Jinchao <wangjinchao600@gmail.com>
> ---
> kernel/module/version.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/module/version.c b/kernel/module/version.c
> index 2beefeba82d9..bc28c697ff3a 100644
> --- a/kernel/module/version.c
> +++ b/kernel/module/version.c
> @@ -42,8 +42,10 @@ int check_version(const struct load_info *info,
> }
>
> /* No versions at all? modprobe --force does this. */
> - if (versindex == 0)
> + if (versindex == 0) {
> + pr_debug("No version info for module %s\n", info->name);
> return try_to_force_load(mod, symname) == 0;
> + }
>
> versions = (void *)sechdrs[versindex].sh_addr;
> num_versions = sechdrs[versindex].sh_size
I think it would be better to instead improve the behavior of
try_to_force_load(). The function should print the error reason prior to
returning -ENOEXEC. This would also help its two other callers,
check_modinfo() and check_export_symbol_versions().
Additionally, I suggest moving the check to ensure version information
is available for imported symbols earlier in the loading process.
A suitable place might be check_modstruct_version(). This way the check
is performed only once per module.
The following is a prototype patch:
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c2c08007029d..c1ccd343e8c3 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1053,6 +1053,7 @@ int try_to_force_load(struct module *mod, const char *reason)
add_taint_module(mod, TAINT_FORCED_MODULE, LOCKDEP_NOW_UNRELIABLE);
return 0;
#else
+ pr_err("%s: %s\n", mod->name, reason);
return -ENOEXEC;
#endif
}
diff --git a/kernel/module/version.c b/kernel/module/version.c
index 2beefeba82d9..4d9ebf0834de 100644
--- a/kernel/module/version.c
+++ b/kernel/module/version.c
@@ -41,9 +41,9 @@ int check_version(const struct load_info *info,
return 1;
}
- /* No versions at all? modprobe --force does this. */
+ /* No versions? Ok, already tainted in check_modstruct_version(). */
if (versindex == 0)
- return try_to_force_load(mod, symname) == 0;
+ return 1;
versions = (void *)sechdrs[versindex].sh_addr;
num_versions = sechdrs[versindex].sh_size
@@ -90,6 +90,11 @@ int check_modstruct_version(const struct load_info *info,
have_symbol = find_symbol(&fsa);
BUG_ON(!have_symbol);
+ /* No versions at all? modprobe --force does this. */
+ if (!info->index.vers && !info->index.vers_ext_crc)
+ return try_to_force_load(
+ mod, "no versions for imported symbols") == 0;
+
return check_version(info, "module_layout", mod, fsa.crc);
}
As a side note, I'm confused why with CONFIG_MODULE_FORCE_LOAD=y, the
code treats missing modversions for imported symbols as ok, even without
MODULE_INIT_IGNORE_MODVERSIONS. This is at least consistent with the
handling of missing vermagic, but it seems this behavior should be
stricter.
--
Thanks,
Petr
^ permalink raw reply related
* Re: [PATCH v3] module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES
From: Vlastimil Babka @ 2025-07-21 10:40 UTC (permalink / raw)
To: Daniel Gomez, Daniel Gomez, Matthias Maennich, Jonathan Corbet,
Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier, Alexander Viro,
Christian Brauner, Jan Kara
Cc: Christoph Hellwig, Peter Zijlstra, David Hildenbrand,
Shivank Garg, Greg Kroah-Hartman, Jiri Slaby (SUSE),
Stephen Rothwell, linux-doc, linux-kernel, linux-modules,
linux-kbuild, linux-fsdevel
In-Reply-To: <b340eb9f-a336-461c-befe-6b09c68b731e@kernel.org>
On 7/15/25 20:58, Daniel Gomez wrote:
> On 15/07/2025 10.43, Vlastimil Babka wrote:
>> Christoph suggested that the explicit _GPL_ can be dropped from the
>> module namespace export macro, as it's intended for in-tree modules
>> only. It would be possible to restrict it technically, but it was
>> pointed out [2] that some cases of using an out-of-tree build of an
>> in-tree module with the same name are legitimate. But in that case those
>> also have to be GPL anyway so it's unnecessary to spell it out in the
>> macro name.
>>
>> Link: https://lore.kernel.org/all/aFleJN_fE-RbSoFD@infradead.org/ [1]
>> Link: https://lore.kernel.org/all/CAK7LNATRkZHwJGpojCnvdiaoDnP%2BaeUXgdey5sb_8muzdWTMkA@mail.gmail.com/ [2]
>> Suggested-by: Christoph Hellwig <hch@infradead.org>
>> Reviewed-by: Shivank Garg <shivankg@amd.com>
>> Acked-by: David Hildenbrand <david@redhat.com>
>> Acked-by: Nicolas Schier <n.schier@avm.de>
>> Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
>> Reviewed-by: Christian Brauner <brauner@kernel.org>
>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>> ---
>> Daniel, please clarify if you'll take this via module tree or Christian
>> can take it via vfs tree?
>
> Patch 707f853d7fa3 ("module: Provide EXPORT_SYMBOL_GPL_FOR_MODULES() helper")
> from Peter was merged through Masahiro in v6.16-rc1. Since this is a related
> fix/rename/cleanup, it'd make sense for it to go through his kbuild tree as
> well. Masahiro, please let me know if you'd prefer otherwise. If not, I'll queue
> it up in the modules tree.
Maybe with no reply, you can queue it then?
Thanks,
Vlastimil
^ permalink raw reply
* Re: [PATCH 3/5] module: Restore the moduleparam prefix length check
From: Petr Pavlu @ 2025-07-21 9:21 UTC (permalink / raw)
To: Daniel Gomez
Cc: Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-modules,
linux-kernel
In-Reply-To: <15f52f4c-7809-46ab-9e13-bd487f35a80c@kernel.org>
On 7/17/25 9:23 PM, Daniel Gomez wrote:
> On 30/06/2025 16.32, Petr Pavlu wrote:
>> The moduleparam code allows modules to provide their own definition of
>> MODULE_PARAM_PREFIX, instead of using the default KBUILD_MODNAME ".".
>>
>> Commit 730b69d22525 ("module: check kernel param length at compile time,
>> not runtime") added a check to ensure the prefix doesn't exceed
>> MODULE_NAME_LEN, as this is what param_sysfs_builtin() expects.
>>
>> Later, commit 58f86cc89c33 ("VERIFY_OCTAL_PERMISSIONS: stricter checking
>> for sysfs perms.") removed this check, but there is no indication this was
>> intentional.
>>
>> Since the check is still useful for param_sysfs_builtin() to function
>> properly, reintroduce it in __module_param_call(), but in a modernized form
>> using static_assert().
>>
>> While here, clean up the __module_param_call() comments. In particular,
>> remove the comment "Default value instead of permissions?", which comes
>> from commit 9774a1f54f17 ("[PATCH] Compile-time check re world-writeable
>> module params"). This comment was related to the test variable
>> __param_perm_check_##name, which was removed in the previously mentioned
>> commit 58f86cc89c33.
>>
>> Fixes: 58f86cc89c33 ("VERIFY_OCTAL_PERMISSIONS: stricter checking for sysfs perms.")
>> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
>> ---
>> include/linux/moduleparam.h | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
>> index bfb85fd13e1f..110e9d09de24 100644
>> --- a/include/linux/moduleparam.h
>> +++ b/include/linux/moduleparam.h
>> @@ -282,10 +282,9 @@ struct kparam_array
>> #define __moduleparam_const const
>> #endif
>>
>> -/* This is the fundamental function for registering boot/module
>> - parameters. */
>> +/* This is the fundamental function for registering boot/module parameters. */
>> #define __module_param_call(prefix, name, ops, arg, perm, level, flags) \
>> - /* Default value instead of permissions? */ \
>> + static_assert(sizeof(""prefix) - 1 <= MAX_PARAM_PREFIX_LEN); \
>
> Can you clarify if -1 to remove the dot from prefix?
>
> Final code
> static_assert(sizeof(""prefix) - 1 <= __MODULE_NAME_LEN); \
>
> with __MODULE_NAME_LEN being:
>
> #define __MODULE_NAME_LEN (64 - sizeof(unsigned long))
Correct, -1 is to account for the dot at the end of the prefix.
I actually also wanted to assert that the prefix ends with a dot, but
unfortunately prefix[sizeof(prefix)-2] (with prefix being a string
literal) is not a constant expression in C.
--
Thanks,
Petr
^ permalink raw reply
* [PATCH] module: pr_debug when there is no version info
From: Wang Jinchao @ 2025-07-21 4:52 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: linux-kernel, Wang Jinchao, linux-modules
When there is no version information, modprobe and insmod only
report "invalid format".
Print the actual cause to make it easier to diagnose the issue.
This helps developers quickly identify version-related module
loading failures.
Signed-off-by: Wang Jinchao <wangjinchao600@gmail.com>
---
kernel/module/version.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/module/version.c b/kernel/module/version.c
index 2beefeba82d9..bc28c697ff3a 100644
--- a/kernel/module/version.c
+++ b/kernel/module/version.c
@@ -42,8 +42,10 @@ int check_version(const struct load_info *info,
}
/* No versions at all? modprobe --force does this. */
- if (versindex == 0)
+ if (versindex == 0) {
+ pr_debug("No version info for module %s\n", info->name);
return try_to_force_load(mod, symname) == 0;
+ }
versions = (void *)sechdrs[versindex].sh_addr;
num_versions = sechdrs[versindex].sh_size
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 3/5] module: Restore the moduleparam prefix length check
From: Daniel Gomez @ 2025-07-17 19:23 UTC (permalink / raw)
To: Petr Pavlu, Luis Chamberlain, Sami Tolvanen, Daniel Gomez
Cc: linux-modules, linux-kernel
In-Reply-To: <20250630143535.267745-4-petr.pavlu@suse.com>
On 30/06/2025 16.32, Petr Pavlu wrote:
> The moduleparam code allows modules to provide their own definition of
> MODULE_PARAM_PREFIX, instead of using the default KBUILD_MODNAME ".".
>
> Commit 730b69d22525 ("module: check kernel param length at compile time,
> not runtime") added a check to ensure the prefix doesn't exceed
> MODULE_NAME_LEN, as this is what param_sysfs_builtin() expects.
>
> Later, commit 58f86cc89c33 ("VERIFY_OCTAL_PERMISSIONS: stricter checking
> for sysfs perms.") removed this check, but there is no indication this was
> intentional.
>
> Since the check is still useful for param_sysfs_builtin() to function
> properly, reintroduce it in __module_param_call(), but in a modernized form
> using static_assert().
>
> While here, clean up the __module_param_call() comments. In particular,
> remove the comment "Default value instead of permissions?", which comes
> from commit 9774a1f54f17 ("[PATCH] Compile-time check re world-writeable
> module params"). This comment was related to the test variable
> __param_perm_check_##name, which was removed in the previously mentioned
> commit 58f86cc89c33.
>
> Fixes: 58f86cc89c33 ("VERIFY_OCTAL_PERMISSIONS: stricter checking for sysfs perms.")
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> include/linux/moduleparam.h | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index bfb85fd13e1f..110e9d09de24 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -282,10 +282,9 @@ struct kparam_array
> #define __moduleparam_const const
> #endif
>
> -/* This is the fundamental function for registering boot/module
> - parameters. */
> +/* This is the fundamental function for registering boot/module parameters. */
> #define __module_param_call(prefix, name, ops, arg, perm, level, flags) \
> - /* Default value instead of permissions? */ \
> + static_assert(sizeof(""prefix) - 1 <= MAX_PARAM_PREFIX_LEN); \
Can you clarify if -1 to remove the dot from prefix?
Final code
static_assert(sizeof(""prefix) - 1 <= __MODULE_NAME_LEN); \
with __MODULE_NAME_LEN being:
#define __MODULE_NAME_LEN (64 - sizeof(unsigned long))
> static const char __param_str_##name[] = prefix #name; \
> static struct kernel_param __moduleparam_const __param_##name \
> __used __section("__param") \
^ permalink raw reply
* Re: [PATCH 2/5] module: Remove unnecessary +1 from last_unloaded_module::name size
From: Daniel Gomez @ 2025-07-16 19:49 UTC (permalink / raw)
To: Petr Pavlu, Luis Chamberlain, Sami Tolvanen, Daniel Gomez
Cc: linux-modules, linux-kernel
In-Reply-To: <20250630143535.267745-3-petr.pavlu@suse.com>
On 30/06/2025 16.32, Petr Pavlu wrote:
> The variable last_unloaded_module::name tracks the name of the last
> unloaded module. It is a string copy of module::name, which is
> MODULE_NAME_LEN bytes in size and includes the NUL terminator. Therefore,
> the size of last_unloaded_module::name can also be just MODULE_NAME_LEN,
> without the need for an extra byte.
>
> Fixes: e14af7eeb47e ("debug: track and print last unloaded module in the oops trace")
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> kernel/module/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 933a9854cb7d..04173543639c 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -580,7 +580,7 @@ MODINFO_ATTR(version);
> MODINFO_ATTR(srcversion);
>
> static struct {
> - char name[MODULE_NAME_LEN + 1];
> + char name[MODULE_NAME_LEN];
> char taints[MODULE_FLAGS_BUF_SIZE];
> } last_unloaded_module;
>
LGTM,
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
^ permalink raw reply
* Re: [PATCH 1/5] module: Prevent silent truncation of module name in delete_module(2)
From: Daniel Gomez @ 2025-07-16 19:47 UTC (permalink / raw)
To: Petr Pavlu, Luis Chamberlain, Sami Tolvanen, Daniel Gomez
Cc: linux-modules, linux-kernel
In-Reply-To: <20250630143535.267745-2-petr.pavlu@suse.com>
On 30/06/2025 16.32, Petr Pavlu wrote:
> Passing a module name longer than MODULE_NAME_LEN to the delete_module
> syscall results in its silent truncation. This really isn't much of
> a problem in practice, but it could theoretically lead to the removal of an
> incorrect module. It is more sensible to return ENAMETOOLONG or ENOENT in
> such a case.
>
> Update the syscall to return ENOENT, as documented in the delete_module(2)
> man page to mean "No module by that name exists." This is appropriate
> because a module with a name
Including the NUL byte...
> longer than MODULE_NAME_LEN cannot be loaded
> in the first place.
>
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> kernel/module/main.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 413ac6ea3702..933a9854cb7d 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -751,14 +751,16 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
> struct module *mod;
> char name[MODULE_NAME_LEN];
> char buf[MODULE_FLAGS_BUF_SIZE];
> - int ret, forced = 0;
> + int ret, len, forced = 0;
>
> if (!capable(CAP_SYS_MODULE) || modules_disabled)
> return -EPERM;
>
> - if (strncpy_from_user(name, name_user, MODULE_NAME_LEN-1) < 0)
> - return -EFAULT;
> - name[MODULE_NAME_LEN-1] = '\0';
> + len = strncpy_from_user(name, name_user, MODULE_NAME_LEN);
> + if (len == 0 || len == MODULE_NAME_LEN)
> + return -ENOENT;
> + if (len < 0)
> + return len;
This looks correct to me. The new code not only returns the correct errors
indicated in delete_module(2) but also checks for the case user passes an
empty string and the case where NUL char is not found when copying (with len
== MODULE_NAME_LEN) as well as it's using the correct length (MODULE_NAME_LEN)
for copying.
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
^ permalink raw reply
* Re: [PATCH v4 0/7] Add generated modalias to modules.builtin.modinfo
From: Alexey Gladkov @ 2025-07-16 10:40 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Petr Pavlu, Luis Chamberlain, Sami Tolvanen, Daniel Gomez,
Nathan Chancellor, Nicolas Schier, linux-kernel, linux-modules,
linux-kbuild, linux-scsi
In-Reply-To: <CAK7LNARjC_FCam14RXfTVTQ4_jtXuBKfDsdyG84_k9L1x5zJyg@mail.gmail.com>
On Wed, Jul 16, 2025 at 01:23:26AM +0900, Masahiro Yamada wrote:
> Hi, sorry for the delay.
>
> On Mon, Jul 14, 2025 at 10:41 PM Alexey Gladkov <legion@kernel.org> wrote:
> >
> > On Sat, Jun 21, 2025 at 03:57:12PM +0200, Alexey Gladkov wrote:
> > > The modules.builtin.modinfo file is used by userspace (kmod to be specific) to
> > > get information about builtin modules. Among other information about the module,
> > > information about module aliases is stored. This is very important to determine
> > > that a particular modalias will be handled by a module that is inside the
> > > kernel.
> > >
> > > There are several mechanisms for creating modalias for modules:
> > >
> > > The first is to explicitly specify the MODULE_ALIAS of the macro. In this case,
> > > the aliases go into the '.modinfo' section of the module if it is compiled
> > > separately or into vmlinux.o if it is builtin into the kernel.
> > >
> > > The second is the use of MODULE_DEVICE_TABLE followed by the use of the
> > > modpost utility. In this case, vmlinux.o no longer has this information and
> > > does not get it into modules.builtin.modinfo.
> > >
> > > For example:
> > >
> > > $ modinfo pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30
> > > modinfo: ERROR: Module pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30 not found.
> > >
> > > $ modinfo xhci_pci
> > > name: xhci_pci
> > > filename: (builtin)
> > > license: GPL
> > > file: drivers/usb/host/xhci-pci
> > > description: xHCI PCI Host Controller Driver
> > >
> > > The builtin module is missing alias "pci:v*d*sv*sd*bc0Csc03i30*" which will be
> > > generated by modpost if the module is built separately.
> > >
> > > To fix this it is necessary to add the generated by modpost modalias to
> > > modules.builtin.modinfo.
> > >
> > > Fortunately modpost already generates .vmlinux.export.c for exported symbols. It
> > > is possible to use this file to create a '.modinfo' section for builtin modules.
> > > The modules.builtin.modinfo file becomes a composite file. One part is extracted
> > > from vmlinux.o, the other part from .vmlinux.export.o.
> >
> > Masahiro Yamada, does this version of the patchset look better to you ?
>
>
> Looks better, but this may break s390 build:
>
> https://lore.kernel.org/linux-kbuild/202506062053.zbkFBEnJ-lkp@intel.com/
>
> I have not taken a close look at it.
> If we do not find how to fix the warning, we would
> end up with the original solution.
I haven't gotten any messages from kernel test robot about this message
since these patches were published.
I've pushed the patches to my brunch and will wait for test results until
the weekend. I will see what happens.
> > > Notes:
> > > - v4:
> > > * Rework the patchset based on top of Masahiro Yamada's patches.
> > > * Add removal of unnecessary __mod_device_table__* symbols to avoid symbol
> > > table growth in vmlinux.
> > > * rust code takes into account changes in __mod_device_table__*.
> > > * v3: https://lore.kernel.org/all/cover.1748335606.git.legion@kernel.org/
> > >
> > > - v3:
> > > * Add `Reviewed-by` tag to patches from Petr Pavlu.
> > > * Rebase to v6.15.
> > > * v2: https://lore.kernel.org/all/20250509164237.2886508-1-legion@kernel.org/
> > >
> > > - v2:
> > > * Drop patch for mfd because it was already applied and is in linux-next.
> > > * The generation of aliases for builtin modules has been redone as
> > > suggested by Masahiro Yamada.
> > > * Rebase to v6.15-rc5-136-g9c69f8884904
> > > * v1: https://lore.kernel.org/all/cover.1745591072.git.legion@kernel.org/
> > >
> > >
> > > Alexey Gladkov (3):
> > > scsi: Always define blogic_pci_tbl structure
> > > modpost: Add modname to mod_device_table alias
> > > modpost: Create modalias for builtin modules
> > >
> > > Masahiro Yamada (4):
> > > module: remove meaningless 'name' parameter from __MODULE_INFO()
> > > kbuild: always create intermediate vmlinux.unstripped
> > > kbuild: keep .modinfo section in vmlinux.unstripped
> > > kbuild: extract modules.builtin.modinfo from vmlinux.unstripped
> > >
> > > drivers/scsi/BusLogic.c | 2 -
> > > include/asm-generic/vmlinux.lds.h | 2 +-
> > > include/crypto/algapi.h | 4 +-
> > > include/linux/module.h | 21 ++++-----
> > > include/linux/moduleparam.h | 9 ++--
> > > include/net/tcp.h | 4 +-
> > > rust/kernel/device_id.rs | 8 ++--
> > > scripts/Makefile.vmlinux | 74 +++++++++++++++++++++----------
> > > scripts/Makefile.vmlinux_o | 26 +----------
> > > scripts/mksysmap | 6 +++
> > > scripts/mod/file2alias.c | 34 ++++++++++++--
> > > scripts/mod/modpost.c | 17 ++++++-
> > > scripts/mod/modpost.h | 2 +
> > > 13 files changed, 131 insertions(+), 78 deletions(-)
> > >
> > > --
> > > 2.49.0
> > >
> >
> > --
> > Rgrds, legion
> >
>
>
> --
> Best Regards
> Masahiro Yamada
>
--
Rgrds, legion
^ permalink raw reply
* Re: [PATCH v3] module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES
From: Daniel Gomez @ 2025-07-15 18:58 UTC (permalink / raw)
To: Vlastimil Babka, Daniel Gomez, Matthias Maennich, Jonathan Corbet,
Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier, Alexander Viro,
Christian Brauner, Jan Kara
Cc: Christoph Hellwig, Peter Zijlstra, David Hildenbrand,
Shivank Garg, Greg Kroah-Hartman, Jiri Slaby (SUSE),
Stephen Rothwell, linux-doc, linux-kernel, linux-modules,
linux-kbuild, linux-fsdevel
In-Reply-To: <20250715-export_modules-v3-1-11fffc67dff7@suse.cz>
On 15/07/2025 10.43, Vlastimil Babka wrote:
> Christoph suggested that the explicit _GPL_ can be dropped from the
> module namespace export macro, as it's intended for in-tree modules
> only. It would be possible to restrict it technically, but it was
> pointed out [2] that some cases of using an out-of-tree build of an
> in-tree module with the same name are legitimate. But in that case those
> also have to be GPL anyway so it's unnecessary to spell it out in the
> macro name.
>
> Link: https://lore.kernel.org/all/aFleJN_fE-RbSoFD@infradead.org/ [1]
> Link: https://lore.kernel.org/all/CAK7LNATRkZHwJGpojCnvdiaoDnP%2BaeUXgdey5sb_8muzdWTMkA@mail.gmail.com/ [2]
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Reviewed-by: Shivank Garg <shivankg@amd.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Acked-by: Nicolas Schier <n.schier@avm.de>
> Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
> Reviewed-by: Christian Brauner <brauner@kernel.org>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> Daniel, please clarify if you'll take this via module tree or Christian
> can take it via vfs tree?
Patch 707f853d7fa3 ("module: Provide EXPORT_SYMBOL_GPL_FOR_MODULES() helper")
from Peter was merged through Masahiro in v6.16-rc1. Since this is a related
fix/rename/cleanup, it'd make sense for it to go through his kbuild tree as
well. Masahiro, please let me know if you'd prefer otherwise. If not, I'll queue
it up in the modules tree.
^ permalink raw reply
* Re: [PATCH v4 0/7] Add generated modalias to modules.builtin.modinfo
From: Masahiro Yamada @ 2025-07-15 16:23 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Petr Pavlu, Luis Chamberlain, Sami Tolvanen, Daniel Gomez,
Nathan Chancellor, Nicolas Schier, linux-kernel, linux-modules,
linux-kbuild, linux-scsi
In-Reply-To: <aHUI8KqD0_dtTY3D@example.org>
Hi, sorry for the delay.
On Mon, Jul 14, 2025 at 10:41 PM Alexey Gladkov <legion@kernel.org> wrote:
>
> On Sat, Jun 21, 2025 at 03:57:12PM +0200, Alexey Gladkov wrote:
> > The modules.builtin.modinfo file is used by userspace (kmod to be specific) to
> > get information about builtin modules. Among other information about the module,
> > information about module aliases is stored. This is very important to determine
> > that a particular modalias will be handled by a module that is inside the
> > kernel.
> >
> > There are several mechanisms for creating modalias for modules:
> >
> > The first is to explicitly specify the MODULE_ALIAS of the macro. In this case,
> > the aliases go into the '.modinfo' section of the module if it is compiled
> > separately or into vmlinux.o if it is builtin into the kernel.
> >
> > The second is the use of MODULE_DEVICE_TABLE followed by the use of the
> > modpost utility. In this case, vmlinux.o no longer has this information and
> > does not get it into modules.builtin.modinfo.
> >
> > For example:
> >
> > $ modinfo pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30
> > modinfo: ERROR: Module pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30 not found.
> >
> > $ modinfo xhci_pci
> > name: xhci_pci
> > filename: (builtin)
> > license: GPL
> > file: drivers/usb/host/xhci-pci
> > description: xHCI PCI Host Controller Driver
> >
> > The builtin module is missing alias "pci:v*d*sv*sd*bc0Csc03i30*" which will be
> > generated by modpost if the module is built separately.
> >
> > To fix this it is necessary to add the generated by modpost modalias to
> > modules.builtin.modinfo.
> >
> > Fortunately modpost already generates .vmlinux.export.c for exported symbols. It
> > is possible to use this file to create a '.modinfo' section for builtin modules.
> > The modules.builtin.modinfo file becomes a composite file. One part is extracted
> > from vmlinux.o, the other part from .vmlinux.export.o.
>
> Masahiro Yamada, does this version of the patchset look better to you ?
Looks better, but this may break s390 build:
https://lore.kernel.org/linux-kbuild/202506062053.zbkFBEnJ-lkp@intel.com/
I have not taken a close look at it.
If we do not find how to fix the warning, we would
end up with the original solution.
> > Notes:
> > - v4:
> > * Rework the patchset based on top of Masahiro Yamada's patches.
> > * Add removal of unnecessary __mod_device_table__* symbols to avoid symbol
> > table growth in vmlinux.
> > * rust code takes into account changes in __mod_device_table__*.
> > * v3: https://lore.kernel.org/all/cover.1748335606.git.legion@kernel.org/
> >
> > - v3:
> > * Add `Reviewed-by` tag to patches from Petr Pavlu.
> > * Rebase to v6.15.
> > * v2: https://lore.kernel.org/all/20250509164237.2886508-1-legion@kernel.org/
> >
> > - v2:
> > * Drop patch for mfd because it was already applied and is in linux-next.
> > * The generation of aliases for builtin modules has been redone as
> > suggested by Masahiro Yamada.
> > * Rebase to v6.15-rc5-136-g9c69f8884904
> > * v1: https://lore.kernel.org/all/cover.1745591072.git.legion@kernel.org/
> >
> >
> > Alexey Gladkov (3):
> > scsi: Always define blogic_pci_tbl structure
> > modpost: Add modname to mod_device_table alias
> > modpost: Create modalias for builtin modules
> >
> > Masahiro Yamada (4):
> > module: remove meaningless 'name' parameter from __MODULE_INFO()
> > kbuild: always create intermediate vmlinux.unstripped
> > kbuild: keep .modinfo section in vmlinux.unstripped
> > kbuild: extract modules.builtin.modinfo from vmlinux.unstripped
> >
> > drivers/scsi/BusLogic.c | 2 -
> > include/asm-generic/vmlinux.lds.h | 2 +-
> > include/crypto/algapi.h | 4 +-
> > include/linux/module.h | 21 ++++-----
> > include/linux/moduleparam.h | 9 ++--
> > include/net/tcp.h | 4 +-
> > rust/kernel/device_id.rs | 8 ++--
> > scripts/Makefile.vmlinux | 74 +++++++++++++++++++++----------
> > scripts/Makefile.vmlinux_o | 26 +----------
> > scripts/mksysmap | 6 +++
> > scripts/mod/file2alias.c | 34 ++++++++++++--
> > scripts/mod/modpost.c | 17 ++++++-
> > scripts/mod/modpost.h | 2 +
> > 13 files changed, 131 insertions(+), 78 deletions(-)
> >
> > --
> > 2.49.0
> >
>
> --
> Rgrds, legion
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* [PATCH v3] module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES
From: Vlastimil Babka @ 2025-07-15 8:43 UTC (permalink / raw)
To: Daniel Gomez, Matthias Maennich, Jonathan Corbet,
Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Masahiro Yamada,
Nathan Chancellor, Nicolas Schier, Alexander Viro,
Christian Brauner, Jan Kara
Cc: Christoph Hellwig, Peter Zijlstra, David Hildenbrand,
Shivank Garg, Greg Kroah-Hartman, Jiri Slaby (SUSE),
Stephen Rothwell, linux-doc, linux-kernel, linux-modules,
linux-kbuild, linux-fsdevel, Vlastimil Babka, Nicolas Schier
Christoph suggested that the explicit _GPL_ can be dropped from the
module namespace export macro, as it's intended for in-tree modules
only. It would be possible to restrict it technically, but it was
pointed out [2] that some cases of using an out-of-tree build of an
in-tree module with the same name are legitimate. But in that case those
also have to be GPL anyway so it's unnecessary to spell it out in the
macro name.
Link: https://lore.kernel.org/all/aFleJN_fE-RbSoFD@infradead.org/ [1]
Link: https://lore.kernel.org/all/CAK7LNATRkZHwJGpojCnvdiaoDnP%2BaeUXgdey5sb_8muzdWTMkA@mail.gmail.com/ [2]
Suggested-by: Christoph Hellwig <hch@infradead.org>
Reviewed-by: Shivank Garg <shivankg@amd.com>
Acked-by: David Hildenbrand <david@redhat.com>
Acked-by: Nicolas Schier <n.schier@avm.de>
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
Daniel, please clarify if you'll take this via module tree or Christian
can take it via vfs tree?
Christian asked [1] for EXPORT_SYMBOL_FOR_MODULES() without the _GPL_
part to avoid controversy converting selected existing EXPORT_SYMBOL().
Christoph argued [2] that the _FOR_MODULES() export is intended for
in-tree modules and thus GPL is implied anyway and can be simply dropped
from the export macro name. Peter agreed [3] about the intention for
in-tree modules only, although nothing currently enforces it.
It seemed straightforward to add this enforcement, so v1 did that. But
there were concerns of breaking the (apparently legitimate) usecases of
loading an updated/development out of tree built version of an in-tree
module.
So leave out the enforcement part and just drop the _GPL_ from the
export macro name and so we're left with EXPORT_SYMBOL_FOR_MODULES()
only. Any in-tree module used in an out-of-tree way will have to be GPL
anyway by definition.
Current -next has some new instances of EXPORT_SYMBOL_GPL_FOR_MODULES()
in drivers/tty/serial/8250/8250_rsa.c by commit b20d6576cdb3 ("serial:
8250: export RSA functions"). Hopefully it's resolvable by a merge
commit fixup and we don't need to provide a temporary alias.
[1] https://lore.kernel.org/all/20250623-warmwasser-giftig-ff656fce89ad@brauner/
[2] https://lore.kernel.org/all/aFleJN_fE-RbSoFD@infradead.org/
[3] https://lore.kernel.org/all/20250623142836.GT1613200@noisy.programming.kicks-ass.net/
---
Changes in v3:
- Clarified the macro documentation about in-tree intention and GPL
implications, per Daniel.
- Applied tags.
- Link to v2: https://patch.msgid.link/20250711-export_modules-v2-1-b59b6fad413a@suse.cz
Changes in v2:
- drop the patch to restrict module namespace export for in-tree modules
- fix a pre-existing documentation typo (Nicolas Schier)
- Link to v1: https://patch.msgid.link/20250708-export_modules-v1-0-fbf7a282d23f@suse.cz
---
Documentation/core-api/symbol-namespaces.rst | 11 ++++++-----
fs/anon_inodes.c | 2 +-
include/linux/export.h | 2 +-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/Documentation/core-api/symbol-namespaces.rst b/Documentation/core-api/symbol-namespaces.rst
index 32fc73dc5529e8844c2ce2580987155bcd13cd09..034898e81ba201097330ab9875429e7d3fa30c0f 100644
--- a/Documentation/core-api/symbol-namespaces.rst
+++ b/Documentation/core-api/symbol-namespaces.rst
@@ -76,20 +76,21 @@ A second option to define the default namespace is directly in the compilation
within the corresponding compilation unit before the #include for
<linux/export.h>. Typically it's placed before the first #include statement.
-Using the EXPORT_SYMBOL_GPL_FOR_MODULES() macro
------------------------------------------------
+Using the EXPORT_SYMBOL_FOR_MODULES() macro
+-------------------------------------------
Symbols exported using this macro are put into a module namespace. This
-namespace cannot be imported.
+namespace cannot be imported. These exports are GPL-only as they are only
+intended for in-tree modules.
The macro takes a comma separated list of module names, allowing only those
modules to access this symbol. Simple tail-globs are supported.
For example::
- EXPORT_SYMBOL_GPL_FOR_MODULES(preempt_notifier_inc, "kvm,kvm-*")
+ EXPORT_SYMBOL_FOR_MODULES(preempt_notifier_inc, "kvm,kvm-*")
-will limit usage of this symbol to modules whoes name matches the given
+will limit usage of this symbol to modules whose name matches the given
patterns.
How to use Symbols exported in Namespaces
diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 1d847a939f29a41356af3f12e5f61372ec2fb550..180a458fc4f74249d674ec3c6e01277df1d9e743 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -129,7 +129,7 @@ struct inode *anon_inode_make_secure_inode(struct super_block *sb, const char *n
}
return inode;
}
-EXPORT_SYMBOL_GPL_FOR_MODULES(anon_inode_make_secure_inode, "kvm");
+EXPORT_SYMBOL_FOR_MODULES(anon_inode_make_secure_inode, "kvm");
static struct file *__anon_inode_getfile(const char *name,
const struct file_operations *fops,
diff --git a/include/linux/export.h b/include/linux/export.h
index f35d03b4113b19798036d2993d67eb932ad8ce6f..a686fd0ba406509da5f397e3a415d05c5a051c0d 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -91,6 +91,6 @@
#define EXPORT_SYMBOL_NS(sym, ns) __EXPORT_SYMBOL(sym, "", ns)
#define EXPORT_SYMBOL_NS_GPL(sym, ns) __EXPORT_SYMBOL(sym, "GPL", ns)
-#define EXPORT_SYMBOL_GPL_FOR_MODULES(sym, mods) __EXPORT_SYMBOL(sym, "GPL", "module:" mods)
+#define EXPORT_SYMBOL_FOR_MODULES(sym, mods) __EXPORT_SYMBOL(sym, "GPL", "module:" mods)
#endif /* _LINUX_EXPORT_H */
---
base-commit: d7b8f8e20813f0179d8ef519541a3527e7661d3a
change-id: 20250708-export_modules-12908fa41006
Best regards,
--
Vlastimil Babka <vbabka@suse.cz>
^ permalink raw reply related
* Re: [PATCH v3 7/8] x86/kprobes: enable EXECMEM_ROX_CACHE for kprobes allocations
From: Masami Hiramatsu @ 2025-07-15 0:21 UTC (permalink / raw)
To: Mike Rapoport
Cc: Andrew Morton, Andy Lutomirski, Borislav Petkov, Christophe Leroy,
Daniel Gomez, Dave Hansen, Ingo Molnar, Liam R. Howlett,
Luis Chamberlain, Mark Rutland, Masami Hiramatsu, H. Peter Anvin,
Peter Zijlstra, Petr Pavlu, Sami Tolvanen, Steven Rostedt,
Thomas Gleixner, Yann Ylavic, linux-kernel, linux-mm,
linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250713071730.4117334-8-rppt@kernel.org>
On Sun, 13 Jul 2025 10:17:29 +0300
Mike Rapoport <rppt@kernel.org> wrote:
> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>
> x86::alloc_insn_page() always allocates ROX memory.
>
> Instead of overriding this method, add EXECMEM_KPROBES entry in
> execmem_info with pgprot set to PAGE_KERNEL_ROX and use ROX cache when
> configuration and CPU features allow it.
>
Looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks!
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
> arch/x86/kernel/kprobes/core.c | 18 ------------------
> arch/x86/mm/init.c | 9 ++++++++-
> 2 files changed, 8 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index 47cb8eb138ba..6079d15dab8c 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -481,24 +481,6 @@ static int prepare_singlestep(kprobe_opcode_t *buf, struct kprobe *p,
> return len;
> }
>
> -/* Make page to RO mode when allocate it */
> -void *alloc_insn_page(void)
> -{
> - void *page;
> -
> - page = execmem_alloc(EXECMEM_KPROBES, PAGE_SIZE);
> - if (!page)
> - return NULL;
> -
> - /*
> - * TODO: Once additional kernel code protection mechanisms are set, ensure
> - * that the page was not maliciously altered and it is still zeroed.
> - */
> - set_memory_rox((unsigned long)page, 1);
> -
> - return page;
> -}
> -
> /* Kprobe x86 instruction emulation - only regs->ip or IF flag modifiers */
>
> static void kprobe_emulate_ifmodifiers(struct kprobe *p, struct pt_regs *regs)
> diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
> index dbc63f0d538f..442fafd8ff52 100644
> --- a/arch/x86/mm/init.c
> +++ b/arch/x86/mm/init.c
> @@ -1098,7 +1098,14 @@ struct execmem_info __init *execmem_arch_setup(void)
> .pgprot = pgprot,
> .alignment = MODULE_ALIGN,
> },
> - [EXECMEM_KPROBES ... EXECMEM_BPF] = {
> + [EXECMEM_KPROBES] = {
> + .flags = flags,
> + .start = start,
> + .end = MODULES_END,
> + .pgprot = PAGE_KERNEL_ROX,
> + .alignment = MODULE_ALIGN,
> + },
> + [EXECMEM_FTRACE ... EXECMEM_BPF] = {
> .flags = EXECMEM_KASAN_SHADOW,
> .start = start,
> .end = MODULES_END,
> --
> 2.47.2
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 8/8] x86/ftrace: enable EXECMEM_ROX_CACHE for ftrace allocations
From: Steven Rostedt @ 2025-07-14 16:22 UTC (permalink / raw)
To: Mike Rapoport
Cc: Andrew Morton, Andy Lutomirski, Borislav Petkov, Christophe Leroy,
Daniel Gomez, Dave Hansen, Ingo Molnar, Liam R. Howlett,
Luis Chamberlain, Mark Rutland, Masami Hiramatsu, H. Peter Anvin,
Peter Zijlstra, Petr Pavlu, Sami Tolvanen, Thomas Gleixner,
Yann Ylavic, linux-kernel, linux-mm, linux-modules,
linux-trace-kernel, x86
In-Reply-To: <20250713071730.4117334-9-rppt@kernel.org>
On Sun, 13 Jul 2025 10:17:30 +0300
Mike Rapoport <rppt@kernel.org> wrote:
> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>
> For the most part ftrace uses text poking and can handle ROX memory.
> The only place that requires writable memory is create_trampoline() that
> updates the allocated memory and in the end makes it ROX.
>
> Use execmem_alloc_rw() in x86::ftrace::alloc_tramp() and enable ROX cache
> for EXECMEM_FTRACE when configuration and CPU features allow that.
>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* Re: [PATCH v4 0/7] Add generated modalias to modules.builtin.modinfo
From: Alexey Gladkov @ 2025-07-14 13:41 UTC (permalink / raw)
To: Masahiro Yamada, Petr Pavlu, Luis Chamberlain, Sami Tolvanen,
Daniel Gomez, Nathan Chancellor, Nicolas Schier
Cc: linux-kernel, linux-modules, linux-kbuild, linux-scsi
In-Reply-To: <cover.1750511018.git.legion@kernel.org>
On Sat, Jun 21, 2025 at 03:57:12PM +0200, Alexey Gladkov wrote:
> The modules.builtin.modinfo file is used by userspace (kmod to be specific) to
> get information about builtin modules. Among other information about the module,
> information about module aliases is stored. This is very important to determine
> that a particular modalias will be handled by a module that is inside the
> kernel.
>
> There are several mechanisms for creating modalias for modules:
>
> The first is to explicitly specify the MODULE_ALIAS of the macro. In this case,
> the aliases go into the '.modinfo' section of the module if it is compiled
> separately or into vmlinux.o if it is builtin into the kernel.
>
> The second is the use of MODULE_DEVICE_TABLE followed by the use of the
> modpost utility. In this case, vmlinux.o no longer has this information and
> does not get it into modules.builtin.modinfo.
>
> For example:
>
> $ modinfo pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30
> modinfo: ERROR: Module pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30 not found.
>
> $ modinfo xhci_pci
> name: xhci_pci
> filename: (builtin)
> license: GPL
> file: drivers/usb/host/xhci-pci
> description: xHCI PCI Host Controller Driver
>
> The builtin module is missing alias "pci:v*d*sv*sd*bc0Csc03i30*" which will be
> generated by modpost if the module is built separately.
>
> To fix this it is necessary to add the generated by modpost modalias to
> modules.builtin.modinfo.
>
> Fortunately modpost already generates .vmlinux.export.c for exported symbols. It
> is possible to use this file to create a '.modinfo' section for builtin modules.
> The modules.builtin.modinfo file becomes a composite file. One part is extracted
> from vmlinux.o, the other part from .vmlinux.export.o.
Masahiro Yamada, does this version of the patchset look better to you ?
> Notes:
> - v4:
> * Rework the patchset based on top of Masahiro Yamada's patches.
> * Add removal of unnecessary __mod_device_table__* symbols to avoid symbol
> table growth in vmlinux.
> * rust code takes into account changes in __mod_device_table__*.
> * v3: https://lore.kernel.org/all/cover.1748335606.git.legion@kernel.org/
>
> - v3:
> * Add `Reviewed-by` tag to patches from Petr Pavlu.
> * Rebase to v6.15.
> * v2: https://lore.kernel.org/all/20250509164237.2886508-1-legion@kernel.org/
>
> - v2:
> * Drop patch for mfd because it was already applied and is in linux-next.
> * The generation of aliases for builtin modules has been redone as
> suggested by Masahiro Yamada.
> * Rebase to v6.15-rc5-136-g9c69f8884904
> * v1: https://lore.kernel.org/all/cover.1745591072.git.legion@kernel.org/
>
>
> Alexey Gladkov (3):
> scsi: Always define blogic_pci_tbl structure
> modpost: Add modname to mod_device_table alias
> modpost: Create modalias for builtin modules
>
> Masahiro Yamada (4):
> module: remove meaningless 'name' parameter from __MODULE_INFO()
> kbuild: always create intermediate vmlinux.unstripped
> kbuild: keep .modinfo section in vmlinux.unstripped
> kbuild: extract modules.builtin.modinfo from vmlinux.unstripped
>
> drivers/scsi/BusLogic.c | 2 -
> include/asm-generic/vmlinux.lds.h | 2 +-
> include/crypto/algapi.h | 4 +-
> include/linux/module.h | 21 ++++-----
> include/linux/moduleparam.h | 9 ++--
> include/net/tcp.h | 4 +-
> rust/kernel/device_id.rs | 8 ++--
> scripts/Makefile.vmlinux | 74 +++++++++++++++++++++----------
> scripts/Makefile.vmlinux_o | 26 +----------
> scripts/mksysmap | 6 +++
> scripts/mod/file2alias.c | 34 ++++++++++++--
> scripts/mod/modpost.c | 17 ++++++-
> scripts/mod/modpost.h | 2 +
> 13 files changed, 131 insertions(+), 78 deletions(-)
>
> --
> 2.49.0
>
--
Rgrds, legion
^ permalink raw reply
* Re: [PATCH v2] module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES
From: Christian Brauner @ 2025-07-14 8:08 UTC (permalink / raw)
To: Vlastimil Babka, Greg Kroah-Hartman
Cc: Matthias Maennich, Jonathan Corbet, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Alexander Viro, Jan Kara, Christoph Hellwig,
Peter Zijlstra, David Hildenbrand, Shivank Garg,
Jiri Slaby (SUSE), Stephen Rothwell, linux-doc, linux-kernel,
linux-modules, linux-kbuild, linux-fsdevel
In-Reply-To: <20250711-export_modules-v2-1-b59b6fad413a@suse.cz>
On Fri, Jul 11, 2025 at 04:05:16PM +0200, Vlastimil Babka wrote:
> Christoph suggested that the explicit _GPL_ can be dropped from the
> module namespace export macro, as it's intended for in-tree modules
> only. It would be possible to resrict it technically, but it was pointed
> out [2] that some cases of using an out-of-tree build of an in-tree
> module with the same name are legitimate. But in that case those also
> have to be GPL anyway so it's unnecessary to spell it out.
>
> Link: https://lore.kernel.org/all/aFleJN_fE-RbSoFD@infradead.org/ [1]
> Link: https://lore.kernel.org/all/CAK7LNATRkZHwJGpojCnvdiaoDnP%2BaeUXgdey5sb_8muzdWTMkA@mail.gmail.com/ [2]
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Reviewed-by: Shivank Garg <shivankg@amd.com>
> Acked-by: Christian Brauner <brauner@kernel.org>
> Acked-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> Christian asked [1] for EXPORT_SYMBOL_FOR_MODULES() without the _GPL_
> part to avoid controversy converting selected existing EXPORT_SYMBOL().
Thank you!
Reviewed-by: Christian Brauner <brauner@kernel.org>
Am I supposed to take this or how's that going to work?
> Christoph argued [2] that the _FOR_MODULES() export is intended for
> in-tree modules and thus GPL is implied anyway and can be simply dropped
> from the export macro name. Peter agreed [3] about the intention for
> in-tree modules only, although nothing currently enforces it.
>
> It seemed straightforward to add this enforcement, so v1 did that. But
> there were concerns of breaking the (apparently legitimate) usecases of
> loading an updated/development out of tree built version of an in-tree
> module.
>
> So leave out the enforcement part and just drop the _GPL_ from the
> export macro name and so we're left with EXPORT_SYMBOL_FOR_MODULES()
> only. Any in-tree module used in an out-of-tree way will have to be GPL
> anyway by definition.
>
> Current -next has some new instances of EXPORT_SYMBOL_GPL_FOR_MODULES()
> in drivers/tty/serial/8250/8250_rsa.c by commit b20d6576cdb3 ("serial:
> 8250: export RSA functions"). Hopefully it's resolvable by a merge
> commit fixup and we don't need to provide a temporary alias.
>
> [1] https://lore.kernel.org/all/20250623-warmwasser-giftig-ff656fce89ad@brauner/
> [2] https://lore.kernel.org/all/aFleJN_fE-RbSoFD@infradead.org/
> [3] https://lore.kernel.org/all/20250623142836.GT1613200@noisy.programming.kicks-ass.net/
> ---
^ permalink raw reply
* Re: [PATCH v2] module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES
From: Vlastimil Babka @ 2025-07-14 7:09 UTC (permalink / raw)
To: Greg Kroah-Hartman, Daniel Gomez
Cc: Matthias Maennich, Jonathan Corbet, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Alexander Viro, Christian Brauner, Jan Kara,
Christoph Hellwig, Peter Zijlstra, David Hildenbrand,
Shivank Garg, Jiri Slaby (SUSE), Stephen Rothwell, linux-doc,
linux-kernel, linux-modules, linux-kbuild, linux-fsdevel
In-Reply-To: <2025071355-debunk-sprang-e1ad@gregkh>
On 7/13/25 10:31, Greg Kroah-Hartman wrote:
> On Sat, Jul 12, 2025 at 08:26:17PM +0200, Daniel Gomez wrote:
>> On 11/07/2025 16.05, Vlastimil Babka wrote:
>> > Changes in v2:
>> > - drop the patch to restrict module namespace export for in-tree modules
>> > - fix a pre-existing documentation typo (Nicolas Schier)
>> > - Link to v1: https://patch.msgid.link/20250708-export_modules-v1-0-fbf7a282d23f@suse.cz
>> > ---
>> > Documentation/core-api/symbol-namespaces.rst | 8 ++++----
>> > fs/anon_inodes.c | 2 +-
>> > include/linux/export.h | 2 +-
>> > 3 files changed, 6 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/Documentation/core-api/symbol-namespaces.rst b/Documentation/core-api/symbol-namespaces.rst
>> > index 32fc73dc5529e8844c2ce2580987155bcd13cd09..6f7f4f47d43cdeb3b5008c795d254ca2661d39a6 100644
>> > --- a/Documentation/core-api/symbol-namespaces.rst
>> > +++ b/Documentation/core-api/symbol-namespaces.rst
>> > @@ -76,8 +76,8 @@ A second option to define the default namespace is directly in the compilation
>> > within the corresponding compilation unit before the #include for
>> > <linux/export.h>. Typically it's placed before the first #include statement.
>> >
>> > -Using the EXPORT_SYMBOL_GPL_FOR_MODULES() macro
>> > ------------------------------------------------
>> > +Using the EXPORT_SYMBOL_FOR_MODULES() macro
>> > +-------------------------------------------
>> >
>> > Symbols exported using this macro are put into a module namespace. This
>> > namespace cannot be imported.
>>
>> The new naming makes sense, but it breaks the pattern with _GPL suffix:
>>
>> * EXPORT_SYMBOL(sym)
>> * EXPORT_SYMBOL_GPL(sym)
>> * EXPORT_SYMBOL_NS(sym, ns)
>> * EXPORT_SYMBOL_NS_GPL(sym, ns)
>> * EXPORT_SYMBOL_FOR_MODULES(sym, mods)
>>
>> So I think when reading this one may forget about the _obvious_ reason. That's
>> why I think clarifying that in the documentation would be great. Something like:
>>
>> Symbols exported using this macro are put into a module namespace. This
>> namespace cannot be imported. And it's implicitly GPL-only as it's only intended
>> for in-tree modules.
>
> s/implicitly/explicitly/
From the point of the macro name,
it was explicit with "EXPORT_SYMBOL_GPL_FOR_MODULES()"
it's implicit with "EXPORT_SYMBOL_FOR_MODULES()"
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [PATCH v2] module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES
From: Greg Kroah-Hartman @ 2025-07-13 8:31 UTC (permalink / raw)
To: Daniel Gomez
Cc: Vlastimil Babka, Matthias Maennich, Jonathan Corbet,
Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Alexander Viro, Christian Brauner, Jan Kara, Christoph Hellwig,
Peter Zijlstra, David Hildenbrand, Shivank Garg,
Jiri Slaby (SUSE), Stephen Rothwell, linux-doc, linux-kernel,
linux-modules, linux-kbuild, linux-fsdevel
In-Reply-To: <b9b74600-4467-4c76-aa41-0a36b1cce1f4@kernel.org>
On Sat, Jul 12, 2025 at 08:26:17PM +0200, Daniel Gomez wrote:
> On 11/07/2025 16.05, Vlastimil Babka wrote:
> > Christoph suggested that the explicit _GPL_ can be dropped from the
> > module namespace export macro, as it's intended for in-tree modules
> > only. It would be possible to resrict it technically, but it was pointed
> > out [2] that some cases of using an out-of-tree build of an in-tree
> > module with the same name are legitimate. But in that case those also
> > have to be GPL anyway so it's unnecessary to spell it out.
> >
> > Link: https://lore.kernel.org/all/aFleJN_fE-RbSoFD@infradead.org/ [1]
> > Link: https://lore.kernel.org/all/CAK7LNATRkZHwJGpojCnvdiaoDnP%2BaeUXgdey5sb_8muzdWTMkA@mail.gmail.com/ [2]
> > Suggested-by: Christoph Hellwig <hch@infradead.org>
> > Reviewed-by: Shivank Garg <shivankg@amd.com>
> > Acked-by: Christian Brauner <brauner@kernel.org>
> > Acked-by: David Hildenbrand <david@redhat.com>
> > Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> > ---
> > Christian asked [1] for EXPORT_SYMBOL_FOR_MODULES() without the _GPL_
> > part to avoid controversy converting selected existing EXPORT_SYMBOL().
> > Christoph argued [2] that the _FOR_MODULES() export is intended for
> > in-tree modules and thus GPL is implied anyway and can be simply dropped
> > from the export macro name. Peter agreed [3] about the intention for
> > in-tree modules only, although nothing currently enforces it.
> >
> > It seemed straightforward to add this enforcement, so v1 did that. But
> > there were concerns of breaking the (apparently legitimate) usecases of
> > loading an updated/development out of tree built version of an in-tree
> > module.
> >
> > So leave out the enforcement part and just drop the _GPL_ from the
> > export macro name and so we're left with EXPORT_SYMBOL_FOR_MODULES()
> > only. Any in-tree module used in an out-of-tree way will have to be GPL
> > anyway by definition.
> >
> > Current -next has some new instances of EXPORT_SYMBOL_GPL_FOR_MODULES()
> > in drivers/tty/serial/8250/8250_rsa.c by commit b20d6576cdb3 ("serial:
> > 8250: export RSA functions"). Hopefully it's resolvable by a merge
> > commit fixup and we don't need to provide a temporary alias.
> >
> > [1] https://lore.kernel.org/all/20250623-warmwasser-giftig-ff656fce89ad@brauner/
> > [2] https://lore.kernel.org/all/aFleJN_fE-RbSoFD@infradead.org/
> > [3] https://lore.kernel.org/all/20250623142836.GT1613200@noisy.programming.kicks-ass.net/
> > ---
> > Changes in v2:
> > - drop the patch to restrict module namespace export for in-tree modules
> > - fix a pre-existing documentation typo (Nicolas Schier)
> > - Link to v1: https://patch.msgid.link/20250708-export_modules-v1-0-fbf7a282d23f@suse.cz
> > ---
> > Documentation/core-api/symbol-namespaces.rst | 8 ++++----
> > fs/anon_inodes.c | 2 +-
> > include/linux/export.h | 2 +-
> > 3 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/core-api/symbol-namespaces.rst b/Documentation/core-api/symbol-namespaces.rst
> > index 32fc73dc5529e8844c2ce2580987155bcd13cd09..6f7f4f47d43cdeb3b5008c795d254ca2661d39a6 100644
> > --- a/Documentation/core-api/symbol-namespaces.rst
> > +++ b/Documentation/core-api/symbol-namespaces.rst
> > @@ -76,8 +76,8 @@ A second option to define the default namespace is directly in the compilation
> > within the corresponding compilation unit before the #include for
> > <linux/export.h>. Typically it's placed before the first #include statement.
> >
> > -Using the EXPORT_SYMBOL_GPL_FOR_MODULES() macro
> > ------------------------------------------------
> > +Using the EXPORT_SYMBOL_FOR_MODULES() macro
> > +-------------------------------------------
> >
> > Symbols exported using this macro are put into a module namespace. This
> > namespace cannot be imported.
>
> The new naming makes sense, but it breaks the pattern with _GPL suffix:
>
> * EXPORT_SYMBOL(sym)
> * EXPORT_SYMBOL_GPL(sym)
> * EXPORT_SYMBOL_NS(sym, ns)
> * EXPORT_SYMBOL_NS_GPL(sym, ns)
> * EXPORT_SYMBOL_FOR_MODULES(sym, mods)
>
> So I think when reading this one may forget about the _obvious_ reason. That's
> why I think clarifying that in the documentation would be great. Something like:
>
> Symbols exported using this macro are put into a module namespace. This
> namespace cannot be imported. And it's implicitly GPL-only as it's only intended
> for in-tree modules.
s/implicitly/explicitly/
thanks,
greg k-h
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox