.gitignore | 1 + Makefile | 1 + include/linux/module.h | 10 ++++- scripts/Makefile.modpost | 17 +++++++- scripts/mod/file2alias.c | 92 +++++++++++++++++++++++++++++++--------- scripts/mod/modpost.c | 23 +++++++++- scripts/mod/modpost.h | 2 + 7 files changed, 121 insertions(+), 25 deletions(-)
Generate modules.builtin.alias from match ids This patch series (v7) pivots to adding `modules.builtin.alias` from the previous approach of adding a sysfs attribute. The goal is for tools like USBGuard to leverage not only modules.aliases but also `modules.builtin.aliases` to associate devices with the modules that may be bound before deciding to authorize a device or not. This is particularly useful in cases when new devices of a particular type shouldn't be allowed part of the time like for lock screens. Note that `modules.builtin.alias` is generated directly by modpost. This differs from how `modules.alias` is generated because modpost converts the match-id based module aliases into c-files that add additional aliases to the module info. No such c-file is present for vmlinuz though it would be possible to add one. A downside of this would be vmlinuz would grow by 100-200kb for a typical ChromeOS kernel config. -- # Generate modules.builtin.alias from match ids Previous versions of this patch series addressed the same problem by adding a sysfs attribute instead of `modules.builtin.alias`. Consequently, they have a different name and include completely different commits than this version. Note, cover letters were first added in v5. RFC (broken patch): https://lore.kernel.org/lkml/CAJzde042-M4UbpNYKw0eDVg4JqYmwmPYSsmgK+kCMTqsi+-2Yw@mail.gmail.com/ v1 (missing v1 label): https://lore.kernel.org/lkml/20221111152852.2837363-1-allenwebb@google.com/ v2 (missing v2 label): https://lore.kernel.org/lkml/20221128201332.3482092-1-allenwebb@google.com/ v3: https://lore.kernel.org/lkml/20221129224313.455862-1-allenwebb@google.com/ v4: https://lore.kernel.org/lkml/20221130221447.1202206-1-allenwebb@google.com/ v5: https://lore.kernel.org/lkml/20221201211630.101541-1-allenwebb@google.com/ v6: https://lore.kernel.org/lkml/20221202224540.1446952-1-allenwebb@google.com/ v7: This version ## Patch series status This series is still going through revisions in response to comments. This version generates match-id based aliases for all subsystems unlike previous patch series versions which only implemented aliases for USB. I believe there is potential to improve the Makefile part of the patch series as well as an open question of whether modpost should generate `modules.built.alias` directly or create a vmlinuz.mod.c containing the missing module info for the match-id based aliases for built-in modules. ## Acknowledgements Thanks to Greg Kroah-Hartman and the Linux maintainers for being patient with me as I have worked through learning the kernel workflow to get this series into a more presentable state. Thanks to Luis Chamberlain for raising the alternative of using kmod to address the primary motivation of the patch series. Also, thanks to Intel's kernel test robot <lkp@intel.com> for catching issues that showed up on different kernel configurations. Allen Webb (5): module.h: MODULE_DEVICE_TABLE for built-in modules modpost: Track module name for built-in modules modpost: Add -b option for emitting built-in aliases file2alias.c: Implement builtin.alias generation build: Add modules.builtin.alias .gitignore | 1 + Makefile | 1 + include/linux/module.h | 10 ++++- scripts/Makefile.modpost | 17 +++++++- scripts/mod/file2alias.c | 92 +++++++++++++++++++++++++++++++--------- scripts/mod/modpost.c | 23 +++++++++- scripts/mod/modpost.h | 2 + 7 files changed, 121 insertions(+), 25 deletions(-) -- 2.37.3
Generate modules.builtin.alias from match ids This patch series (v8) generates `modules.builtin.alias` during modpost. The goal is for tools like USBGuard to leverage not only modules.aliases but also `modules.builtin.aliases` to associate devices with the modules that may be bound before deciding to authorize a device or not. This is particularly useful in cases when new devices of a particular type shouldn't be allowed part of the time like for lock screens. Also included in this series are style fixes and fixes for build breakages for built-in modules that relied on MODULE_DEVICE_TABLE being a no-op. Some of these were typos in the device table name and one ifdef-ed out the device table. -- # Generate modules.builtin.alias from match ids This series (v7) has incremental improvements over the previous series. One big positive of this patch series is it makes it harder for bugs in kernel modules related to MODULE_DEVICE_TABLE to hide when a module is only ever tested as a built-in module. This is demonstrated by all the required fixes at the beginning of the series. Note, cover letters were first added in v5. RFC (broken patch): https://lore.kernel.org/lkml/CAJzde042-M4UbpNYKw0eDVg4JqYmwmPYSsmgK+kCMTqsi+-2Yw@mail.gmail.com/ v1 (missing v1 label): https://lore.kernel.org/lkml/20221111152852.2837363-1-allenwebb@google.com/ v2 (missing v2 label): https://lore.kernel.org/lkml/20221128201332.3482092-1-allenwebb@google.com/ v3: https://lore.kernel.org/lkml/20221129224313.455862-1-allenwebb@google.com/ v4: https://lore.kernel.org/lkml/20221130221447.1202206-1-allenwebb@google.com/ v5: https://lore.kernel.org/lkml/20221201211630.101541-1-allenwebb@google.com/ v6: https://lore.kernel.org/lkml/20221202224540.1446952-1-allenwebb@google.com/ v7: https://lore.kernel.org/lkml/20221216221703.294683-1-allenwebb@google.com/ v8: This version ## Patch series status 123456789012345678901234567890123456789012345678901234567890123456789012 This series is still going through revisions in response to comments. I believe there is potential to improve the Makefile part of the patch series as well as an open question of whether modpost should generate `modules.built.alias` directly or create a vmlinuz.mod.c containing the missing module info for the match-id based aliases for built-in modules. ## Acknowledgements Thanks to Greg Kroah-Hartman, Christophe Leroy, and the Linux maintainers for being patient with me as I have worked through learning the kernel workflow to get this series into a more presentable state. Thanks to Luis Chamberlain for raising the alternative of using kmod to address the primary motivation of the patch series. Also, thanks to Intel's kernel test robot <lkp@intel.com> for catching issues that showed up on different kernel configurations. Allen Webb (9): imx: Fix typo rockchip-mailbox: Fix typo scsi/BusLogic: Always include device id table stmpe-spi: Fix typo module.h: MODULE_DEVICE_TABLE for built-in modules modpost: Track module name for built-in modules modpost: Add -b option for emitting built-in aliases file2alias.c: Implement builtin.alias generation build: Add modules.builtin.alias .gitignore | 1 + Makefile | 1 + drivers/mailbox/rockchip-mailbox.c | 2 +- drivers/mfd/stmpe-spi.c | 2 +- drivers/scsi/BusLogic.c | 2 - drivers/soc/imx/imx8mp-blk-ctrl.c | 2 +- include/linux/module.h | 15 ++++- scripts/Makefile.modpost | 17 +++++- scripts/mod/file2alias.c | 94 +++++++++++++++++++++++------- scripts/mod/modpost.c | 23 +++++++- scripts/mod/modpost.h | 2 + 11 files changed, 131 insertions(+), 30 deletions(-) -- 2.37.3
On Mon, Dec 19, 2022 at 01:18:46PM -0600, Allen Webb wrote: > Generate modules.builtin.alias from match ids This is looking much better, thanks! Please expand with proper documentation on the use case / value of this on the file: Documentation/kbuild/kbuild.rst Luis
On Mon, Dec 19, 2022 at 2:06 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Mon, Dec 19, 2022 at 01:18:46PM -0600, Allen Webb wrote: > > Generate modules.builtin.alias from match ids > > This is looking much better, thanks! Please expand with proper > documentation on the use case / value of this on the file: > > Documentation/kbuild/kbuild.rst Thanks I added another commit with an update to the documentation which will be included in the next version of the series. > > Luis >
Generate modules.builtin.alias from match ids This patch series (v8) generates `modules.builtin.alias` during modpost. The goal is for tools like USBGuard to leverage not only modules.aliases but also `modules.builtin.aliases` to associate devices with the modules that may be bound before deciding to authorize a device or not. This is particularly useful in cases when new devices of a particular type shouldn't be allowed part of the time like for lock screens. Also included in this series are added documentation, style fixes and fixes for build breakages for built-in modules that relied on MODULE_DEVICE_TABLE being a no-op. Some of these were typos in the device table name and one ifdef-ed out the device table. -- # Generate modules.builtin.alias from match ids This series (v8) adds missing `cc:stable` and `fixes:` commit tags to the relevant commits. It is unlikely these drivers were being built as modules because compilation would have failed. It also updates the build documentation to cover `modules.builtin.alias`. Note, cover letters were first added in v5. RFC (broken patch): https://lore.kernel.org/lkml/CAJzde042-M4UbpNYKw0eDVg4JqYmwmPYSsmgK+kCMTqsi+-2Yw@mail.gmail.com/ v1 (missing v1 label): https://lore.kernel.org/lkml/20221111152852.2837363-1-allenwebb@google.com/ v2 (missing v2 label): https://lore.kernel.org/lkml/20221128201332.3482092-1-allenwebb@google.com/ v3: https://lore.kernel.org/lkml/20221129224313.455862-1-allenwebb@google.com/ v4: https://lore.kernel.org/lkml/20221130221447.1202206-1-allenwebb@google.com/ v5: https://lore.kernel.org/lkml/20221201211630.101541-1-allenwebb@google.com/ v6: https://lore.kernel.org/lkml/20221202224540.1446952-1-allenwebb@google.com/ v7: https://lore.kernel.org/lkml/20221216221703.294683-1-allenwebb@google.com/ v8: https://lore.kernel.org/lkml/20221219191855.2010466-1-allenwebb@google.com/ v9: This version ## Patch series status This series is still going through revisions in response to comments. I believe there is potential to improve the Makefile part of the patch series as well as an open question of whether modpost should generate `modules.built.alias` directly or create a vmlinuz.mod.c containing the missing module info for the match-id based aliases for built-in modules. ## Acknowledgements Thanks to Greg Kroah-Hartman, Christophe Leroy, Luis Chamberlain and the other Linux maintainers for being patient with me as I have worked through learning the kernel workflow to get this series into a more presentable state. Thanks to Luis Chamberlain for raising the alternative of using kmod to address the primary motivation of the patch series. Also, thanks to Intel's kernel test robot <lkp@intel.com> for catching issues that showed up on different kernel configurations. Allen Webb (10): imx: Fix typo rockchip-mailbox: Fix typo scsi/BusLogic: Always include device id table stmpe-spi: Fix typo module.h: MODULE_DEVICE_TABLE for built-in modules modpost: Track module name for built-in modules modpost: Add -b option for emitting built-in aliases file2alias.c: Implement builtin.alias generation build: Add modules.builtin.alias Documentation: Include modules.builtin.alias .gitignore | 1 + Documentation/kbuild/kbuild.rst | 6 ++ Makefile | 1 + drivers/mailbox/rockchip-mailbox.c | 2 +- drivers/mfd/stmpe-spi.c | 2 +- drivers/scsi/BusLogic.c | 2 - drivers/soc/imx/imx8mp-blk-ctrl.c | 2 +- include/linux/module.h | 15 ++++- scripts/Makefile.modpost | 17 +++++- scripts/mod/file2alias.c | 94 +++++++++++++++++++++++------- scripts/mod/modpost.c | 23 +++++++- scripts/mod/modpost.h | 2 + 12 files changed, 137 insertions(+), 30 deletions(-) -- 2.37.3
Generate modules.builtin.alias from match ids This patch series (v10) generates `modules.builtin.alias` during modpost. The goal is for tools like USBGuard to leverage not only modules.aliases but also `modules.builtin.aliases` to associate devices with the modules that may be bound before deciding to authorize a device or not. This is particularly useful in cases when new devices of a particular type shouldn't be allowed part of the time like for lock screens. Also included in this series are added documentation, style fixes and fixes for build breakages for built-in modules that relied on MODULE_DEVICE_TABLE being a no-op. Some of these were typos in device table name that do not need aliases and one ifdef-ed out the device table. --- Generate modules.builtin.alias from match ids ============================================= This series (v10) removes the `cc:stable` commit tags since the fixes only are needed going forward. It also includes documentation updates and unifies the MODULE_DEVICE_TABLE macro for both the builtin and module case. Additionally, rather than fixing the typo-ed device table names the commits were updated to drop the broken MODULE_DEVICE_TABLE invocations, since they belong to device types that would not benefit from the intended purpose of `modules.builtin.alias`. Note, cover letters were first added in v5. RFC (broken patch): https://lore.kernel.org/lkml/CAJzde042-M4UbpNYKw0eDVg4JqYmwmPYSsmgK+kCMTqsi+-2Yw@mail.gmail.com/ v1 (missing v1 label): https://lore.kernel.org/lkml/20221111152852.2837363-1-allenwebb@google.com/ v2 (missing v2 label): https://lore.kernel.org/lkml/20221128201332.3482092-1-allenwebb@google.com/ v3: https://lore.kernel.org/lkml/20221129224313.455862-1-allenwebb@google.com/ v4: https://lore.kernel.org/lkml/20221130221447.1202206-1-allenwebb@google.com/ v5: https://lore.kernel.org/lkml/20221201211630.101541-1-allenwebb@google.com/ v6: https://lore.kernel.org/lkml/20221202224540.1446952-1-allenwebb@google.com/ v7: https://lore.kernel.org/lkml/20221216221703.294683-1-allenwebb@google.com/ v8: https://lore.kernel.org/lkml/20221219191855.2010466-1-allenwebb@google.com/ v9: https://lore.kernel.org/lkml/20221219204619.2205248-1-allenwebb@google.com/ v10: This version Patch series status ------------------- This series should be close to ready. Acknowledgements ---------------- Thanks to Greg Kroah-Hartman, Christophe Leroy, Luis Chamberlain and the other Linux maintainers for being patient with me as I have worked through learning the kernel workflow to get this series into a more presentable state. Thanks to Luis Chamberlain for raising the alternative of using kmod to address the primary motivation of the patch series. Thanks to Dmitry Torokhov and Benson Leung for feedback on the USB authorization documentation for the driver API. Also, thanks to Intel's kernel test robot <lkp@intel.com> for catching issues that showed up with different kernel configurations. Allen Webb (11): rockchip-mailbox: Remove unneeded MODULE_DEVICE_TABLE scsi/BusLogic: Always include device id table stmpe-spi: Fix MODULE_DEVICE_TABLE entries module.h: MODULE_DEVICE_TABLE for built-in modules modpost: Track module name for built-in modules modpost: Add -b option for emitting built-in aliases file2alias.c: Implement builtin.alias generation build: Add modules.builtin.alias Documentation: Include modules.builtin.alias Documentation: Update writing_usb_driver for built-in modules Documentation: add USB authorization document to driver-api .gitignore | 1 + .../driver-api/usb/authorization.rst | 71 ++++++++++++++ Documentation/driver-api/usb/index.rst | 1 + .../driver-api/usb/writing_usb_driver.rst | 3 + Documentation/kbuild/kbuild.rst | 7 ++ Makefile | 1 + drivers/mailbox/rockchip-mailbox.c | 1 - drivers/mfd/stmpe-spi.c | 1 - drivers/scsi/BusLogic.c | 2 - include/linux/module.h | 36 ++++++-- scripts/Makefile.modpost | 15 +++ scripts/mod/file2alias.c | 92 ++++++++++++++----- scripts/mod/modpost.c | 30 +++++- scripts/mod/modpost.h | 2 + 14 files changed, 229 insertions(+), 34 deletions(-) create mode 100644 Documentation/driver-api/usb/authorization.rst -- 2.39.2
A one character difference in the name supplied to MODULE_DEVICE_TABLE
breaks compilation for ROCKCHIP_MBOX after built-in modules can
generate match-id based module aliases. Since this wasn't being used
before and builtin aliases aren't needed in this case, remove it.
This was not caught earlier because ROCKCHIP_MBOX can not be built as a
module and MODULE_DEVICE_TABLE is a no-op for built-in modules.
Fixes: f70ed3b5dc8b ("mailbox: rockchip: Add Rockchip mailbox driver")
Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/lkml/202212171140.NB93eVvI-lkp@intel.com/
Signed-off-by: Allen Webb <allenwebb@google.com>
---
drivers/mailbox/rockchip-mailbox.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/mailbox/rockchip-mailbox.c b/drivers/mailbox/rockchip-mailbox.c
index e02d3c9e3693..1f0adc283d1b 100644
--- a/drivers/mailbox/rockchip-mailbox.c
+++ b/drivers/mailbox/rockchip-mailbox.c
@@ -159,7 +159,6 @@ static const struct of_device_id rockchip_mbox_of_match[] = {
{ .compatible = "rockchip,rk3368-mailbox", .data = &rk3368_drv_data},
{ },
};
-MODULE_DEVICE_TABLE(of, rockchp_mbox_of_match);
static int rockchip_mbox_probe(struct platform_device *pdev)
{
--
2.39.2A future patch makes use of the device table for built-in modules, so
do not ifdef out the match id table.
Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/lkml/202212171257.0oLypyYS-lkp@intel.com/
Signed-off-by: Allen Webb <allenwebb@google.com>
---
drivers/scsi/BusLogic.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index f7b7ffda1161..79fc8a24e614 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -3713,7 +3713,6 @@ static void __exit blogic_exit(void)
__setup("BusLogic=", blogic_setup);
-#ifdef MODULE
/*static struct pci_device_id blogic_pci_tbl[] = {
{ PCI_VENDOR_ID_BUSLOGIC, PCI_DEVICE_ID_BUSLOGIC_MULTIMASTER,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
@@ -3729,7 +3728,6 @@ static const struct pci_device_id blogic_pci_tbl[] = {
{PCI_DEVICE(PCI_VENDOR_ID_BUSLOGIC, PCI_DEVICE_ID_BUSLOGIC_FLASHPOINT)},
{0, },
};
-#endif
MODULE_DEVICE_TABLE(pci, blogic_pci_tbl);
module_init(blogic_init);
--
2.39.2A one character difference in the name supplied to MODULE_DEVICE_TABLE
breaks compilation for STMPE_SPI after built-in modules can generate
match-id based module aliases. Since this wasn't being used before and
builtin aliases aren't needed in this case, remove it.
This was not caught earlier because STMPE_SPI can not be built as a
module and MODULE_DEVICE_TABLE is a no-op for built-in modules.
Fixes: e789995d5c61 ("mfd: Add support for STMPE SPI interface")
Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/lkml/202212171140.NB93eVvI-lkp@intel.com/
Signed-off-by: Allen Webb <allenwebb@google.com>
---
drivers/mfd/stmpe-spi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/mfd/stmpe-spi.c b/drivers/mfd/stmpe-spi.c
index e9cbf33502b3..e9cb6a635472 100644
--- a/drivers/mfd/stmpe-spi.c
+++ b/drivers/mfd/stmpe-spi.c
@@ -129,7 +129,6 @@ static const struct spi_device_id stmpe_spi_id[] = {
{ "stmpe2403", STMPE2403 },
{ }
};
-MODULE_DEVICE_TABLE(spi, stmpe_id);
static struct spi_driver stmpe_spi_driver = {
.driver = {
--
2.39.2On Thu, Apr 06, 2023 at 02:00:22PM -0500, Allen Webb wrote:
> A one character difference in the name supplied to MODULE_DEVICE_TABLE
> breaks compilation for STMPE_SPI after built-in modules can generate
> match-id based module aliases. Since this wasn't being used before and
> builtin aliases aren't needed in this case, remove it.
>
> This was not caught earlier because STMPE_SPI can not be built as a
> module and MODULE_DEVICE_TABLE is a no-op for built-in modules.
>
> Fixes: e789995d5c61 ("mfd: Add support for STMPE SPI interface")
> Reported-by: kernel test robot <lkp@intel.com>
> Link: https://lore.kernel.org/lkml/202212171140.NB93eVvI-lkp@intel.com/
> Signed-off-by: Allen Webb <allenwebb@google.com>
> ---
I'm happy to take patches 1-3 thorugh modules-next, but please try to
Cc the driver maintainers on the next iteration of this patch series
and let them know these are related to this work and see if you can
get their ACKs to go through another tree for this purpose.
Luis
On Thu, Apr 06, 2023 at 02:00:22PM -0500, Allen Webb wrote:
> A one character difference in the name supplied to MODULE_DEVICE_TABLE
> breaks compilation for STMPE_SPI after built-in modules can generate
> match-id based module aliases. Since this wasn't being used before and
> builtin aliases aren't needed in this case, remove it.
>
> This was not caught earlier because STMPE_SPI can not be built as a
> module and MODULE_DEVICE_TABLE is a no-op for built-in modules.
>
> Fixes: e789995d5c61 ("mfd: Add support for STMPE SPI interface")
> Reported-by: kernel test robot <lkp@intel.com>
> Link: https://lore.kernel.org/lkml/202212171140.NB93eVvI-lkp@intel.com/
> Signed-off-by: Allen Webb <allenwebb@google.com>
> ---
Oh feel free to add Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
on patches 1-3.
Luis
Implement MODULE_DEVICE_TABLE for build-in modules to make it possible
to generate a builtin.alias file to complement modules.alias.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
include/linux/module.h | 36 +++++++++++++++++++++++++++++-------
1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 4435ad9439ab..b1cb12e06996 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -237,14 +237,36 @@ extern void cleanup_module(void);
/* What your module does. */
#define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
-#ifdef MODULE
-/* Creates an alias so file2alias.c can find device table. */
+/*
+ * Creates an alias so file2alias.c can find device table.
+ *
+ * Use this in cases where a device table is used to match devices because it
+ * surfaces match-id based module aliases to userspace for:
+ * - Automatic module loading through modules.alias.
+ * - Tools like USBGuard which block devices based on policy such as which
+ * modules match a device.
+ *
+ * The only use-case for built-in drivers today is to enable userspace to
+ * prevent / allow probe for devices on certain subsystems even if the driver is
+ * already loaded. An example is the USB subsystem with its authorized_default
+ * sysfs attribute. For more details refer to the kernel's Documentation for USB
+ * about authorized_default.
+ *
+ * The module name is included in the alias for two reasons:
+ * - It avoids creating two aliases with the same name for built-in modules.
+ * Historically MODULE_DEVICE_TABLE was a no-op for built-in modules, so
+ * there was nothing to stop different modules from having the same device
+ * table name and consequently the same alias when building as a module.
+ * - The module name is needed by files2alias.c to associate a particular
+ * device table with its associated module for built-in modules since
+ * files2alias would otherwise see the module name as `vmlinuz.o`.
+ */
#define MODULE_DEVICE_TABLE(type, name) \
-extern typeof(name) __mod_##type##__##name##_device_table \
- __attribute__ ((unused, alias(__stringify(name))))
-#else /* !MODULE */
-#define MODULE_DEVICE_TABLE(type, name)
-#endif
+extern void *CONCATENATE( \
+ CONCATENATE(__mod_##type##__##name##__, \
+ __KBUILD_MODNAME), \
+ _device_table) \
+ __attribute__ ((unused, alias(__stringify(name))))
/* Version of form [<epoch>:]<version>[-<extra-version>].
* Or for CVS/RCS ID version, everything but the number is stripped.
--
2.39.2On Thu, Apr 06, 2023 at 02:00:23PM -0500, Allen Webb wrote: > Implement MODULE_DEVICE_TABLE for build-in modules to make it possible > to generate a builtin.alias file to complement modules.alias. > > Signed-off-by: Allen Webb <allenwebb@google.com> > --- > include/linux/module.h | 36 +++++++++++++++++++++++++++++------- > 1 file changed, 29 insertions(+), 7 deletions(-) > > diff --git a/include/linux/module.h b/include/linux/module.h > index 4435ad9439ab..b1cb12e06996 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -237,14 +237,36 @@ extern void cleanup_module(void); > /* What your module does. */ > #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description) > > -#ifdef MODULE > -/* Creates an alias so file2alias.c can find device table. */ > +/* > + * Creates an alias so file2alias.c can find device table. > + * > + * Use this in cases where a device table is used to match devices because it > + * surfaces match-id based module aliases to userspace for: > + * - Automatic module loading through modules.alias. > + * - Tools like USBGuard which block devices based on policy such as which > + * modules match a device. > + * > + * The only use-case for built-in drivers today is to enable userspace to > + * prevent / allow probe for devices on certain subsystems even if the driver is > + * already loaded. An example is the USB subsystem with its authorized_default > + * sysfs attribute. For more details refer to the kernel's Documentation for USB > + * about authorized_default. > + * > + * The module name is included in the alias for two reasons: > + * - It avoids creating two aliases with the same name for built-in modules. > + * Historically MODULE_DEVICE_TABLE was a no-op for built-in modules, so > + * there was nothing to stop different modules from having the same device > + * table name and consequently the same alias when building as a module. > + * - The module name is needed by files2alias.c to associate a particular > + * device table with its associated module for built-in modules since > + * files2alias would otherwise see the module name as `vmlinuz.o`. > + */ > #define MODULE_DEVICE_TABLE(type, name) \ > -extern typeof(name) __mod_##type##__##name##_device_table \ > - __attribute__ ((unused, alias(__stringify(name)))) > -#else /* !MODULE */ > -#define MODULE_DEVICE_TABLE(type, name) > -#endif > +extern void *CONCATENATE( \ > + CONCATENATE(__mod_##type##__##name##__, \ > + __KBUILD_MODNAME), \ > + _device_table) \ > + __attribute__ ((unused, alias(__stringify(name)))) Why does it seem like we're changing extern typeof(name) to a void *? Also the addition of CONCATENATE() makes it not clear if you are modifying the definition before, so it would be good to first add CONCATENATE() to replace the old way without making any functional changes first. Then a secondary patch which extends the world for built-in. Luis
Keep track of the module name when processing match table symbols.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
scripts/mod/file2alias.c | 39 +++++++++++++++++++++++++++++++++++----
scripts/mod/modpost.h | 1 +
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 91c2e7ba5e52..b392d51c3b06 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -28,6 +28,7 @@ typedef Elf64_Addr kernel_ulong_t;
#include <stdint.h>
#endif
+#include <assert.h>
#include <ctype.h>
#include <stdbool.h>
@@ -1540,9 +1541,9 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
Elf_Sym *sym, const char *symname)
{
void *symval;
- char *zeros = NULL;
- const char *name, *identifier;
- unsigned int namelen;
+ char *zeros = NULL, *modname_str = NULL;
+ const char *name, *identifier, *modname;
+ unsigned int namelen, modnamelen;
/* We're looking for a section relative symbol */
if (!sym->st_shndx || get_secindex(info, sym) >= info->num_sections)
@@ -1552,7 +1553,12 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
return;
- /* All our symbols are of form __mod_<name>__<identifier>_device_table. */
+ /*
+ * All our symbols are either of form
+ * __mod_<name>__<identifier>_device_table
+ * or
+ * __mod_<name>__<identifier>__kmod_<builtin-name>_device_table
+ */
if (strncmp(symname, "__mod_", strlen("__mod_")))
return;
name = symname + strlen("__mod_");
@@ -1564,8 +1570,30 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
identifier = strstr(name, "__");
if (!identifier)
return;
+ modnamelen = namelen;
namelen = identifier - name;
+ /*
+ * In the vmlinuz.o case we want to handle __kmod_ so aliases from
+ * builtin modules are attributed correctly.
+ */
+ modname = strstr(identifier + 2, "__kmod_");
+ if (modname) {
+ modname += strlen("__kmod_");
+ modnamelen -= (modname - name) + strlen("_device_table");
+ modname_str = malloc(modnamelen + 1);
+ /* We don't want to continue if the allocation fails. */
+ assert(modname_str);
+ memcpy(modname_str, modname, modnamelen);
+ modname_str[modnamelen] = '\0';
+ }
+
+ if (modname_str)
+ modname = modname_str;
+ else
+ modname = mod->name;
+ mod->builtin_name = modname;
+
/* Handle all-NULL symbols allocated into .bss */
if (info->sechdrs[get_secindex(info, sym)].sh_type & SHT_NOBITS) {
zeros = calloc(1, sym->st_size);
@@ -1597,6 +1625,9 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
}
}
free(zeros);
+ mod->builtin_name = NULL;
+ if (modname_str)
+ free(modname_str);
}
/* Now add out buffered information to the generated C source */
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 1178f40a73f3..34fe5fc0b02c 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -128,6 +128,7 @@ struct module {
struct list_head missing_namespaces;
// Actual imported namespaces
struct list_head imported_namespaces;
+ const char *builtin_name;
char name[];
};
--
2.39.2On Thu, Apr 06, 2023 at 02:00:24PM -0500, Allen Webb wrote:
> Keep track of the module name when processing match table symbols.
This should mention why this would be good. Otherwise, think about it,
ok, it's done but why? If the reason is that it will be needed later
you need to say that in this commit log entry. If its not used now, it
also needs to say that in this commit log so it is easier to review
and set expecataions correctly for the reviewer.
Luis
> Signed-off-by: Allen Webb <allenwebb@google.com>
> ---
> scripts/mod/file2alias.c | 39 +++++++++++++++++++++++++++++++++++----
> scripts/mod/modpost.h | 1 +
> 2 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 91c2e7ba5e52..b392d51c3b06 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -28,6 +28,7 @@ typedef Elf64_Addr kernel_ulong_t;
> #include <stdint.h>
> #endif
>
> +#include <assert.h>
> #include <ctype.h>
> #include <stdbool.h>
>
> @@ -1540,9 +1541,9 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
> Elf_Sym *sym, const char *symname)
> {
> void *symval;
> - char *zeros = NULL;
> - const char *name, *identifier;
> - unsigned int namelen;
> + char *zeros = NULL, *modname_str = NULL;
> + const char *name, *identifier, *modname;
> + unsigned int namelen, modnamelen;
>
> /* We're looking for a section relative symbol */
> if (!sym->st_shndx || get_secindex(info, sym) >= info->num_sections)
> @@ -1552,7 +1553,12 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
> if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
> return;
>
> - /* All our symbols are of form __mod_<name>__<identifier>_device_table. */
> + /*
> + * All our symbols are either of form
> + * __mod_<name>__<identifier>_device_table
> + * or
> + * __mod_<name>__<identifier>__kmod_<builtin-name>_device_table
> + */
> if (strncmp(symname, "__mod_", strlen("__mod_")))
> return;
> name = symname + strlen("__mod_");
> @@ -1564,8 +1570,30 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
> identifier = strstr(name, "__");
> if (!identifier)
> return;
> + modnamelen = namelen;
> namelen = identifier - name;
>
> + /*
> + * In the vmlinuz.o case we want to handle __kmod_ so aliases from
> + * builtin modules are attributed correctly.
> + */
> + modname = strstr(identifier + 2, "__kmod_");
> + if (modname) {
> + modname += strlen("__kmod_");
> + modnamelen -= (modname - name) + strlen("_device_table");
> + modname_str = malloc(modnamelen + 1);
> + /* We don't want to continue if the allocation fails. */
> + assert(modname_str);
> + memcpy(modname_str, modname, modnamelen);
> + modname_str[modnamelen] = '\0';
> + }
> +
> + if (modname_str)
> + modname = modname_str;
> + else
> + modname = mod->name;
> + mod->builtin_name = modname;
> +
> /* Handle all-NULL symbols allocated into .bss */
> if (info->sechdrs[get_secindex(info, sym)].sh_type & SHT_NOBITS) {
> zeros = calloc(1, sym->st_size);
> @@ -1597,6 +1625,9 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
> }
> }
> free(zeros);
> + mod->builtin_name = NULL;
> + if (modname_str)
> + free(modname_str);
> }
>
> /* Now add out buffered information to the generated C source */
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 1178f40a73f3..34fe5fc0b02c 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -128,6 +128,7 @@ struct module {
> struct list_head missing_namespaces;
> // Actual imported namespaces
> struct list_head imported_namespaces;
> + const char *builtin_name;
> char name[];
> };
>
> --
> 2.39.2
>
On Thu, Apr 06, 2023 at 02:00:24PM -0500, Allen Webb wrote: > Keep track of the module name when processing match table symbols. This describes _what_ you are doing here, but no explanation for _why_ you want to do this. thanks, greg k-h
This adds a command line option for writing the match-id based built-in
aliases to a file. A future patch extends file2alias.c to support this
command.
The -b option accepts the output path as a parameter and requires
vmlinuz.o to be one of the input files for the aliases to be found.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
scripts/mod/modpost.c | 30 ++++++++++++++++++++++++++++--
scripts/mod/modpost.h | 1 +
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index efff8078e395..2e452aec0fc6 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2173,6 +2173,19 @@ static void write_if_changed(struct buffer *b, const char *fname)
write_buf(b, fname);
}
+/* Write the builtin aliases to the specified file. */
+static void write_builtin(const char *fname)
+{
+ struct buffer buf = { };
+ struct module *mod;
+
+ list_for_each_entry(mod, &modules, list)
+ buf_write(&buf, mod->modalias_buf.p, mod->modalias_buf.pos);
+
+ write_if_changed(&buf, fname);
+ free(buf.p);
+}
+
static void write_vmlinux_export_c_file(struct module *mod)
{
struct buffer buf = { };
@@ -2329,13 +2342,23 @@ int main(int argc, char **argv)
{
struct module *mod;
char *missing_namespace_deps = NULL;
- char *dump_write = NULL, *files_source = NULL;
+ char *builtin_write = NULL, *dump_write = NULL, *files_source = NULL;
int opt;
LIST_HEAD(dump_lists);
struct dump_list *dl, *dl2;
- while ((opt = getopt(argc, argv, "ei:mnT:o:awENd:")) != -1) {
+ while ((opt = getopt(argc, argv, "b:ei:mnT:o:awENd:")) != -1) {
switch (opt) {
+ case 'b':
+ /*
+ * Writes the match-id based built-in module aliases to
+ * the specified path.
+ *
+ * vmlinuz.o needs to be one of the input files for the
+ * aliases to be found.
+ */
+ builtin_write = optarg;
+ break;
case 'e':
external_module = true;
break;
@@ -2398,6 +2421,9 @@ int main(int argc, char **argv)
write_mod_c_file(mod);
}
+ if (builtin_write)
+ write_builtin(builtin_write);
+
if (missing_namespace_deps)
write_namespace_deps_files(missing_namespace_deps);
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 34fe5fc0b02c..c55a6aeb46bf 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -123,6 +123,7 @@ struct module {
bool has_init;
bool has_cleanup;
struct buffer dev_table_buf;
+ struct buffer modalias_buf;
char srcversion[25];
// Missing namespace dependencies
struct list_head missing_namespaces;
--
2.39.2On Thu, Apr 06, 2023 at 02:00:25PM -0500, Allen Webb wrote:
> This adds a command line option for writing the match-id based
Can you explain in your commit log and in code what is "match-id" ?
Do we not have this documeted anywhere perhaps where we can point to
what it is?
> built-in
> aliases to a file. A future patch extends file2alias.c to support this
> command.
>
> The -b option accepts the output path as a parameter and requires
> vmlinuz.o to be one of the input files for the aliases to be found.
>
> Signed-off-by: Allen Webb <allenwebb@google.com>
> ---
> scripts/mod/modpost.c | 30 ++++++++++++++++++++++++++++--
> scripts/mod/modpost.h | 1 +
> 2 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index efff8078e395..2e452aec0fc6 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -2173,6 +2173,19 @@ static void write_if_changed(struct buffer *b, const char *fname)
> write_buf(b, fname);
> }
>
> +/* Write the builtin aliases to the specified file. */
> +static void write_builtin(const char *fname)
> +{
> + struct buffer buf = { };
> + struct module *mod;
> +
> + list_for_each_entry(mod, &modules, list)
> + buf_write(&buf, mod->modalias_buf.p, mod->modalias_buf.pos);
> +
> + write_if_changed(&buf, fname);
> + free(buf.p);
> +}
> +
> static void write_vmlinux_export_c_file(struct module *mod)
> {
> struct buffer buf = { };
> @@ -2329,13 +2342,23 @@ int main(int argc, char **argv)
> {
> struct module *mod;
> char *missing_namespace_deps = NULL;
> - char *dump_write = NULL, *files_source = NULL;
> + char *builtin_write = NULL, *dump_write = NULL, *files_source = NULL;
> int opt;
> LIST_HEAD(dump_lists);
> struct dump_list *dl, *dl2;
>
> - while ((opt = getopt(argc, argv, "ei:mnT:o:awENd:")) != -1) {
> + while ((opt = getopt(argc, argv, "b:ei:mnT:o:awENd:")) != -1) {
> switch (opt) {
> + case 'b':
> + /*
> + * Writes the match-id based built-in module aliases to
> + * the specified path.
> + *
> + * vmlinuz.o needs to be one of the input files for the
> + * aliases to be found.
> + */
> + builtin_write = optarg;
> + break;
> case 'e':
> external_module = true;
> break;
> @@ -2398,6 +2421,9 @@ int main(int argc, char **argv)
> write_mod_c_file(mod);
> }
>
> + if (builtin_write)
> + write_builtin(builtin_write);
> +
> if (missing_namespace_deps)
> write_namespace_deps_files(missing_namespace_deps);
>
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 34fe5fc0b02c..c55a6aeb46bf 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -123,6 +123,7 @@ struct module {
> bool has_init;
> bool has_cleanup;
> struct buffer dev_table_buf;
> + struct buffer modalias_buf;
> char srcversion[25];
> // Missing namespace dependencies
> struct list_head missing_namespaces;
> --
> 2.39.2
>
This populates the mod->modalias_buf with aliases for built-in modules
when modpost is run against vmlinuz.o.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
scripts/mod/file2alias.c | 61 ++++++++++++++++++++++++++--------------
1 file changed, 40 insertions(+), 21 deletions(-)
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index b392d51c3b06..3793d4632b94 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -233,6 +233,8 @@ static void do_usb_entry(void *symval,
add_wildcard(alias);
buf_printf(&mod->dev_table_buf,
"MODULE_ALIAS(\"%s\");\n", alias);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias %s %s\n", alias, mod->builtin_name);
}
/* Handles increment/decrement of BCD formatted integers */
@@ -377,9 +379,13 @@ static void do_of_entry_multi(void *symval, struct module *mod)
*tmp = '_';
buf_printf(&mod->dev_table_buf, "MODULE_ALIAS(\"%s\");\n", alias);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias %s %s\n", alias, mod->builtin_name);
strcat(alias, "C");
add_wildcard(alias);
buf_printf(&mod->dev_table_buf, "MODULE_ALIAS(\"%s\");\n", alias);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias %s %s\n", alias, mod->builtin_name);
}
static void do_of_table(void *symval, unsigned long size,
@@ -611,12 +617,18 @@ static void do_pnp_device_entry(void *symval, unsigned long size,
buf_printf(&mod->dev_table_buf,
"MODULE_ALIAS(\"pnp:d%s*\");\n", *id);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias pnp:d%s* %s\n",
+ *id, mod->builtin_name);
/* fix broken pnp bus lowercasing */
for (j = 0; j < sizeof(acpi_id); j++)
acpi_id[j] = toupper((*id)[j]);
buf_printf(&mod->dev_table_buf,
"MODULE_ALIAS(\"acpi*:%s:*\");\n", acpi_id);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias acpi*:%s:* %s\n",
+ acpi_id, mod->builtin_name);
}
}
@@ -638,6 +650,8 @@ static void do_pnp_card_entries(void *symval, unsigned long size,
const char *id = (char *)(*devs)[j].id;
int i2, j2;
int dup = 0;
+ char acpi_id[PNP_ID_LEN];
+ int k;
if (!id[0])
break;
@@ -663,19 +677,23 @@ static void do_pnp_card_entries(void *symval, unsigned long size,
}
/* add an individual alias for every device entry */
- if (!dup) {
- char acpi_id[PNP_ID_LEN];
- int k;
-
- buf_printf(&mod->dev_table_buf,
- "MODULE_ALIAS(\"pnp:d%s*\");\n", id);
-
- /* fix broken pnp bus lowercasing */
- for (k = 0; k < sizeof(acpi_id); k++)
- acpi_id[k] = toupper(id[k]);
- buf_printf(&mod->dev_table_buf,
- "MODULE_ALIAS(\"acpi*:%s:*\");\n", acpi_id);
- }
+ if (dup)
+ continue;
+
+ buf_printf(&mod->dev_table_buf,
+ "MODULE_ALIAS(\"pnp:d%s*\");\n", id);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias pnp:d%s* %s\n",
+ id, mod->builtin_name);
+
+ /* fix broken pnp bus lowercasing */
+ for (k = 0; k < sizeof(acpi_id); k++)
+ acpi_id[k] = toupper(id[k]);
+ buf_printf(&mod->dev_table_buf,
+ "MODULE_ALIAS(\"acpi*:%s:*\");\n", acpi_id);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias acpi*:%s:* %s\n",
+ acpi_id, mod->builtin_name);
}
}
}
@@ -1476,10 +1494,13 @@ static void do_table(void *symval, unsigned long size,
size -= id_size;
for (i = 0; i < size; i += id_size) {
- if (do_entry(mod->name, symval+i, alias)) {
- buf_printf(&mod->dev_table_buf,
- "MODULE_ALIAS(\"%s\");\n", alias);
- }
+ if (!do_entry(mod->name, symval + i, alias))
+ continue;
+ buf_printf(&mod->dev_table_buf, "MODULE_ALIAS(\"%s\");\n", alias);
+ if (!mod->builtin_name)
+ continue;
+ buf_printf(&mod->modalias_buf, "alias %s %s\n", alias,
+ mod->builtin_name);
}
}
@@ -1554,10 +1575,8 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
return;
/*
- * All our symbols are either of form
- * __mod_<name>__<identifier>_device_table
- * or
- * __mod_<name>__<identifier>__kmod_<builtin-name>_device_table
+ * All our symbols are of form
+ * __mod_<name>__<identifier>__kmod_<modname>_device_table
*/
if (strncmp(symname, "__mod_", strlen("__mod_")))
return;
--
2.39.2On Thu, Apr 06, 2023 at 02:00:26PM -0500, Allen Webb wrote:
> This populates the mod->modalias_buf with aliases for built-in modules
> when modpost is run against vmlinuz.o.
The commit log should describe why. And if its not used now why is it
being introduced separately. If its to make changes eaiser to read it
shoudl say so.
So builtin thing is set but is it used at this point? Does this patch
make any functional changes? If not why not?
> Signed-off-by: Allen Webb <allenwebb@google.com>
> ---
> scripts/mod/file2alias.c | 61 ++++++++++++++++++++++++++--------------
> 1 file changed, 40 insertions(+), 21 deletions(-)
>
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index b392d51c3b06..3793d4632b94 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -233,6 +233,8 @@ static void do_usb_entry(void *symval,
> add_wildcard(alias);
> buf_printf(&mod->dev_table_buf,
> "MODULE_ALIAS(\"%s\");\n", alias);
> + if (mod->builtin_name)
> + buf_printf(&mod->modalias_buf, "alias %s %s\n", alias, mod->builtin_name);
> }
>
> /* Handles increment/decrement of BCD formatted integers */
> @@ -377,9 +379,13 @@ static void do_of_entry_multi(void *symval, struct module *mod)
> *tmp = '_';
>
> buf_printf(&mod->dev_table_buf, "MODULE_ALIAS(\"%s\");\n", alias);
> + if (mod->builtin_name)
> + buf_printf(&mod->modalias_buf, "alias %s %s\n", alias, mod->builtin_name);
> strcat(alias, "C");
> add_wildcard(alias);
> buf_printf(&mod->dev_table_buf, "MODULE_ALIAS(\"%s\");\n", alias);
> + if (mod->builtin_name)
> + buf_printf(&mod->modalias_buf, "alias %s %s\n", alias, mod->builtin_name);
> }
>
> static void do_of_table(void *symval, unsigned long size,
> @@ -611,12 +617,18 @@ static void do_pnp_device_entry(void *symval, unsigned long size,
>
> buf_printf(&mod->dev_table_buf,
> "MODULE_ALIAS(\"pnp:d%s*\");\n", *id);
> + if (mod->builtin_name)
> + buf_printf(&mod->modalias_buf, "alias pnp:d%s* %s\n",
> + *id, mod->builtin_name);
>
> /* fix broken pnp bus lowercasing */
> for (j = 0; j < sizeof(acpi_id); j++)
> acpi_id[j] = toupper((*id)[j]);
> buf_printf(&mod->dev_table_buf,
> "MODULE_ALIAS(\"acpi*:%s:*\");\n", acpi_id);
> + if (mod->builtin_name)
> + buf_printf(&mod->modalias_buf, "alias acpi*:%s:* %s\n",
> + acpi_id, mod->builtin_name);
> }
> }
>
> @@ -638,6 +650,8 @@ static void do_pnp_card_entries(void *symval, unsigned long size,
> const char *id = (char *)(*devs)[j].id;
> int i2, j2;
> int dup = 0;
> + char acpi_id[PNP_ID_LEN];
> + int k;
>
> if (!id[0])
> break;
> @@ -663,19 +677,23 @@ static void do_pnp_card_entries(void *symval, unsigned long size,
> }
>
> /* add an individual alias for every device entry */
> - if (!dup) {
> - char acpi_id[PNP_ID_LEN];
> - int k;
> -
> - buf_printf(&mod->dev_table_buf,
> - "MODULE_ALIAS(\"pnp:d%s*\");\n", id);
> -
> - /* fix broken pnp bus lowercasing */
> - for (k = 0; k < sizeof(acpi_id); k++)
> - acpi_id[k] = toupper(id[k]);
> - buf_printf(&mod->dev_table_buf,
> - "MODULE_ALIAS(\"acpi*:%s:*\");\n", acpi_id);
> - }
> + if (dup)
> + continue;
The change from !dup to (dup) continue makes your changes harder to
read. It would be good to make that change separately so to make it
easier to read what you are doing differently.
> +
> + buf_printf(&mod->dev_table_buf,
> + "MODULE_ALIAS(\"pnp:d%s*\");\n", id);
> + if (mod->builtin_name)
> + buf_printf(&mod->modalias_buf, "alias pnp:d%s* %s\n",
> + id, mod->builtin_name);
> +
> + /* fix broken pnp bus lowercasing */
> + for (k = 0; k < sizeof(acpi_id); k++)
> + acpi_id[k] = toupper(id[k]);
> + buf_printf(&mod->dev_table_buf,
> + "MODULE_ALIAS(\"acpi*:%s:*\");\n", acpi_id);
> + if (mod->builtin_name)
> + buf_printf(&mod->modalias_buf, "alias acpi*:%s:* %s\n",
> + acpi_id, mod->builtin_name);
> }
> }
> }
> @@ -1476,10 +1494,13 @@ static void do_table(void *symval, unsigned long size,
> size -= id_size;
>
> for (i = 0; i < size; i += id_size) {
> - if (do_entry(mod->name, symval+i, alias)) {
> - buf_printf(&mod->dev_table_buf,
> - "MODULE_ALIAS(\"%s\");\n", alias);
> - }
> + if (!do_entry(mod->name, symval + i, alias))
> + continue;
Same here. You could just fold the changes which negate the check into
and shif the code into one patch with 0 functional changes. Then a
second patch with your changes.
> + buf_printf(&mod->dev_table_buf, "MODULE_ALIAS(\"%s\");\n", alias);
> + if (!mod->builtin_name)
> + continue;
> + buf_printf(&mod->modalias_buf, "alias %s %s\n", alias,
> + mod->builtin_name);
> }
> }
>
> @@ -1554,10 +1575,8 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
> return;
>
> /*
> - * All our symbols are either of form
> - * __mod_<name>__<identifier>_device_table
> - * or
> - * __mod_<name>__<identifier>__kmod_<builtin-name>_device_table
> + * All our symbols are of form
> + * __mod_<name>__<identifier>__kmod_<modname>_device_table
> */
> if (strncmp(symname, "__mod_", strlen("__mod_")))
> return;
> --
> 2.39.2
>
Generate modules.builtin.alias using modpost and install it with the
modules.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
.gitignore | 1 +
Makefile | 1 +
scripts/Makefile.modpost | 15 +++++++++++++++
3 files changed, 17 insertions(+)
diff --git a/.gitignore b/.gitignore
index 13a7f08a3d73..ddaa622bddac 100644
--- a/.gitignore
+++ b/.gitignore
@@ -71,6 +71,7 @@ modules.order
/System.map
/Module.markers
/modules.builtin
+/modules.builtin.alias
/modules.builtin.modinfo
/modules.nsdeps
diff --git a/Makefile b/Makefile
index a2c310df2145..43dcc1ea5fcf 100644
--- a/Makefile
+++ b/Makefile
@@ -1578,6 +1578,7 @@ __modinst_pre:
fi
@sed 's:^\(.*\)\.o$$:kernel/\1.ko:' modules.order > $(MODLIB)/modules.order
@cp -f modules.builtin $(MODLIB)/
+ @cp -f modules.builtin.alias $(MODLIB)/
@cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/
endif # CONFIG_MODULES
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 0980c58d8afc..e3ecc17a7a19 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -15,6 +15,7 @@
# 2) modpost is then used to
# 3) create one <module>.mod.c file per module
# 4) create one Module.symvers file with CRC for all exported symbols
+# 5) create modules.builtin.alias the aliases for built-in modules
# Step 3 is used to place certain information in the module's ELF
# section, including information such as:
@@ -63,6 +64,20 @@ modpost-args += -T $(MODORDER)
modpost-deps += $(MODORDER)
endif
+ifneq ($(wildcard vmlinux.o),)
+output-builtin.alias := modules.builtin.alias
+modpost-args += -b .modules.builtin.alias.in
+.modules.builtin.alias.in: $(output-symdump)
+ @# Building $(output-symdump) generates .modules.builtin.alias.in as a
+ @# side effect.
+ @[ -e $@ ] || $(MODPOST) -b .modules.builtin.alias.in vmlinux.o
+
+$(output-builtin.alias): .modules.builtin.alias.in
+ sort -o $@ $^
+
+__modpost: $(output-builtin.alias)
+endif
+
ifeq ($(KBUILD_EXTMOD),)
# Generate the list of in-tree objects in vmlinux
--
2.39.2On Thu, Apr 06, 2023 at 02:00:27PM -0500, Allen Webb wrote: > Generate modules.builtin.alias using modpost and install it with the > modules. Why? This is probably one of the more important commits and the commit log is pretty slim. > Signed-off-by: Allen Webb <allenwebb@google.com> > --- > .gitignore | 1 + > Makefile | 1 + > scripts/Makefile.modpost | 15 +++++++++++++++ > 3 files changed, 17 insertions(+) > > diff --git a/.gitignore b/.gitignore > index 13a7f08a3d73..ddaa622bddac 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -71,6 +71,7 @@ modules.order > /System.map > /Module.markers > /modules.builtin > +/modules.builtin.alias > /modules.builtin.modinfo > /modules.nsdeps > > diff --git a/Makefile b/Makefile > index a2c310df2145..43dcc1ea5fcf 100644 > --- a/Makefile > +++ b/Makefile > @@ -1578,6 +1578,7 @@ __modinst_pre: > fi > @sed 's:^\(.*\)\.o$$:kernel/\1.ko:' modules.order > $(MODLIB)/modules.order > @cp -f modules.builtin $(MODLIB)/ > + @cp -f modules.builtin.alias $(MODLIB)/ > @cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/ > > endif # CONFIG_MODULES > diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost > index 0980c58d8afc..e3ecc17a7a19 100644 > --- a/scripts/Makefile.modpost > +++ b/scripts/Makefile.modpost > @@ -15,6 +15,7 @@ > # 2) modpost is then used to > # 3) create one <module>.mod.c file per module > # 4) create one Module.symvers file with CRC for all exported symbols > +# 5) create modules.builtin.alias the aliases for built-in modules Does everyone want that file? > # Step 3 is used to place certain information in the module's ELF > # section, including information such as: > @@ -63,6 +64,20 @@ modpost-args += -T $(MODORDER) > modpost-deps += $(MODORDER) > endif > > +ifneq ($(wildcard vmlinux.o),) > +output-builtin.alias := modules.builtin.alias > +modpost-args += -b .modules.builtin.alias.in > +.modules.builtin.alias.in: $(output-symdump) > + @# Building $(output-symdump) generates .modules.builtin.alias.in as a > + @# side effect. > + @[ -e $@ ] || $(MODPOST) -b .modules.builtin.alias.in vmlinux.o Does using -b create a delay in builds ? What is the effect on build time on a typical 4-core or 8-core build? Does everyone want it? Should we add a new option which lets people decide if they want this at build time or not? Luis > + > +$(output-builtin.alias): .modules.builtin.alias.in > + sort -o $@ $^ > + > +__modpost: $(output-builtin.alias) > +endif > + > ifeq ($(KBUILD_EXTMOD),) > > # Generate the list of in-tree objects in vmlinux > -- > 2.39.2 >
I finally got a chance to go through the comments and work on a follow-up to this series, but it probably makes sense to get this sorted ahead of the follow-up (if possible). On Wed, May 24, 2023 at 2:02 AM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Thu, Apr 06, 2023 at 02:00:27PM -0500, Allen Webb wrote: > > Generate modules.builtin.alias using modpost and install it with the > > modules. > > Why? This is probably one of the more important commits and the > commit log is pretty slim. > > > Signed-off-by: Allen Webb <allenwebb@google.com> > > --- > > .gitignore | 1 + > > Makefile | 1 + > > scripts/Makefile.modpost | 15 +++++++++++++++ > > 3 files changed, 17 insertions(+) > > > > diff --git a/.gitignore b/.gitignore > > index 13a7f08a3d73..ddaa622bddac 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -71,6 +71,7 @@ modules.order > > /System.map > > /Module.markers > > /modules.builtin > > +/modules.builtin.alias > > /modules.builtin.modinfo > > /modules.nsdeps > > > > diff --git a/Makefile b/Makefile > > index a2c310df2145..43dcc1ea5fcf 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1578,6 +1578,7 @@ __modinst_pre: > > fi > > @sed 's:^\(.*\)\.o$$:kernel/\1.ko:' modules.order > $(MODLIB)/modules.order > > @cp -f modules.builtin $(MODLIB)/ > > + @cp -f modules.builtin.alias $(MODLIB)/ > > @cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/ > > > > endif # CONFIG_MODULES > > diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost > > index 0980c58d8afc..e3ecc17a7a19 100644 > > --- a/scripts/Makefile.modpost > > +++ b/scripts/Makefile.modpost > > @@ -15,6 +15,7 @@ > > # 2) modpost is then used to > > # 3) create one <module>.mod.c file per module > > # 4) create one Module.symvers file with CRC for all exported symbols > > +# 5) create modules.builtin.alias the aliases for built-in modules > > Does everyone want that file? Not everyone needs it so we could exclude it, but the cost of adding it isn't that high. I am fine with putting it behind a config, though we would need to decide whether to have it default on/off. > > > # Step 3 is used to place certain information in the module's ELF > > # section, including information such as: > > @@ -63,6 +64,20 @@ modpost-args += -T $(MODORDER) > > modpost-deps += $(MODORDER) > > endif > > > > +ifneq ($(wildcard vmlinux.o),) > > +output-builtin.alias := modules.builtin.alias > > +modpost-args += -b .modules.builtin.alias.in > > +.modules.builtin.alias.in: $(output-symdump) > > + @# Building $(output-symdump) generates .modules.builtin.alias.in as a > > + @# side effect. > > + @[ -e $@ ] || $(MODPOST) -b .modules.builtin.alias.in vmlinux.o > > Does using -b create a delay in builds ? What is the effect on build > time on a typical 4-core or 8-core build? Does everyone want it? Here is some data I collected related to build time and memory usage impact: Without builtin.alias: TIME="real %e\nuser %U\nsys %S\nres-max %M" time scripts/mod/modpost -E -o Module.symvers -T modules.order ERROR: modpost: "__x86_return_thunk" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "kernel_fpu_end" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "hchacha_block_generic" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "boot_cpu_data" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "static_key_enable" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "cpu_has_xfeatures" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "crypto_register_skciphers" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "crypto_unregister_skciphers" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "__stack_chk_fail" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "memset" [arch/x86/crypto/chacha-x86_64.ko] undefined! WARNING: modpost: suppressed 17432 unresolved symbol warnings because there were too many) Command exited with non-zero status 1 real 0.44 user 0.43 sys 0.01 res-max 4896 With builtin.alias: TIME="real %e\nuser %U\nsys %S\nres-max %M" time scripts/mod/modpost -E -o Module.symvers -T modules.order -b .modules.builtin.alias.in vmlinux.o real 1.43 user 1.38 sys 0.05 res-max 51920 Notice that modpost only uses a single core, so multicore isn't really as much of a factor here. While it more than triples the time required for the modpost operation the difference is only about one second of CPU time. The memory usage is much larger when generating modules.builtin.alias because of the size of vmlinux.o. My biggest performance related concern is actually the size difference of vmlinux caused by the modules.h changes, but it looks like that is negligible (24KiB): Without builtin.alias: du vmlinux.o 663048 vmlinux.o With builtin.alias: du vmlinux.o 663072 vmlinux.o > > Should we add a new option which lets people decide if they want this > at build time or not? I don't feel strongly that there should or should not be a config for this. On the side for a config the extra second of CPU time and space taken up by the modules.builtin.alias file would add up across all the builds and machines, so removing it where it isn't used would help mitigate that. On the flip side if it isn't used widely enough, it is more likely that breakages are missed until someone who actually uses it notices. Please let me know if you feel strongly either way given the data. Thanks, Allen > > Luis > > > + > > +$(output-builtin.alias): .modules.builtin.alias.in > > + sort -o $@ $^ > > + > > +__modpost: $(output-builtin.alias) > > +endif > > + > > ifeq ($(KBUILD_EXTMOD),) > > > > # Generate the list of in-tree objects in vmlinux > > -- > > 2.39.2 > >
Please cc Alexander and Alessandro in future patch series as well, as they could likley be interested in your work too. On Wed, Jul 19, 2023 at 02:51:48PM -0500, Allen Webb wrote: > I finally got a chance to go through the comments and work on a > follow-up to this series, but it probably makes sense to get this > sorted ahead of the follow-up (if possible). > > On Wed, May 24, 2023 at 2:02 AM Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > On Thu, Apr 06, 2023 at 02:00:27PM -0500, Allen Webb wrote: > > > Generate modules.builtin.alias using modpost and install it with the > > > modules. > > > > Why? This is probably one of the more important commits and the > > commit log is pretty slim. > > > > > Signed-off-by: Allen Webb <allenwebb@google.com> > > > --- > > > .gitignore | 1 + > > > Makefile | 1 + > > > scripts/Makefile.modpost | 15 +++++++++++++++ > > > 3 files changed, 17 insertions(+) > > > > > > diff --git a/.gitignore b/.gitignore > > > index 13a7f08a3d73..ddaa622bddac 100644 > > > --- a/.gitignore > > > +++ b/.gitignore > > > @@ -71,6 +71,7 @@ modules.order > > > /System.map > > > /Module.markers > > > /modules.builtin > > > +/modules.builtin.alias > > > /modules.builtin.modinfo > > > /modules.nsdeps > > > > > > diff --git a/Makefile b/Makefile > > > index a2c310df2145..43dcc1ea5fcf 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -1578,6 +1578,7 @@ __modinst_pre: > > > fi > > > @sed 's:^\(.*\)\.o$$:kernel/\1.ko:' modules.order > $(MODLIB)/modules.order > > > @cp -f modules.builtin $(MODLIB)/ > > > + @cp -f modules.builtin.alias $(MODLIB)/ > > > @cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/ > > > > > > endif # CONFIG_MODULES > > > diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost > > > index 0980c58d8afc..e3ecc17a7a19 100644 > > > --- a/scripts/Makefile.modpost > > > +++ b/scripts/Makefile.modpost > > > @@ -15,6 +15,7 @@ > > > # 2) modpost is then used to > > > # 3) create one <module>.mod.c file per module > > > # 4) create one Module.symvers file with CRC for all exported symbols > > > +# 5) create modules.builtin.alias the aliases for built-in modules > > > > Does everyone want that file? > > Not everyone needs it so we could exclude it, but the cost of adding > it isn't that high. I am fine with putting it behind a config, though > we would need to decide whether to have it default on/off. We didn't know the cost until I asked, it was the point of asking. Perhaps Nick, Alessandro or Alexander could use it too later. > > > # Step 3 is used to place certain information in the module's ELF > > > # section, including information such as: > > > @@ -63,6 +64,20 @@ modpost-args += -T $(MODORDER) > > > modpost-deps += $(MODORDER) > > > endif > > > > > > +ifneq ($(wildcard vmlinux.o),) > > > +output-builtin.alias := modules.builtin.alias > > > +modpost-args += -b .modules.builtin.alias.in > > > +.modules.builtin.alias.in: $(output-symdump) > > > + @# Building $(output-symdump) generates .modules.builtin.alias.in as a > > > + @# side effect. > > > + @[ -e $@ ] || $(MODPOST) -b .modules.builtin.alias.in vmlinux.o > > > > Does using -b create a delay in builds ? What is the effect on build > > time on a typical 4-core or 8-core build? Does everyone want it? > > Here is some data I collected related to build time and memory usage impact: > > Without builtin.alias: > TIME="real %e\nuser %U\nsys %S\nres-max %M" time scripts/mod/modpost > -E -o Module.symvers -T modules.order > ERROR: modpost: "__x86_return_thunk" > [arch/x86/crypto/chacha-x86_64.ko] undefined! > ERROR: modpost: "kernel_fpu_end" [arch/x86/crypto/chacha-x86_64.ko] undefined! > ERROR: modpost: "hchacha_block_generic" > [arch/x86/crypto/chacha-x86_64.ko] undefined! > ERROR: modpost: "boot_cpu_data" [arch/x86/crypto/chacha-x86_64.ko] undefined! > ERROR: modpost: "static_key_enable" [arch/x86/crypto/chacha-x86_64.ko] > undefined! > ERROR: modpost: "cpu_has_xfeatures" [arch/x86/crypto/chacha-x86_64.ko] > undefined! > ERROR: modpost: "crypto_register_skciphers" > [arch/x86/crypto/chacha-x86_64.ko] undefined! > ERROR: modpost: "crypto_unregister_skciphers" > [arch/x86/crypto/chacha-x86_64.ko] undefined! > ERROR: modpost: "__stack_chk_fail" [arch/x86/crypto/chacha-x86_64.ko] undefined! > ERROR: modpost: "memset" [arch/x86/crypto/chacha-x86_64.ko] undefined! > WARNING: modpost: suppressed 17432 unresolved symbol warnings because > there were too many) > Command exited with non-zero status 1 > real 0.44 > user 0.43 > sys 0.01 > res-max 4896 > > With builtin.alias: > TIME="real %e\nuser %U\nsys %S\nres-max %M" time scripts/mod/modpost > -E -o Module.symvers -T modules.order -b .modules.builtin.alias.in > vmlinux.o > real 1.43 > user 1.38 > sys 0.05 > res-max 51920 > > Notice that modpost only uses a single core, so multicore isn't really > as much of a factor here. While it more than triples the time required > for the modpost operation the difference is only about one second of > CPU time. The memory usage is much larger when generating > modules.builtin.alias because of the size of vmlinux.o. The modpost impact time of about 1 second for a type of config you used should be described in your commit log and or kconfig entry to enable this. > My biggest performance related concern is actually the size difference > of vmlinux caused by the modules.h changes, but it looks like that is > negligible (24KiB): And this size too. 24 KiB is not that small, so I'd prefer we kconfig'ize it for now and have those who need it to select it. If we later all want it, we can default to yes but for now default to no. The default today by kconfig is to no so an empty default is fine. > Without builtin.alias: > du vmlinux.o > 663048 vmlinux.o > > With builtin.alias: > du vmlinux.o > 663072 vmlinux.o What type of configuration was used? allyesconfig? > > > > Should we add a new option which lets people decide if they want this > > at build time or not? > > I don't feel strongly that there should or should not be a config for > this. On the side for a config the extra second of CPU time and space > taken up by the modules.builtin.alias file would add up across all the > builds and machines, so removing it where it isn't used would help > mitigate that. On the flip side if it isn't used widely enough, it is > more likely that breakages are missed until someone who actually uses > it notices. > > Please let me know if you feel strongly either way given the data. For now I'd prefer a kconfig option, it's easy to default to y later, but saving 64 KiB seems like a desirable thing for some folks. Luis
Update the documentation to include the presense and use case of
modules.builtin.alias.
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Allen Webb <allenwebb@google.com>
---
Documentation/kbuild/kbuild.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst
index 5202186728b4..b27c66c3ca9e 100644
--- a/Documentation/kbuild/kbuild.rst
+++ b/Documentation/kbuild/kbuild.rst
@@ -17,6 +17,13 @@ modules.builtin
This file lists all modules that are built into the kernel. This is used
by modprobe to not fail when trying to load something builtin.
+modules.builtin.alias
+---------------------
+This file lists all match-id based aliases for modules built into the kernel.
+An example usage of the built-in aliases is to enable software such as
+USBGuard to allow or block devices outside of just the vendor, product, and
+device ID. This enables more flexible security policies in userspace.
+
modules.builtin.modinfo
-----------------------
This file contains modinfo from all modules that are built into the kernel.
--
2.39.2Built-in modules that set id_table need to set MODULE_DEVICE_TABLE so
update the documentation accordingly.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
Documentation/driver-api/usb/writing_usb_driver.rst | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/driver-api/usb/writing_usb_driver.rst b/Documentation/driver-api/usb/writing_usb_driver.rst
index 95c4f5d14052..5f38e3bd469a 100644
--- a/Documentation/driver-api/usb/writing_usb_driver.rst
+++ b/Documentation/driver-api/usb/writing_usb_driver.rst
@@ -128,6 +128,9 @@ single device with a specific vendor and product ID::
};
MODULE_DEVICE_TABLE (usb, skel_table);
+The ``MODULE_DEVICE_TABLE`` should also be set for built-in USB drivers
+that provide an id_table, so that tools like USBGuard can properly
+associate devices with your driver.
There are other macros that can be used in describing a struct
:c:type:`usb_device_id` for drivers that support a whole class of USB
--
2.39.2There is a user-facing USB authorization document, but it is midding
details a driver should have developer, so add them in a new document.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
.../driver-api/usb/authorization.rst | 71 +++++++++++++++++++
Documentation/driver-api/usb/index.rst | 1 +
2 files changed, 72 insertions(+)
create mode 100644 Documentation/driver-api/usb/authorization.rst
diff --git a/Documentation/driver-api/usb/authorization.rst b/Documentation/driver-api/usb/authorization.rst
new file mode 100644
index 000000000000..383dcc037a15
--- /dev/null
+++ b/Documentation/driver-api/usb/authorization.rst
@@ -0,0 +1,71 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+====================
+Device Authorization
+====================
+
+This document is intended for driver developers. See
+Documentation/usb/authorization.rst if you are looking for how to use
+USB authorization.
+
+Authorization provides userspace a way to allow or block configuring
+devices early during enumeration before any modules are probed for the
+device. While it is possible to block a device by not loading the
+required modules, this also prevents other devices from using the
+module as well. For example someone might have an unattended computer
+downloading installation media to a USB drive. Presumably this computer
+would be locked to make it more difficult for a bad actor to access the
+computer. Since USB storage devices are not needed to interact with the
+lock screen, the authorized_default sysfs attribute can be set to not
+authorize new USB devices by default. A userspace tool like USBGuard
+can then vet the devices. Mice, keyboards, etc can be allowed by
+writing to their authorized sysfs attribute so that the lock screen can
+still be used (this important in cases like suspend+resume or docks)
+while other devices can be blocked as long as the lock screen is shown.
+
+Sysfs Attributes
+================
+
+Userspace can control USB device authorization through the
+authorized_default and authorized sysfs attributes.
+
+authorized_default
+------------------
+
+Defined in ``drivers/usb/core/hcd.c``
+
+The authorized_default sysfs attribute is only present for host
+controllers. It determines the initial state of the authorized sysfs
+attribute of USB devices newly connected to the corresponding host
+controller. It can take on the following values:
+
++---------------------------------------------------+
+| Value | Behavior |
++=======+===========================================+
+| -1 | Authorize all devices except wireless USB |
++-------+-------------------------------------------+
+| 0 | Do not authorize new devices |
++-------+-------------------------------------------+
+| 1 | Authorize new devices |
++-------+-------------------------------------------+
+| 2 | Authorize new internal devices only |
++---------------------------------------------------+
+
+Note that firmware platform code determines if a device is internal or
+not and this is reported as the connect_type sysfs attribute of the USB
+port. This is currently supported by ACPI, but device tree still needs
+an implementation. Authorizing new internal devices only can be useful
+to work around issues with devices that misbehave if there are delays
+in probing their module.
+
+authorized
+----------
+
+Defined in ``drivers/usb/core/sysfs.c``
+
+Every USB device has an authorized sysfs attribute which can take the
+values 0 and 1. When authorized is 0, the device still is present in
+sysfs, but none of its interfaces can be associated with drivers and
+modules will not be probed. When authorized is 1 (or set to one) a
+configuration is chosen for the device and its interfaces are
+registered allowing drivers to bind to them.
diff --git a/Documentation/driver-api/usb/index.rst b/Documentation/driver-api/usb/index.rst
index cfa8797ea614..ffe37916f99f 100644
--- a/Documentation/driver-api/usb/index.rst
+++ b/Documentation/driver-api/usb/index.rst
@@ -7,6 +7,7 @@ Linux USB API
usb
gadget
anchors
+ authorization
bulk-streams
callbacks
dma
--
2.39.2On Thu, Apr 06, 2023 at 02:00:30PM -0500, Allen Webb wrote: > There is a user-facing USB authorization document, but it is midding > details a driver should have developer, so add them in a new document. I'm sorry, but I can not parse this sentence :( Can you rephrase it? > Signed-off-by: Allen Webb <allenwebb@google.com> > --- > .../driver-api/usb/authorization.rst | 71 +++++++++++++++++++ > Documentation/driver-api/usb/index.rst | 1 + > 2 files changed, 72 insertions(+) > create mode 100644 Documentation/driver-api/usb/authorization.rst > > diff --git a/Documentation/driver-api/usb/authorization.rst b/Documentation/driver-api/usb/authorization.rst > new file mode 100644 > index 000000000000..383dcc037a15 > --- /dev/null > +++ b/Documentation/driver-api/usb/authorization.rst > @@ -0,0 +1,71 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +==================== > +Device Authorization > +==================== > + > +This document is intended for driver developers. See > +Documentation/usb/authorization.rst if you are looking for how to use > +USB authorization. > + > +Authorization provides userspace a way to allow or block configuring > +devices early during enumeration before any modules are probed for the > +device. While it is possible to block a device by not loading the > +required modules, this also prevents other devices from using the > +module as well. For example someone might have an unattended computer > +downloading installation media to a USB drive. Presumably this computer > +would be locked to make it more difficult for a bad actor to access the > +computer. Since USB storage devices are not needed to interact with the > +lock screen, the authorized_default sysfs attribute can be set to not > +authorize new USB devices by default. A userspace tool like USBGuard > +can then vet the devices. Mice, keyboards, etc can be allowed by > +writing to their authorized sysfs attribute so that the lock screen can > +still be used (this important in cases like suspend+resume or docks) > +while other devices can be blocked as long as the lock screen is shown. > + > +Sysfs Attributes > +================ > + > +Userspace can control USB device authorization through the > +authorized_default and authorized sysfs attributes. > + > +authorized_default > +------------------ > + > +Defined in ``drivers/usb/core/hcd.c`` > + > +The authorized_default sysfs attribute is only present for host > +controllers. It determines the initial state of the authorized sysfs > +attribute of USB devices newly connected to the corresponding host > +controller. It can take on the following values: > + > ++---------------------------------------------------+ > +| Value | Behavior | > ++=======+===========================================+ > +| -1 | Authorize all devices except wireless USB | > ++-------+-------------------------------------------+ > +| 0 | Do not authorize new devices | > ++-------+-------------------------------------------+ > +| 1 | Authorize new devices | > ++-------+-------------------------------------------+ > +| 2 | Authorize new internal devices only | > ++---------------------------------------------------+ > + > +Note that firmware platform code determines if a device is internal or > +not and this is reported as the connect_type sysfs attribute of the USB > +port. This is currently supported by ACPI, but device tree still needs > +an implementation. Authorizing new internal devices only can be useful > +to work around issues with devices that misbehave if there are delays > +in probing their module. > + > +authorized > +---------- > + > +Defined in ``drivers/usb/core/sysfs.c`` > + > +Every USB device has an authorized sysfs attribute which can take the > +values 0 and 1. When authorized is 0, the device still is present in > +sysfs, but none of its interfaces can be associated with drivers and > +modules will not be probed. When authorized is 1 (or set to one) a > +configuration is chosen for the device and its interfaces are > +registered allowing drivers to bind to them. Why would a driver author care about any of this? It's all user-facing, so shouldn't it go into the other document? thanks, greg k-h
A one character difference in the name supplied to MODULE_DEVICE_TABLE
breaks a future patch set, so fix the typo.
Cc: stable@vger.kernel.org
Fixes: 556f5cf9568a ("soc: imx: add i.MX8MP HSIO blk-ctrl")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Allen Webb <allenwebb@google.com>
---
drivers/soc/imx/imx8mp-blk-ctrl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
index 0e3b6ba22f94..344a0a71df14 100644
--- a/drivers/soc/imx/imx8mp-blk-ctrl.c
+++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
@@ -743,7 +743,7 @@ static const struct of_device_id imx8mp_blk_ctrl_of_match[] = {
/* Sentinel */
}
};
-MODULE_DEVICE_TABLE(of, imx8m_blk_ctrl_of_match);
+MODULE_DEVICE_TABLE(of, imx8mp_blk_ctrl_of_match);
static struct platform_driver imx8mp_blk_ctrl_driver = {
.probe = imx8mp_blk_ctrl_probe,
--
2.37.3On Mon, Dec 19, 2022 at 02:46:09PM -0600, Allen Webb wrote: > A one character difference in the name supplied to MODULE_DEVICE_TABLE > breaks a future patch set, so fix the typo. Breaking a future change is not worth a stable backport, right? Doesn't this fix a real issue now? If so, please explain that. thanks, greg k-h
On Mon, Dec 19, 2022 at 3:23 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Mon, Dec 19, 2022 at 02:46:09PM -0600, Allen Webb wrote: > > A one character difference in the name supplied to MODULE_DEVICE_TABLE > > breaks a future patch set, so fix the typo. > > What behaviour is broken here for older kernels? What would not work > that makes this patch worthy of consideration for stable? The commit > log should be clear on that. > > In the future, it may be useful for you to wait at least 1 week or so > before sending a new series becuase just a couple of days is not enough > if you are getting feedback. > > So before sending a v10, please give it at least a few days or a week. > > Luis On Tue, Dec 20, 2022 at 12:42 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Mon, Dec 19, 2022 at 02:46:09PM -0600, Allen Webb wrote: > > A one character difference in the name supplied to MODULE_DEVICE_TABLE > > breaks a future patch set, so fix the typo. > > Breaking a future change is not worth a stable backport, right? Doesn't > this fix a real issue now? If so, please explain that. > > thanks, > > greg k-h I will update the commit message to say that it breaks compilation when building imx8mp-blk-ctrl as a module (and so forth for the other similar patches).
On Tue, Dec 20, 2022 at 08:26:06AM -0600, Allen Webb wrote: > On Mon, Dec 19, 2022 at 3:23 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > On Mon, Dec 19, 2022 at 02:46:09PM -0600, Allen Webb wrote: > > > A one character difference in the name supplied to MODULE_DEVICE_TABLE > > > breaks a future patch set, so fix the typo. > > > > What behaviour is broken here for older kernels? What would not work > > that makes this patch worthy of consideration for stable? The commit > > log should be clear on that. > > > > In the future, it may be useful for you to wait at least 1 week or so > > before sending a new series becuase just a couple of days is not enough > > if you are getting feedback. > > > > So before sending a v10, please give it at least a few days or a week. > > > > Luis > > On Tue, Dec 20, 2022 at 12:42 AM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Mon, Dec 19, 2022 at 02:46:09PM -0600, Allen Webb wrote: > > > A one character difference in the name supplied to MODULE_DEVICE_TABLE > > > breaks a future patch set, so fix the typo. > > > > Breaking a future change is not worth a stable backport, right? Doesn't > > this fix a real issue now? If so, please explain that. > > > > thanks, > > > > greg k-h > > I will update the commit message to say that it breaks compilation > when building imx8mp-blk-ctrl as a module (and so forth for the other > similar patches). Can that code be built as a module? Same for the other changes...
On Tue, Dec 20, 2022 at 8:32 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Tue, Dec 20, 2022 at 08:26:06AM -0600, Allen Webb wrote: > > On Mon, Dec 19, 2022 at 3:23 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > > > On Mon, Dec 19, 2022 at 02:46:09PM -0600, Allen Webb wrote: > > > > A one character difference in the name supplied to MODULE_DEVICE_TABLE > > > > breaks a future patch set, so fix the typo. > > > > > > What behaviour is broken here for older kernels? What would not work > > > that makes this patch worthy of consideration for stable? The commit > > > log should be clear on that. > > > > > > In the future, it may be useful for you to wait at least 1 week or so > > > before sending a new series becuase just a couple of days is not enough > > > if you are getting feedback. > > > > > > So before sending a v10, please give it at least a few days or a week. > > > > > > Luis > > > > On Tue, Dec 20, 2022 at 12:42 AM Greg Kroah-Hartman > > <gregkh@linuxfoundation.org> wrote: > > > > > > On Mon, Dec 19, 2022 at 02:46:09PM -0600, Allen Webb wrote: > > > > A one character difference in the name supplied to MODULE_DEVICE_TABLE > > > > breaks a future patch set, so fix the typo. > > > > > > Breaking a future change is not worth a stable backport, right? Doesn't > > > this fix a real issue now? If so, please explain that. > > > > > > thanks, > > > > > > greg k-h > > > > I will update the commit message to say that it breaks compilation > > when building imx8mp-blk-ctrl as a module (and so forth for the other > > similar patches). > > Can that code be built as a module? Same for the other changes... Nope, I will remove the cc: stable and revert the commit messages back to just referencing the future change, but this time I will be more specific. SOC_IMX8M symbol value 'm' invalid for SOC_IMX8M ROCKCHIP_MBOX symbol value 'm' invalid for ROCKCHIP_MBOX STMPE_SPI symbol value 'm' invalid for STMPE_SPI
On Mon, Dec 19, 2022 at 02:46:09PM -0600, Allen Webb wrote: > A one character difference in the name supplied to MODULE_DEVICE_TABLE > breaks a future patch set, so fix the typo. What behaviour is broken here for older kernels? What would not work that makes this patch worthy of consideration for stable? The commit log should be clear on that. In the future, it may be useful for you to wait at least 1 week or so before sending a new series becuase just a couple of days is not enough if you are getting feedback. So before sending a v10, please give it at least a few days or a week. Luis
A one character difference in the name supplied to MODULE_DEVICE_TABLE
breaks a future patch set, so fix the typo.
Cc: stable@vger.kernel.org
Fixes: f70ed3b5dc8b ("mailbox: rockchip: Add Rockchip mailbox driver")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Allen Webb <allenwebb@google.com>
---
drivers/mailbox/rockchip-mailbox.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mailbox/rockchip-mailbox.c b/drivers/mailbox/rockchip-mailbox.c
index 979acc810f30..ca50f7f176f6 100644
--- a/drivers/mailbox/rockchip-mailbox.c
+++ b/drivers/mailbox/rockchip-mailbox.c
@@ -159,7 +159,7 @@ static const struct of_device_id rockchip_mbox_of_match[] = {
{ .compatible = "rockchip,rk3368-mailbox", .data = &rk3368_drv_data},
{ },
};
-MODULE_DEVICE_TABLE(of, rockchp_mbox_of_match);
+MODULE_DEVICE_TABLE(of, rockchip_mbox_of_match);
static int rockchip_mbox_probe(struct platform_device *pdev)
{
--
2.37.3On Mon, Dec 19, 2022 at 02:46:10PM -0600, Allen Webb wrote:
> A one character difference in the name supplied to MODULE_DEVICE_TABLE
> breaks a future patch set, so fix the typo.
>
> Cc: stable@vger.kernel.org
> Fixes: f70ed3b5dc8b ("mailbox: rockchip: Add Rockchip mailbox driver")
How has this been an issue since the 4.6 kernel and no one has noticed
it? Can this code not be built as a module? If not, then please
explain this.
thanks,
greg k-h
On Tue, Dec 20, 2022 at 12:46 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Dec 19, 2022 at 02:46:10PM -0600, Allen Webb wrote:
> > A one character difference in the name supplied to MODULE_DEVICE_TABLE
> > breaks a future patch set, so fix the typo.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: f70ed3b5dc8b ("mailbox: rockchip: Add Rockchip mailbox driver")
>
> How has this been an issue since the 4.6 kernel and no one has noticed
> it? Can this code not be built as a module? If not, then please
> explain this.
As mentioned in a different sub-thread this cannot be built as a
module so I updated the commit message to:
imx: Fix typo
A one character difference in the name supplied to MODULE_DEVICE_TABLE
breaks compilation for SOC_IMX8M after built-in modules can generate
match-id based module aliases, so fix the typo.
This was not caught earlier because SOC_IMX8M can not be built as a
module and MODULE_DEVICE_TABLE is a no-op for built-in modules.
Fixes: 556f5cf9568a ("soc: imx: add i.MX8MP HSIO blk-ctrl")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Allen Webb <allenwebb@google.com>
>
> thanks,
>
> greg k-h
On Tue, Dec 20, 2022 at 08:58:36AM -0600, Allen Webb wrote: > As mentioned in a different sub-thread this cannot be built as a > module so I updated the commit message to: > > imx: Fix typo > > A one character difference in the name supplied to MODULE_DEVICE_TABLE > breaks compilation for SOC_IMX8M after built-in modules can generate > match-id based module aliases, so fix the typo. Are you saying that it is a typo *now* only, and fixing it does not fix compilation now, but that fixing the typo will fix a future compilation issue once your patches get merged for built-in module aliases? > This was not caught earlier because SOC_IMX8M can not be built as a > module and MODULE_DEVICE_TABLE is a no-op for built-in modules. Odd, so why did it use MODULE_DEVICE_TABLE then? What was the reason for the driver having MODULE_DEVICE_TABLE if it was a no-op? Luis
On Tue, Dec 20, 2022 at 12:12 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Tue, Dec 20, 2022 at 08:58:36AM -0600, Allen Webb wrote: > > As mentioned in a different sub-thread this cannot be built as a > > module so I updated the commit message to: > > > > imx: Fix typo > > > > A one character difference in the name supplied to MODULE_DEVICE_TABLE > > breaks compilation for SOC_IMX8M after built-in modules can generate > > match-id based module aliases, so fix the typo. > > Are you saying that it is a typo *now* only, and fixing it does not fix > compilation now, but that fixing the typo will fix a future compilation > issue once your patches get merged for built-in module aliases? It was always a typo, it just doesn't affect the build because MODULE_DEVICE_TABLE is not doing anything for built-in modules before this patch series. > > > This was not caught earlier because SOC_IMX8M can not be built as a > > module and MODULE_DEVICE_TABLE is a no-op for built-in modules. > > Odd, so why did it use MODULE_DEVICE_TABLE then? What was the reason for > the driver having MODULE_DEVICE_TABLE if it was a no-op? That is a good question. I can only speculate as to the answer but it is plausible people copied a common pattern and since no breakage was noticed left it as is. It also raises the question how many modules have device tables, but do not call MODULE_DEVICE_TABLE since they are only ever built-in. Maybe there should be some build time enforcement mechanism to make sure that these are consistent. > > Luis
On Tue, Dec 20, 2022 at 12:19:49PM -0600, Allen Webb wrote:
> On Tue, Dec 20, 2022 at 12:12 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> >
> > On Tue, Dec 20, 2022 at 08:58:36AM -0600, Allen Webb wrote:
> > > As mentioned in a different sub-thread this cannot be built as a
> > > module so I updated the commit message to:
> > >
> > > imx: Fix typo
> > >
> > > A one character difference in the name supplied to MODULE_DEVICE_TABLE
> > > breaks compilation for SOC_IMX8M after built-in modules can generate
> > > match-id based module aliases, so fix the typo.
> >
> > Are you saying that it is a typo *now* only, and fixing it does not fix
> > compilation now, but that fixing the typo will fix a future compilation
> > issue once your patches get merged for built-in module aliases?
>
> It was always a typo, it just doesn't affect the build because
> MODULE_DEVICE_TABLE is not doing anything for built-in modules before
> this patch series.
>
> >
> > > This was not caught earlier because SOC_IMX8M can not be built as a
> > > module and MODULE_DEVICE_TABLE is a no-op for built-in modules.
> >
> > Odd, so why did it use MODULE_DEVICE_TABLE then? What was the reason for
> > the driver having MODULE_DEVICE_TABLE if it was a no-op?
>
> That is a good question. I can only speculate as to the answer
You can use git blame to trace back to the original commit that added
it, then use git blame foo.c commit-id~1 on the file to keep going
back until you get to the first commit that added that entry, check out
that as a branch and see if the driver was still not a module then
(tristate). If so then your speculation is very likely accurate and
can be spelled out in the commit log message.
It begs the inverse question too though, you are finding uses of
built-in-always code (never tristate) which uses MODULE_DEVICE_TABLE().
Although today that's a no-op, after your changes this becomes useful
information, so do you need to scrape to see what built-in-aways code
*do* not use MODULE_DEVICE_TABLE() where after your patches it would
have become useful?
Determing if there is value to that endeavour should be easily grasped by
reading the description you are adding to MODULE_DEVICE_TABLE() --
in your patch 5 "module.h: MODULE_DEVICE_TABLE for built-in modules".
Driver developers for built-in-always code should read that description
and be able to decide if they should use it or not. But even your latest
replies to Greg do not make that clear, *I personally gather* rather that
this would in no way shape or form be useful. But is that true?
So why not just remove MODULE_DEVICE_TABLE() from code we know is
built-in-always code instead of fixing a typo just to fix a future
compile issue?
Then your commit log is not about "fix typo", but rather remove a no-op
macro as the driver is always built-in and keeping that macro would not
help built-in code.
> but it
> is plausible people copied a common pattern and since no breakage was
> noticed left it as is.
This level of clarity is important to spell out in the commit log
message, specially if you are suggesting it is just a typo fix. Because
I will take it for granted that it is just that.
If it fixes a future use case where the typo would be more of an issue,
you can mention that in a secondary paragraph or sentence.
> It also raises the question how many modules have device tables, but
> do not call MODULE_DEVICE_TABLE since they are only ever built-in.
> Maybe there should be some build time enforcement mechanism to make
> sure that these are consistent.
Definitely, Nick Alcock is doing some related work where the semantics
of built-in modules needs to be clearer, he for instance is now removing
a few MODULE_() macros for things which *are never* modules, and this is
because after commit 8b41fc4454e ("kbuild: create modules.builtin
without Makefile.modbuiltin or tristate.conf") we rely on the module
license tag to generate the modules.builtin file. Without that commit
we end up traversing the source tree twice. Nick's work builds on
that work and futher clarifies these semantics by adding tooling which
complains when something which is *never* capable of being a module
uses module macros. The macro you are extending, MODULE_DEVICE_TABLE(),
today is a no-op for built-in, but you are adding support to extend it
for built-in stuff. Nick's work will help with clarifying symbol locality
and so he may be interested in your association for the data in
MODULE_DEVICE_TABLE and how you associate to a respective would-be
module. His work is useful for making tracing more accurate with respect
to symbol associations, so the data in MODULE_DEVICE_TABLE() may be
useful as well to him.
You folks may want to Cc each other on your patches.
If we know for certain things *will never* be used or *should not be
used*, as in the case of the module license tag, we should certainly
have tooling to pick up on that crap to help us tidy it up.
If you determine MODULE_DEVICE_TABLE() *should* not be used for built-in
always code (never tristate) then you and Nick likely have overlap of
macros to tidy up and tooling to share to spot these sort of issues.
Luis
On 20 Dec 2022, Luis Chamberlain uttered the following:
>> It also raises the question how many modules have device tables, but
>> do not call MODULE_DEVICE_TABLE since they are only ever built-in.
>> Maybe there should be some build time enforcement mechanism to make
>> sure that these are consistent.
>
> Definitely, Nick Alcock is doing some related work where the semantics
> of built-in modules needs to be clearer, he for instance is now removing
> a few MODULE_() macros for things which *are never* modules, and this is
> because after commit 8b41fc4454e ("kbuild: create modules.builtin
> without Makefile.modbuiltin or tristate.conf") we rely on the module
> license tag to generate the modules.builtin file. Without that commit
> we end up traversing the source tree twice. Nick's work builds on
> that work and futher clarifies these semantics by adding tooling which
> complains when something which is *never* capable of being a module
> uses module macros. The macro you are extending, MODULE_DEVICE_TABLE(),
> today is a no-op for built-in, but you are adding support to extend it
> for built-in stuff. Nick's work will help with clarifying symbol locality
> and so he may be interested in your association for the data in
> MODULE_DEVICE_TABLE and how you associate to a respective would-be
> module. His work is useful for making tracing more accurate with respect
> to symbol associations, so the data in MODULE_DEVICE_TABLE() may be
> useful as well to him.
The kallmodsyms module info (and, thus, modules.builtin) and
MODULE_DEVICE_TABLE do seem interestingly related. I wonder if we can in
future reuse at least the module names so we can save a few KiB more
space... (in this case, the canonical copy should probably be the one in
kallmodsyms, because that lets kallmodsyms reuse strings where modules
and their source file have similar names. Something for the future...)
> You folks may want to Cc each other on your patches.
I'd welcome that.
btw, do you want another kallmodsyms patch series from me just arranging
to drop fewer MODULE_ entries from non-modules (just MODULE_LICENSE) or
would this be considered noise for now? (Are we deadlocked on each
other, or are you still looking at the last series I sent, which I think
was v10 in late November?)
--
NULL && (void)
On Mon, Jan 9, 2023 at 5:54 AM Nick Alcock <nick.alcock@oracle.com> wrote:
>
> On 20 Dec 2022, Luis Chamberlain uttered the following:
> >> It also raises the question how many modules have device tables, but
> >> do not call MODULE_DEVICE_TABLE since they are only ever built-in.
> >> Maybe there should be some build time enforcement mechanism to make
> >> sure that these are consistent.
> >
> > Definitely, Nick Alcock is doing some related work where the semantics
> > of built-in modules needs to be clearer, he for instance is now removing
> > a few MODULE_() macros for things which *are never* modules, and this is
> > because after commit 8b41fc4454e ("kbuild: create modules.builtin
> > without Makefile.modbuiltin or tristate.conf") we rely on the module
> > license tag to generate the modules.builtin file. Without that commit
> > we end up traversing the source tree twice. Nick's work builds on
> > that work and futher clarifies these semantics by adding tooling which
> > complains when something which is *never* capable of being a module
> > uses module macros. The macro you are extending, MODULE_DEVICE_TABLE(),
> > today is a no-op for built-in, but you are adding support to extend it
> > for built-in stuff. Nick's work will help with clarifying symbol locality
> > and so he may be interested in your association for the data in
> > MODULE_DEVICE_TABLE and how you associate to a respective would-be
> > module. His work is useful for making tracing more accurate with respect
> > to symbol associations, so the data in MODULE_DEVICE_TABLE() may be
> > useful as well to him.
>
> The kallmodsyms module info (and, thus, modules.builtin) and
> MODULE_DEVICE_TABLE do seem interestingly related. I wonder if we can in
> future reuse at least the module names so we can save a few KiB more
> space... (in this case, the canonical copy should probably be the one in
> kallmodsyms, because that lets kallmodsyms reuse strings where modules
> and their source file have similar names. Something for the future...)
It appeared to me like the symbols added for MODULE_DEVICE_TABLE are
only needed temporarily and could be stripped as part of the final
linking step. This would make space less of a concern, but extern
variables don't support the visibility attribute and in the build I am
using the space difference is less than 1MB out of 613MB for the
uncompressed kernel.
>
> > You folks may want to Cc each other on your patches.
>
> I'd welcome that.
>
> btw, do you want another kallmodsyms patch series from me just arranging
> to drop fewer MODULE_ entries from non-modules (just MODULE_LICENSE) or
> would this be considered noise for now? (Are we deadlocked on each
> other, or are you still looking at the last series I sent, which I think
> was v10 in late November?)
For now I just need MODULE_DEVICE_TABLE to stick around for USB and
thunderbolt related modules (including built-in modules), so if you
aren't removing it for any then I don't think we are blocking each
other.
Longer term it makes sense to have MODULE_DEVICE_TABLE for any module
that makes use of a subsystem that had the authorized attribute. While
this is currently just USB/thunderbolt it could expand in the future,
but there are subsystems where it is likely to make no difference.
We might have a tiny amount of redundancy in our patch sets because
there are some cases of invalid MODULE_DEVICE_TABLE entries I fixed in
my patch series, but that could be dropped. These have the potential
for conflicts / blocking each other, but it should be easy to resolve
them if I change my fixes to a removal of the MODULE_DEVICE_TABLE
entries.
>
> --
> NULL && (void)
On Tue, Dec 20, 2022 at 12:47 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> On Tue, Dec 20, 2022 at 12:19:49PM -0600, Allen Webb wrote:
> > On Tue, Dec 20, 2022 at 12:12 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> > >
> > > On Tue, Dec 20, 2022 at 08:58:36AM -0600, Allen Webb wrote:
> > > > As mentioned in a different sub-thread this cannot be built as a
> > > > module so I updated the commit message to:
> > > >
> > > > imx: Fix typo
> > > >
> > > > A one character difference in the name supplied to MODULE_DEVICE_TABLE
> > > > breaks compilation for SOC_IMX8M after built-in modules can generate
> > > > match-id based module aliases, so fix the typo.
> > >
> > > Are you saying that it is a typo *now* only, and fixing it does not fix
> > > compilation now, but that fixing the typo will fix a future compilation
> > > issue once your patches get merged for built-in module aliases?
> >
> > It was always a typo, it just doesn't affect the build because
> > MODULE_DEVICE_TABLE is not doing anything for built-in modules before
> > this patch series.
> >
> > >
> > > > This was not caught earlier because SOC_IMX8M can not be built as a
> > > > module and MODULE_DEVICE_TABLE is a no-op for built-in modules.
> > >
> > > Odd, so why did it use MODULE_DEVICE_TABLE then? What was the reason for
> > > the driver having MODULE_DEVICE_TABLE if it was a no-op?
> >
> > That is a good question. I can only speculate as to the answer
>
> You can use git blame to trace back to the original commit that added
> it, then use git blame foo.c commit-id~1 on the file to keep going
> back until you get to the first commit that added that entry, check out
> that as a branch and see if the driver was still not a module then
> (tristate). If so then your speculation is very likely accurate and
> can be spelled out in the commit log message.
All three cases are bool configs.
>
> It begs the inverse question too though, you are finding uses of
> built-in-always code (never tristate) which uses MODULE_DEVICE_TABLE().
> Although today that's a no-op, after your changes this becomes useful
> information, so do you need to scrape to see what built-in-aways code
> *do* not use MODULE_DEVICE_TABLE() where after your patches it would
> have become useful?
>
> Determing if there is value to that endeavour should be easily grasped by
> reading the description you are adding to MODULE_DEVICE_TABLE() --
> in your patch 5 "module.h: MODULE_DEVICE_TABLE for built-in modules".
> Driver developers for built-in-always code should read that description
> and be able to decide if they should use it or not. But even your latest
> replies to Greg do not make that clear, *I personally gather* rather that
> this would in no way shape or form be useful. But is that true?
I took another stab at clarifying (and also dropped the ifdev since
the same macro works both for separate and built-in modules:
/*
* Creates an alias so file2alias.c can find device table.
*
* Use this in cases where a device table is used to match devices because it
* surfaces match-id based module aliases to userspace for:
* - Automatic module loading.
* - Tools like USBGuard which allow or block devices based on policy such as
* which modules match a device.
*
* The module name is included in the alias for two reasons:
* - It avoids creating two aliases with the same name for built-in modules.
* Historically MODULE_DEVICE_TABLE was a no-op for built-in modules, so
* there was nothing to stop different modules from having the same device
* table name and consequently the same alias when building as a module.
* - The module name is needed by files2alias.c to associate a particular
* device table with its associated module for built-in modules since
* files2alias would otherwise see the module name as `vmlinuz.o`.
*/
#define MODULE_DEVICE_TABLE(type, name) \
extern void *CONCATENATE( \
CONCATENATE(__mod_##type##__##name##__, \
__KBUILD_MODNAME), \
_device_table) \
__attribute__ ((unused, alias(__stringify(name))))
>
> So why not just remove MODULE_DEVICE_TABLE() from code we know is
> built-in-always code instead of fixing a typo just to fix a future
> compile issue?
>
> Then your commit log is not about "fix typo", but rather remove a no-op
> macro as the driver is always built-in and keeping that macro would not
> help built-in code.
The deciding factor in whether it makes sense to remove these vs fix
them seems to be, "How complete do we want modules.builtin.alias to
be?"
Arguably we should just drop these in cases where there isn't an
"authorized" sysfs attribute but following that logic there is not any
reason to generate built-in aliases for anything except USB and
thunderbolt.
On the flip side, if we are going to the effort to make this a generic
solution that covers everything, the built-in aliases are only as
useful as they are complete, so we would want everything that defines
a device table to call the macro correctly.
>
> > but it
> > is plausible people copied a common pattern and since no breakage was
> > noticed left it as is.
>
> This level of clarity is important to spell out in the commit log
> message, specially if you are suggesting it is just a typo fix. Because
> I will take it for granted that it is just that.
>
> If it fixes a future use case where the typo would be more of an issue,
> you can mention that in a secondary paragraph or sentence.
>
> > It also raises the question how many modules have device tables, but
> > do not call MODULE_DEVICE_TABLE since they are only ever built-in.
> > Maybe there should be some build time enforcement mechanism to make
> > sure that these are consistent.
>
> Definitely, Nick Alcock is doing some related work where the semantics
> of built-in modules needs to be clearer, he for instance is now removing
> a few MODULE_() macros for things which *are never* modules, and this is
> because after commit 8b41fc4454e ("kbuild: create modules.builtin
> without Makefile.modbuiltin or tristate.conf") we rely on the module
> license tag to generate the modules.builtin file. Without that commit
> we end up traversing the source tree twice. Nick's work builds on
> that work and futher clarifies these semantics by adding tooling which
> complains when something which is *never* capable of being a module
> uses module macros. The macro you are extending, MODULE_DEVICE_TABLE(),
> today is a no-op for built-in, but you are adding support to extend it
> for built-in stuff. Nick's work will help with clarifying symbol locality
> and so he may be interested in your association for the data in
> MODULE_DEVICE_TABLE and how you associate to a respective would-be
> module. His work is useful for making tracing more accurate with respect
> to symbol associations, so the data in MODULE_DEVICE_TABLE() may be
> useful as well to him.
Thanks, I will look through what I can find.
>
> You folks may want to Cc each other on your patches.
>
> If we know for certain things *will never* be used or *should not be
> used*, as in the case of the module license tag, we should certainly
> have tooling to pick up on that crap to help us tidy it up.
>
> If you determine MODULE_DEVICE_TABLE() *should* not be used for built-in
> always code (never tristate) then you and Nick likely have overlap of
> macros to tidy up and tooling to share to spot these sort of issues.
It definitely is needed for never-tristate modules that match devices
in subsystems that surface the authorized attribute.
>
> Luis
On Tue, Dec 20, 2022 at 01:49:04PM -0600, Allen Webb wrote: > I took another stab at clarifying (and also dropped the ifdev since > the same macro works both for separate and built-in modules: > > /* > * Creates an alias so file2alias.c can find device table. > * > * Use this in cases where a device table is used to match devices because it > * surfaces match-id based module aliases to userspace for: > * - Automatic module loading. > * - Tools like USBGuard which allow or block devices based on policy such as > * which modules match a device. > * > * The module name is included in the alias for two reasons: > * - It avoids creating two aliases with the same name for built-in modules. > * Historically MODULE_DEVICE_TABLE was a no-op for built-in modules, so > * there was nothing to stop different modules from having the same device > * table name and consequently the same alias when building as a module. > * - The module name is needed by files2alias.c to associate a particular > * device table with its associated module for built-in modules since > * files2alias would otherwise see the module name as `vmlinuz.o`. > */ This is still weak in light of the questions I had. It does not make it easy for a driver developer who is going to support only built-in only if they need to define this or not. And it seems we're still discussing the merits of this, so I'd wait until this is fleshed out, but I think we are on the right track finally. > The deciding factor in whether it makes sense to remove these vs fix > them seems to be, "How complete do we want modules.builtin.alias to > be?" > > Arguably we should just drop these in cases where there isn't an > "authorized" sysfs attribute but following that logic there is not any > reason to generate built-in aliases for anything except USB and > thunderbolt. There we go, now we have a *real* use case for this for built-in stuff to consider. Is USBGuard effective even for built-in stuff? Given everything discussed so far I'd like to get clarification if it even help for built-in USB / thunderbolt. Does it? If so how? What could userspace do with this information if the driver is already built-in? > On the flip side, if we are going to the effort to make this a generic > solution that covers everything, the built-in aliases are only as > useful as they are complete, so we would want everything that defines > a device table to call the macro correctly. It is the ambiguity which is terrible to add. If the only use case is for USB and Thunderbolt then we can spell it out, then only those driver developers would care to consider it if the driver is bool. And, a respective tooling would scrape only those drivers to verify if the table is missing for built-in too. > It definitely is needed for never-tristate modules that match devices > in subsystems that surface the authorized attribute. What is this "authorized attribute" BTW exactly? Do have some documentation reference? Luis
On Tue, Dec 20, 2022 at 2:03 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Tue, Dec 20, 2022 at 01:49:04PM -0600, Allen Webb wrote: > > I took another stab at clarifying (and also dropped the ifdev since > > the same macro works both for separate and built-in modules: > > > > /* > > * Creates an alias so file2alias.c can find device table. > > * > > * Use this in cases where a device table is used to match devices because it > > * surfaces match-id based module aliases to userspace for: > > * - Automatic module loading. > > * - Tools like USBGuard which allow or block devices based on policy such as > > * which modules match a device. > > * > > * The module name is included in the alias for two reasons: > > * - It avoids creating two aliases with the same name for built-in modules. > > * Historically MODULE_DEVICE_TABLE was a no-op for built-in modules, so > > * there was nothing to stop different modules from having the same device > > * table name and consequently the same alias when building as a module. > > * - The module name is needed by files2alias.c to associate a particular > > * device table with its associated module for built-in modules since > > * files2alias would otherwise see the module name as `vmlinuz.o`. > > */ > > This is still weak in light of the questions I had. It does not make it > easy for a driver developer who is going to support only built-in only > if they need to define this or not. And it seems we're still discussing > the merits of this, so I'd wait until this is fleshed out, but I think > we are on the right track finally. > > > The deciding factor in whether it makes sense to remove these vs fix > > them seems to be, "How complete do we want modules.builtin.alias to > > be?" > > > > Arguably we should just drop these in cases where there isn't an > > "authorized" sysfs attribute but following that logic there is not any > > reason to generate built-in aliases for anything except USB and > > thunderbolt. > > There we go, now we have a *real* use case for this for built-in stuff > to consider. Is USBGuard effective even for built-in stuff? Yes, just because a module is loaded doesn't mean a specific device has probed the driver yet. > > Given everything discussed so far I'd like to get clarification if it > even help for built-in USB / thunderbolt. Does it? If so how? What could > userspace do with this information if the driver is already built-in? We are not trying to stop the module from being loaded (which is always the case for built-in modules) and in fact it is possible to have devices already using the module and still not authorize (and by extension probe the module for) newly connected devices. For example someone might have an unattended computer downloading installation media to a USB drive. Presumably this computer would be locked to make it more difficult for a bad actor to access the computer. Since USB storage devices are not needed to interact with the lock screen, we can use the authorized_default sysfs attribute to not allow new USB devices to probe modules by default and have USBGuard vet the devices. Mice, keyboards, etc can be allowed so that the lock screen can still be used (this important in cases like suspend+resume or docks). > > > On the flip side, if we are going to the effort to make this a generic > > solution that covers everything, the built-in aliases are only as > > useful as they are complete, so we would want everything that defines > > a device table to call the macro correctly. > > It is the ambiguity which is terrible to add. If the only use case is > for USB and Thunderbolt then we can spell it out, then only those driver > developers would care to consider it if the driver is bool. And, a > respective tooling would scrape only those drivers to verify if the > table is missing for built-in too. I was aiming to write it so that it wouldn't easily become obsolete by later changes, so tying it to the authorized and authorized_default sysfs attributes is probably the ideal deciding factor and listing USB and thunderbolt as examples makes sense. > > > It definitely is needed for never-tristate modules that match devices > > in subsystems that surface the authorized attribute. > > What is this "authorized attribute" BTW exactly? Do have some > documentation reference? There are sysfs attributes called authorized and authorized_default that together can prevent devices from being fully enumerated and probed. authorized_default gets set to 0 for the hub and any devices connected after that will show in sysfs, but not fully enumerate or probe until the device's authorized attribute is set to 1. There are some edge cases like internal devices which have some extra complexity. As for documentation, I wasn't able to find much other than: https://github.com/torvalds/linux/blob/v6.1/drivers/usb/core/hcd.c#L370 /* authorized_default behaviour: * -1 is authorized for all devices except wireless (old behaviour) * 0 is unauthorized for all devices * 1 is authorized for all devices * 2 is authorized for internal devices */ ... and https://github.com/torvalds/linux/blob/v6.1/Documentation/admin-guide/kernel-parameters.txt#L6424 usbcore.authorized_default= [USB] Default USB device authorization: (default -1 = authorized except for wireless USB, 0 = not authorized, 1 = authorized, 2 = authorized if device connected to internal port) ... The feature looks like it was originally introduced for wireless USB in: https://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg54289.html and later adapted for use cases like USBGuard here: https://github.com/torvalds/linux/commit/c4fc2342cb611f945fa468e742759e25984005ad > > Luis
On Tue, Dec 20, 2022 at 03:57:57PM -0600, Allen Webb wrote: > On Tue, Dec 20, 2022 at 2:03 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > On Tue, Dec 20, 2022 at 01:49:04PM -0600, Allen Webb wrote: > > > I took another stab at clarifying (and also dropped the ifdev since > > > the same macro works both for separate and built-in modules: > > > > > > /* > > > * Creates an alias so file2alias.c can find device table. > > > * > > > * Use this in cases where a device table is used to match devices because it > > > * surfaces match-id based module aliases to userspace for: > > > * - Automatic module loading. > > > * - Tools like USBGuard which allow or block devices based on policy such as > > > * which modules match a device. > > > * > > > * The module name is included in the alias for two reasons: > > > * - It avoids creating two aliases with the same name for built-in modules. > > > * Historically MODULE_DEVICE_TABLE was a no-op for built-in modules, so > > > * there was nothing to stop different modules from having the same device > > > * table name and consequently the same alias when building as a module. > > > * - The module name is needed by files2alias.c to associate a particular > > > * device table with its associated module for built-in modules since > > > * files2alias would otherwise see the module name as `vmlinuz.o`. > > > */ > > > > This is still weak in light of the questions I had. It does not make it > > easy for a driver developer who is going to support only built-in only > > if they need to define this or not. And it seems we're still discussing > > the merits of this, so I'd wait until this is fleshed out, but I think > > we are on the right track finally. > > > > > The deciding factor in whether it makes sense to remove these vs fix > > > them seems to be, "How complete do we want modules.builtin.alias to > > > be?" > > > > > > Arguably we should just drop these in cases where there isn't an > > > "authorized" sysfs attribute but following that logic there is not any > > > reason to generate built-in aliases for anything except USB and > > > thunderbolt. > > > > There we go, now we have a *real* use case for this for built-in stuff > > to consider. Is USBGuard effective even for built-in stuff? > > Yes, just because a module is loaded doesn't mean a specific device > has probed the driver yet. > > > > > Given everything discussed so far I'd like to get clarification if it > > even help for built-in USB / thunderbolt. Does it? If so how? What could > > userspace do with this information if the driver is already built-in? > > We are not trying to stop the module from being loaded (which is > always the case for built-in modules) and in fact it is possible to > have devices already using the module and still not authorize (and by > extension probe the module for) newly connected devices. > > For example someone might have an unattended computer downloading > installation media to a USB drive. Presumably this computer would be > locked to make it more difficult for a bad actor to access the > computer. Since USB storage devices are not needed to interact with > the lock screen, we can use the authorized_default sysfs attribute to > not allow new USB devices to probe modules by default and have > USBGuard vet the devices. Mice, keyboards, etc can be allowed so that > the lock screen can still be used (this important in cases like > suspend+resume or docks). I see thanks! > > > On the flip side, if we are going to the effort to make this a generic > > > solution that covers everything, the built-in aliases are only as > > > useful as they are complete, so we would want everything that defines > > > a device table to call the macro correctly. > > > > It is the ambiguity which is terrible to add. If the only use case is > > for USB and Thunderbolt then we can spell it out, then only those driver > > developers would care to consider it if the driver is bool. And, a > > respective tooling would scrape only those drivers to verify if the > > table is missing for built-in too. > > I was aiming to write it so that it wouldn't easily become obsolete by > later changes, so tying it to the authorized and authorized_default > sysfs attributes is probably the ideal deciding factor and listing USB > and thunderbolt as examples makes sense. I think it would make sense then to be explicit about this for now, even if it seems we can obsolete this. Right now the justification for having this for built-in is *very* specific to this feature for USB, which makes use of special USB sysfs attributes which as you explained, allows to restrict probe of devices even though the respective driver is already loaded. > There are sysfs attributes called authorized and authorized_default > that together can prevent devices from being fully enumerated and > probed. Although these attributes are USB specfic today it gets me wondering if other subsystems may benefit from a similar feature. > authorized_default gets set to 0 for the hub and any devices > connected after that will show in sysfs, but not fully enumerate or > probe until the device's authorized attribute is set to 1. There are > some edge cases like internal devices which have some extra > complexity. > > As for documentation, I wasn't able to find much other than: > https://github.com/torvalds/linux/blob/v6.1/drivers/usb/core/hcd.c#L370 > /* authorized_default behaviour: > * -1 is authorized for all devices except wireless (old behaviour) > * 0 is unauthorized for all devices > * 1 is authorized for all devices > * 2 is authorized for internal devices > */ > ... > and > https://github.com/torvalds/linux/blob/v6.1/Documentation/admin-guide/kernel-parameters.txt#L6424 > usbcore.authorized_default= > [USB] Default USB device authorization: > (default -1 = authorized except for wireless USB, > 0 = not authorized, 1 = authorized, 2 = authorized > if device connected to internal port) > ... > The feature looks like it was originally introduced for wireless USB in: > https://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg54289.html > and later adapted for use cases like USBGuard here: > https://github.com/torvalds/linux/commit/c4fc2342cb611f945fa468e742759e25984005ad Thanks for digging all this up. Can you extend the docs on Documentation/driver-api/usb/ somewhere about this attribute as part of your changes so its clear the motivation, *then* you make your changes. The documentation for MODULE_DEVICE_TABLE() can just say: The only use-case for built-in drivers today is enable userspace to prevent / allow probe for devices on certain subsystems even if the driver is already loaded. An example is the USB subsystem with its authorized_default sysfs attribute. For more details refer to the kernel's Documentation for USB about authorized_default. That should be clear enough for both USB driver writers and others. Please also extend the docs for MODULE_DEVICE_TABLE() on Documentation/driver-api/usb/writing_usb_driver.rst or where you see fit for your changes. That can go into depth about the USBGuard stuff. Luis
On Tue, Dec 20, 2022 at 5:09 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Tue, Dec 20, 2022 at 03:57:57PM -0600, Allen Webb wrote: > > On Tue, Dec 20, 2022 at 2:03 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > > > On Tue, Dec 20, 2022 at 01:49:04PM -0600, Allen Webb wrote: > > > > I took another stab at clarifying (and also dropped the ifdev since > > > > the same macro works both for separate and built-in modules: > > > > > > > > /* > > > > * Creates an alias so file2alias.c can find device table. > > > > * > > > > * Use this in cases where a device table is used to match devices because it > > > > * surfaces match-id based module aliases to userspace for: > > > > * - Automatic module loading. > > > > * - Tools like USBGuard which allow or block devices based on policy such as > > > > * which modules match a device. > > > > * > > > > * The module name is included in the alias for two reasons: > > > > * - It avoids creating two aliases with the same name for built-in modules. > > > > * Historically MODULE_DEVICE_TABLE was a no-op for built-in modules, so > > > > * there was nothing to stop different modules from having the same device > > > > * table name and consequently the same alias when building as a module. > > > > * - The module name is needed by files2alias.c to associate a particular > > > > * device table with its associated module for built-in modules since > > > > * files2alias would otherwise see the module name as `vmlinuz.o`. > > > > */ > > > > > > This is still weak in light of the questions I had. It does not make it > > > easy for a driver developer who is going to support only built-in only > > > if they need to define this or not. And it seems we're still discussing > > > the merits of this, so I'd wait until this is fleshed out, but I think > > > we are on the right track finally. > > > > > > > The deciding factor in whether it makes sense to remove these vs fix > > > > them seems to be, "How complete do we want modules.builtin.alias to > > > > be?" > > > > > > > > Arguably we should just drop these in cases where there isn't an > > > > "authorized" sysfs attribute but following that logic there is not any > > > > reason to generate built-in aliases for anything except USB and > > > > thunderbolt. > > > > > > There we go, now we have a *real* use case for this for built-in stuff > > > to consider. Is USBGuard effective even for built-in stuff? > > > > Yes, just because a module is loaded doesn't mean a specific device > > has probed the driver yet. > > > > > > > > Given everything discussed so far I'd like to get clarification if it > > > even help for built-in USB / thunderbolt. Does it? If so how? What could > > > userspace do with this information if the driver is already built-in? > > > > We are not trying to stop the module from being loaded (which is > > always the case for built-in modules) and in fact it is possible to > > have devices already using the module and still not authorize (and by > > extension probe the module for) newly connected devices. > > > > For example someone might have an unattended computer downloading > > installation media to a USB drive. Presumably this computer would be > > locked to make it more difficult for a bad actor to access the > > computer. Since USB storage devices are not needed to interact with > > the lock screen, we can use the authorized_default sysfs attribute to > > not allow new USB devices to probe modules by default and have > > USBGuard vet the devices. Mice, keyboards, etc can be allowed so that > > the lock screen can still be used (this important in cases like > > suspend+resume or docks). > > I see thanks! > > > > > On the flip side, if we are going to the effort to make this a generic > > > > solution that covers everything, the built-in aliases are only as > > > > useful as they are complete, so we would want everything that defines > > > > a device table to call the macro correctly. > > > > > > It is the ambiguity which is terrible to add. If the only use case is > > > for USB and Thunderbolt then we can spell it out, then only those driver > > > developers would care to consider it if the driver is bool. And, a > > > respective tooling would scrape only those drivers to verify if the > > > table is missing for built-in too. > > > > I was aiming to write it so that it wouldn't easily become obsolete by > > later changes, so tying it to the authorized and authorized_default > > sysfs attributes is probably the ideal deciding factor and listing USB > > and thunderbolt as examples makes sense. > > I think it would make sense then to be explicit about this for now, even > if it seems we can obsolete this. Right now the justification for having > this for built-in is *very* specific to this feature for USB, which > makes use of special USB sysfs attributes which as you explained, allows > to restrict probe of devices even though the respective driver is already > loaded. The thing we might obsolete is limiting it to just the USB subsystem. I am fine with expanding the documentation and limiting the scope of the feature to USB/thunderbolt for now. > > > There are sysfs attributes called authorized and authorized_default > > that together can prevent devices from being fully enumerated and > > probed. > > Although these attributes are USB specfic today it gets me wondering if > other subsystems may benefit from a similar feature. The subsystems that would likely benefit the most are ones that are externally reachable. The external ports that come to mind are USB / thunderbolt, firewire, PCMCIA / expresscard, eSATA, serial and parallel ports. Supporting PCMCIA / expresscard seems like it would require adding the authorized sysfs attribute to pci. eSATA would be covered by ata. > > > authorized_default gets set to 0 for the hub and any devices > > connected after that will show in sysfs, but not fully enumerate or > > probe until the device's authorized attribute is set to 1. There are > > some edge cases like internal devices which have some extra > > complexity. > > > > As for documentation, I wasn't able to find much other than: > > https://github.com/torvalds/linux/blob/v6.1/drivers/usb/core/hcd.c#L370 > > /* authorized_default behaviour: > > * -1 is authorized for all devices except wireless (old behaviour) > > * 0 is unauthorized for all devices > > * 1 is authorized for all devices > > * 2 is authorized for internal devices > > */ > > ... > > and > > https://github.com/torvalds/linux/blob/v6.1/Documentation/admin-guide/kernel-parameters.txt#L6424 > > usbcore.authorized_default= > > [USB] Default USB device authorization: > > (default -1 = authorized except for wireless USB, > > 0 = not authorized, 1 = authorized, 2 = authorized > > if device connected to internal port) > > ... > > The feature looks like it was originally introduced for wireless USB in: > > https://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg54289.html > > and later adapted for use cases like USBGuard here: > > https://github.com/torvalds/linux/commit/c4fc2342cb611f945fa468e742759e25984005ad > > Thanks for digging all this up. Can you extend the docs on > Documentation/driver-api/usb/ somewhere about this attribute as part of > your changes so its clear the motivation, *then* you make your changes. > The documentation for MODULE_DEVICE_TABLE() can just say: > > The only use-case for built-in drivers today is enable userspace to > prevent / allow probe for devices on certain subsystems even if the > driver is already loaded. An example is the USB subsystem with its > authorized_default sysfs attribute. For more details refer to the > kernel's Documentation for USB about authorized_default. > > That should be clear enough for both USB driver writers and others. > > Please also extend the docs for MODULE_DEVICE_TABLE() on > Documentation/driver-api/usb/writing_usb_driver.rst or where you see > fit for your changes. That can go into depth about the USBGuard stuff. > > Luis How do you feel about only having one version of the macro for both cases and merging the documentation so things are kept simple? Here is what I have locally for the macro without the ifdef and the updated documentation: /* * Creates an alias so file2alias.c can find device table. * * Use this in cases where a device table is used to match devices because it * surfaces match-id based module aliases to userspace for: * - Automatic module loading through modules.alias. * - Tools like USBGuard which allow or block devices based on policy such as * which modules match a device. * * The only use-case for built-in drivers today is enable userspace to prevent / * allow probe for devices on certain subsystems even if the driver is already * loaded. An example is the USB subsystem with its authorized_default sysfs * attribute. For more details refer to the kernel's Documentation for USB about * authorized_default. * * The module name is included in the alias for two reasons: * - It avoids creating two aliases with the same name for built-in modules. * Historically MODULE_DEVICE_TABLE was a no-op for built-in modules, so * there was nothing to stop different modules from having the same device * table name and consequently the same alias when building as a module. * - The module name is needed by files2alias.c to associate a particular * device table with its associated module for built-in modules since * files2alias would otherwise see the module name as `vmlinuz.o`. */ #define MODULE_DEVICE_TABLE(type, name) \ extern void *CONCATENATE( \ CONCATENATE(__mod_##type##__##name##__, \ __KBUILD_MODNAME), \ _device_table) \ __attribute__ ((unused, alias(__stringify(name)))) Here is a draft version for an updated to Documentation/driver-api/usb/ (I will add the 80 char line breaks later) in case you have feedback: # Authorization Authorization provides userspace a way to allow or block configuring devices early during enumeration before any modules are probed for the device. While it is possible to block a device by not loading the required modules, this also prevents other devices from using the module as well. For example someone might have an unattended computer downloading installation media to a USB drive. Presumably this computer would be locked to make it more difficult for a bad actor to access the computer. Since USB storage devices are not needed to interact with the lock screen, the authorized_default sysfs attribute can be set to not authorize new USB devices by default. A userspace tool like USBGuard can then vet the devices. Mice, keyboards, etc can be allowed by writing to their authorized sysfs attribute so that the lock screen can still be used (this important in cases like suspend+resume or docks) while other devices can be blocked as long as the lock screen is shown. ## Sysfs Attributes Userspace can control USB device authorization through the authorized_default and authorized sysfs attributes. ### authorized_default .. kernel-doc:: drivers/usb/core/hcd.c :export: The authorized_default sysfs attribute is only present for host controllers. It determines the initial state of the authorized sysfs attribute of USB devices newly connected to the corresponding host controller. It can take on the following values: +---------------------------------------------------+ | Value | Behavior | +=======+===========================================+ | -1 | Authorize all devices except wireless USB | +-------+-------------------------------------------+ | 0 | Do not authorize new devices | +-------+-------------------------------------------+ | 1 | Authorize new devices | +-------+-------------------------------------------+ | 2 | Authorize new internal devices only | +---------------------------------------------------+ Note that firmware platform code determines if a device is internal or not and this is reported as the connect_type sysfs attribute of the USB port. This is currently supported by ACPI, but device tree still needs an implementation. Authorizing new internal devices only can be useful to work around issues with devices that misbehave if there are delays in probing their module. ### authorized .. kernel-doc:: drivers/usb/core/sysfs.c :export: Every USB device has an authorized sysfs attribute which can take the values 0 and 1. When authorized is 0, the device still is present in sysfs, but none of its interfaces can be associated with drivers and modules will not be probed. When authorized is 1 (or set to one) a configuration is chosen for the device and its interfaces are registered allowing drivers to bind to them.
On Tue, Dec 27, 2022 at 11:42:36AM -0600, Allen Webb wrote:
> On Tue, Dec 20, 2022 at 5:09 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> > I think it would make sense then to be explicit about this for now, even
> > if it seems we can obsolete this. Right now the justification for having
> > this for built-in is *very* specific to this feature for USB, which
> > makes use of special USB sysfs attributes which as you explained, allows
> > to restrict probe of devices even though the respective driver is already
> > loaded.
>
> The thing we might obsolete is limiting it to just the USB subsystem.
> I am fine with expanding the documentation and limiting the scope of
> the feature to USB/thunderbolt for now.
Great let's do that as otherwise it can leave a few folks scratchign
their head.
> > > There are sysfs attributes called authorized and authorized_default
> > > that together can prevent devices from being fully enumerated and
> > > probed.
> >
> > Although these attributes are USB specfic today it gets me wondering if
> > other subsystems may benefit from a similar feature.
>
> The subsystems that would likely benefit the most are ones that are
> externally reachable.
Makes sense.
> The external ports that come to mind are USB /
> thunderbolt, firewire, PCMCIA / expresscard, eSATA, serial and
> parallel ports. Supporting PCMCIA / expresscard seems like it would
> require adding the authorized sysfs attribute to pci. eSATA would be
> covered by ata.
Makes sense, I'd personally ignore anything legacy such as PCMCIA though.
> > > authorized_default gets set to 0 for the hub and any devices
> > > connected after that will show in sysfs, but not fully enumerate or
> > > probe until the device's authorized attribute is set to 1. There are
> > > some edge cases like internal devices which have some extra
> > > complexity.
> > >
> > > As for documentation, I wasn't able to find much other than:
> > > https://github.com/torvalds/linux/blob/v6.1/drivers/usb/core/hcd.c#L370
> > > /* authorized_default behaviour:
> > > * -1 is authorized for all devices except wireless (old behaviour)
> > > * 0 is unauthorized for all devices
> > > * 1 is authorized for all devices
> > > * 2 is authorized for internal devices
> > > */
> > > ...
> > > and
> > > https://github.com/torvalds/linux/blob/v6.1/Documentation/admin-guide/kernel-parameters.txt#L6424
> > > usbcore.authorized_default=
> > > [USB] Default USB device authorization:
> > > (default -1 = authorized except for wireless USB,
> > > 0 = not authorized, 1 = authorized, 2 = authorized
> > > if device connected to internal port)
> > > ...
> > > The feature looks like it was originally introduced for wireless USB in:
> > > https://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg54289.html
> > > and later adapted for use cases like USBGuard here:
> > > https://github.com/torvalds/linux/commit/c4fc2342cb611f945fa468e742759e25984005ad
> >
> > Thanks for digging all this up. Can you extend the docs on
> > Documentation/driver-api/usb/ somewhere about this attribute as part of
> > your changes so its clear the motivation, *then* you make your changes.
> > The documentation for MODULE_DEVICE_TABLE() can just say:
> >
> > The only use-case for built-in drivers today is enable userspace to
> > prevent / allow probe for devices on certain subsystems even if the
> > driver is already loaded. An example is the USB subsystem with its
> > authorized_default sysfs attribute. For more details refer to the
> > kernel's Documentation for USB about authorized_default.
> >
> > That should be clear enough for both USB driver writers and others.
> >
> > Please also extend the docs for MODULE_DEVICE_TABLE() on
> > Documentation/driver-api/usb/writing_usb_driver.rst or where you see
> > fit for your changes. That can go into depth about the USBGuard stuff.
> >
> > Luis
>
> How do you feel about only having one version of the macro for both
> cases and merging the documentation so things are kept simple? Here is
> what I have locally for the macro without the ifdef and the updated
> documentation:
>
> /*
> * Creates an alias so file2alias.c can find device table.
> *
> * Use this in cases where a device table is used to match devices because it
> * surfaces match-id based module aliases to userspace for:
> * - Automatic module loading through modules.alias.
> * - Tools like USBGuard which allow or block devices based on policy such as
^ allow to
> * which modules match a device.
> *
> * The only use-case for built-in drivers today is enable userspace to prevent /
^ is to
> * allow probe for devices on certain subsystems even if the driver is already
> * loaded. An example is the USB subsystem with its authorized_default sysfs
> * attribute. For more details refer to the kernel's Documentation for USB about
> * authorized_default.
> *
> * The module name is included in the alias for two reasons:
> * - It avoids creating two aliases with the same name for built-in modules.
> * Historically MODULE_DEVICE_TABLE was a no-op for built-in modules, so
> * there was nothing to stop different modules from having the same device
> * table name and consequently the same alias when building as a module.
> * - The module name is needed by files2alias.c to associate a particular
> * device table with its associated module for built-in modules since
> * files2alias would otherwise see the module name as `vmlinuz.o`.
Yeah sure this reads much better.
> */
> #define MODULE_DEVICE_TABLE(type, name) \
> extern void *CONCATENATE( \
> CONCATENATE(__mod_##type##__##name##__, \
> __KBUILD_MODNAME), \
> _device_table) \
> __attribute__ ((unused, alias(__stringify(name))))
>
>
> Here is a draft version for an updated to
> Documentation/driver-api/usb/ (I will add the 80 char line breaks
> later) in case you have feedback:
>
>
> # Authorization
>
> Authorization provides userspace a way to allow or block configuring
> devices early during enumeration before any modules are probed for the
> device. While it is possible to block a device by not loading the
> required modules, this also prevents other devices from using the
> module as well. For example someone might have an unattended computer
> downloading installation media to a USB drive. Presumably this
> computer would be locked to make it more difficult for a bad actor to
> access the computer. Since USB storage devices are not needed to
> interact with the lock screen, the authorized_default sysfs attribute
> can be set to not authorize new USB devices by default. A userspace
> tool like USBGuard can then vet the devices. Mice, keyboards, etc can
> be allowed by writing to their authorized sysfs attribute so that the
> lock screen can still be used (this important in cases like
> suspend+resume or docks) while other devices can be blocked as long as
> the lock screen is shown.
>
> ## Sysfs Attributes
>
> Userspace can control USB device authorization through the
> authorized_default and authorized sysfs attributes.
>
> ### authorized_default
>
> .. kernel-doc:: drivers/usb/core/hcd.c
> :export:
>
> The authorized_default sysfs attribute is only present for host
> controllers. It determines the initial state of the authorized sysfs
> attribute of USB devices newly connected to the corresponding host
> controller. It can take on the following values:
>
> +---------------------------------------------------+
> | Value | Behavior |
> +=======+===========================================+
> | -1 | Authorize all devices except wireless USB |
> +-------+-------------------------------------------+
> | 0 | Do not authorize new devices |
> +-------+-------------------------------------------+
> | 1 | Authorize new devices |
> +-------+-------------------------------------------+
> | 2 | Authorize new internal devices only |
> +---------------------------------------------------+
>
> Note that firmware platform code determines if a device is internal or
> not and this is reported as the connect_type sysfs attribute of the
> USB port. This is currently supported by ACPI, but device tree still
> needs an implementation. Authorizing new internal devices only can be
> useful to work around issues with devices that misbehave if there are
> delays in probing their module.
>
> ### authorized
>
> .. kernel-doc:: drivers/usb/core/sysfs.c
> :export:
>
> Every USB device has an authorized sysfs attribute which can take the
> values 0 and 1. When authorized is 0, the device still is present in
> sysfs, but none of its interfaces can be associated with drivers and
> modules will not be probed. When authorized is 1 (or set to one) a
> configuration is chosen for the device and its interfaces are
> registered allowing drivers to bind to them.
Good stuff!
Luis
A future patch makes use of the device table for built-in modules, so
do not ifdef out the match id table.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Allen Webb <allenwebb@google.com>
---
drivers/scsi/BusLogic.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index f2abffce2659..0c60867c9e7c 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -3715,7 +3715,6 @@ static void __exit blogic_exit(void)
__setup("BusLogic=", blogic_setup);
-#ifdef MODULE
/*static struct pci_device_id blogic_pci_tbl[] = {
{ PCI_VENDOR_ID_BUSLOGIC, PCI_DEVICE_ID_BUSLOGIC_MULTIMASTER,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
@@ -3731,7 +3730,6 @@ static const struct pci_device_id blogic_pci_tbl[] = {
{PCI_DEVICE(PCI_VENDOR_ID_BUSLOGIC, PCI_DEVICE_ID_BUSLOGIC_FLASHPOINT)},
{0, },
};
-#endif
MODULE_DEVICE_TABLE(pci, blogic_pci_tbl);
module_init(blogic_init);
--
2.37.3A small difference in the name supplied to MODULE_DEVICE_TABLE
breaks a future patch set, so fix the typo.
Cc: stable@vger.kernel.org
Fixes: e789995d5c61 ("mfd: Add support for STMPE SPI interface")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Allen Webb <allenwebb@google.com>
---
drivers/mfd/stmpe-spi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mfd/stmpe-spi.c b/drivers/mfd/stmpe-spi.c
index ad8055a0e286..6791a5368977 100644
--- a/drivers/mfd/stmpe-spi.c
+++ b/drivers/mfd/stmpe-spi.c
@@ -129,7 +129,7 @@ static const struct spi_device_id stmpe_spi_id[] = {
{ "stmpe2403", STMPE2403 },
{ }
};
-MODULE_DEVICE_TABLE(spi, stmpe_id);
+MODULE_DEVICE_TABLE(spi, stmpe_spi_id);
static struct spi_driver stmpe_spi_driver = {
.driver = {
--
2.37.3Implement MODULE_DEVICE_TABLE for build-in modules to make it possible
to generate a builtin.alias file to complement modules.alias.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
include/linux/module.h | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index ec61fb53979a..3d1b04ca6350 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -243,7 +243,20 @@ extern void cleanup_module(void);
extern typeof(name) __mod_##type##__##name##_device_table \
__attribute__ ((unused, alias(__stringify(name))))
#else /* !MODULE */
-#define MODULE_DEVICE_TABLE(type, name)
+/*
+ * The names may not be unique for built-in modules, so include the module name
+ * to guarantee uniqueness.
+ *
+ * Note that extern is needed because modpost reads these symbols to generate
+ * modalias entries for each match id in each device table. They are not used
+ * at runtime.
+ */
+#define MODULE_DEVICE_TABLE(type, name) \
+extern void *CONCATENATE( \
+ CONCATENATE(__mod_##type##__##name##__, \
+ __KBUILD_MODNAME), \
+ _device_table) \
+ __attribute__ ((unused, alias(__stringify(name))))
#endif
/* Version of form [<epoch>:]<version>[-<extra-version>].
--
2.37.3On Mon, Dec 19, 2022 at 02:46:13PM -0600, Allen Webb wrote: > Implement MODULE_DEVICE_TABLE for build-in modules to make it possible > to generate a builtin.alias file to complement modules.alias. > > Signed-off-by: Allen Webb <allenwebb@google.com> > --- > include/linux/module.h | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/include/linux/module.h b/include/linux/module.h > index ec61fb53979a..3d1b04ca6350 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -243,7 +243,20 @@ extern void cleanup_module(void); > extern typeof(name) __mod_##type##__##name##_device_table \ > __attribute__ ((unused, alias(__stringify(name)))) > #else /* !MODULE */ > -#define MODULE_DEVICE_TABLE(type, name) > +/* > + * The names may not be unique for built-in modules, so include the module name > + * to guarantee uniqueness. What "names" are you referring to here with the words, "The names"? And built-in modules have the same rules as external names, they have to be unique so I do not understand the problem you are trying to solve here, which means you need to describe it better in both the changelog text and the comment. > + * > + * Note that extern is needed because modpost reads these symbols to generate > + * modalias entries for each match id in each device table. They are not used > + * at runtime. This comment isn't explaining much about what the #define is to be used for, is it? confused, greg k-h
On Tue, Dec 20, 2022 at 12:45 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Mon, Dec 19, 2022 at 02:46:13PM -0600, Allen Webb wrote: > > Implement MODULE_DEVICE_TABLE for build-in modules to make it possible > > to generate a builtin.alias file to complement modules.alias. > > > > Signed-off-by: Allen Webb <allenwebb@google.com> > > --- > > include/linux/module.h | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/module.h b/include/linux/module.h > > index ec61fb53979a..3d1b04ca6350 100644 > > --- a/include/linux/module.h > > +++ b/include/linux/module.h > > @@ -243,7 +243,20 @@ extern void cleanup_module(void); > > extern typeof(name) __mod_##type##__##name##_device_table \ > > __attribute__ ((unused, alias(__stringify(name)))) > > #else /* !MODULE */ > > -#define MODULE_DEVICE_TABLE(type, name) > > +/* > > + * The names may not be unique for built-in modules, so include the module name > > + * to guarantee uniqueness. > > What "names" are you referring to here with the words, "The names"? > > And built-in modules have the same rules as external names, they have to > be unique so I do not understand the problem you are trying to solve > here, which means you need to describe it better in both the changelog > text and the comment. I changed the comment to: /* * Creates an alias so file2alias.c can find device table for built in modules. * * The module name is included for two reasons: * - Adding the module name to the alias avoids creating two aliases with the * same name. Historically MODULE_DEVICE_TABLE was a no-op for built-in * modules, so there was nothing to stop different modules from having the * same device table name and consequently the same alias when building as a * module. * - The module name is needed by files2alias.c to associate a particular * device table with its associated module since files2alias would otherwise * see the module name as `vmlinuz.o` for built-in modules. */ > > > + * > > + * Note that extern is needed because modpost reads these symbols to generate > > + * modalias entries for each match id in each device table. They are not used > > + * at runtime. > > This comment isn't explaining much about what the #define is to be used > for, is it? I will drop this. I originally added the comment because Christophe Leroy said: "'extern' keyword is pointless of function prototypes and deprecated. Don't add new occurences." This is clearly not a typical function prototype and the guidance from: https://www.kernel.org/doc/html/latest/process/coding-style.html#function-prototypes should not apply. > > confused, > > greg k-h
Keep track of the module name when processing match table symbols.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
scripts/mod/file2alias.c | 39 +++++++++++++++++++++++++++++++++++----
scripts/mod/modpost.h | 1 +
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 80d973144fde..e41ff8de7a87 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -28,6 +28,7 @@ typedef Elf64_Addr kernel_ulong_t;
#include <stdint.h>
#endif
+#include <assert.h>
#include <ctype.h>
#include <stdbool.h>
@@ -1540,9 +1541,9 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
Elf_Sym *sym, const char *symname)
{
void *symval;
- char *zeros = NULL;
- const char *name, *identifier;
- unsigned int namelen;
+ char *zeros = NULL, *modname_str = NULL;
+ const char *name, *identifier, *modname;
+ unsigned int namelen, modnamelen;
/* We're looking for a section relative symbol */
if (!sym->st_shndx || get_secindex(info, sym) >= info->num_sections)
@@ -1552,7 +1553,12 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
return;
- /* All our symbols are of form __mod_<name>__<identifier>_device_table. */
+ /*
+ * All our symbols are either of form
+ * __mod_<name>__<identifier>_device_table
+ * or
+ * __mod_<name>__<identifier>__kmod_<builtin-name>_device_table
+ */
if (strncmp(symname, "__mod_", strlen("__mod_")))
return;
name = symname + strlen("__mod_");
@@ -1564,8 +1570,30 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
identifier = strstr(name, "__");
if (!identifier)
return;
+ modnamelen = namelen;
namelen = identifier - name;
+ /*
+ * In the vmlinuz.o case we want to handle __kmod_ so aliases from
+ * builtin modules are attributed correctly.
+ */
+ modname = strstr(identifier + 2, "__kmod_");
+ if (modname) {
+ modname += strlen("__kmod_");
+ modnamelen -= (modname - name) + strlen("_device_table");
+ modname_str = malloc(modnamelen + 1);
+ /* We don't want to continue if the allocation fails. */
+ assert(modname_str);
+ memcpy(modname_str, modname, modnamelen);
+ modname_str[modnamelen] = '\0';
+ }
+
+ if (modname_str)
+ modname = modname_str;
+ else
+ modname = mod->name;
+ mod->builtin_name = modname;
+
/* Handle all-NULL symbols allocated into .bss */
if (info->sechdrs[get_secindex(info, sym)].sh_type & SHT_NOBITS) {
zeros = calloc(1, sym->st_size);
@@ -1597,6 +1625,9 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
}
}
free(zeros);
+ mod->builtin_name = NULL;
+ if (modname_str)
+ free(modname_str);
}
/* Now add out buffered information to the generated C source */
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 1178f40a73f3..34fe5fc0b02c 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -128,6 +128,7 @@ struct module {
struct list_head missing_namespaces;
// Actual imported namespaces
struct list_head imported_namespaces;
+ const char *builtin_name;
char name[];
};
--
2.37.3This adds an unimplemented command line flag for writing the built-in
aliases to a file.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
scripts/mod/modpost.c | 23 +++++++++++++++++++++--
scripts/mod/modpost.h | 1 +
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 2c80da0220c3..e38d6b2ceea4 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2165,6 +2165,19 @@ static void write_if_changed(struct buffer *b, const char *fname)
write_buf(b, fname);
}
+/* Write the builtin aliases to the specified file. */
+static void write_builtin(const char *fname)
+{
+ struct buffer buf = { };
+ struct module *mod;
+
+ list_for_each_entry(mod, &modules, list)
+ buf_write(&buf, mod->modalias_buf.p, mod->modalias_buf.pos);
+
+ write_if_changed(&buf, fname);
+ free(buf.p);
+}
+
static void write_vmlinux_export_c_file(struct module *mod)
{
struct buffer buf = { };
@@ -2321,13 +2334,16 @@ int main(int argc, char **argv)
{
struct module *mod;
char *missing_namespace_deps = NULL;
- char *dump_write = NULL, *files_source = NULL;
+ char *builtin_write = NULL, *dump_write = NULL, *files_source = NULL;
int opt;
LIST_HEAD(dump_lists);
struct dump_list *dl, *dl2;
- while ((opt = getopt(argc, argv, "ei:mnT:o:awENd:")) != -1) {
+ while ((opt = getopt(argc, argv, "b:ei:mnT:o:awENd:")) != -1) {
switch (opt) {
+ case 'b':
+ builtin_write = optarg;
+ break;
case 'e':
external_module = true;
break;
@@ -2390,6 +2406,9 @@ int main(int argc, char **argv)
write_mod_c_file(mod);
}
+ if (builtin_write)
+ write_builtin(builtin_write);
+
if (missing_namespace_deps)
write_namespace_deps_files(missing_namespace_deps);
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 34fe5fc0b02c..c55a6aeb46bf 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -123,6 +123,7 @@ struct module {
bool has_init;
bool has_cleanup;
struct buffer dev_table_buf;
+ struct buffer modalias_buf;
char srcversion[25];
// Missing namespace dependencies
struct list_head missing_namespaces;
--
2.37.3On Mon, Dec 19, 2022 at 02:46:15PM -0600, Allen Webb wrote: > This adds an unimplemented command line flag for writing the built-in > aliases to a file. If it is unimplemented, why add it? And this needs a lot more description as to why you are changing this here, as well as documentation for the flag you added, right? thanks, greg k-h
On Tue, Dec 20, 2022 at 12:43 AM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Mon, Dec 19, 2022 at 02:46:15PM -0600, Allen Webb wrote: > > This adds an unimplemented command line flag for writing the built-in > > aliases to a file. > > If it is unimplemented, why add it? This is a fairly self contained change even though the code that populates mod->modalias_buf is in the next patch. I could flatten the two commits, but I was trying to keep this to a single logical change. There is somewhat of a chicken and egg problem here. If the other patch comes first it will be dead code, if this comes first it is pretty much dead code. > > And this needs a lot more description as to why you are changing this > here, as well as documentation for the flag you added, right? I might be missing something, but I don't see an obvious place where the other command line parameters for modpost are documented, so I will add a comment inside the case statement unless you have a better idea. > > thanks, > > greg k-h
This populates the mod->modalias_buf with aliases for built-in modules
when modpost is run against vmlinuz.o.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
scripts/mod/file2alias.c | 55 +++++++++++++++++++++++++++-------------
1 file changed, 38 insertions(+), 17 deletions(-)
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index e41ff8de7a87..e840cb51281a 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -232,6 +232,8 @@ static void do_usb_entry(void *symval,
add_wildcard(alias);
buf_printf(&mod->dev_table_buf,
"MODULE_ALIAS(\"%s\");\n", alias);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias %s %s\n", alias, mod->builtin_name);
}
/* Handles increment/decrement of BCD formatted integers */
@@ -376,9 +378,13 @@ static void do_of_entry_multi(void *symval, struct module *mod)
*tmp = '_';
buf_printf(&mod->dev_table_buf, "MODULE_ALIAS(\"%s\");\n", alias);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias %s %s\n", alias, mod->builtin_name);
strcat(alias, "C");
add_wildcard(alias);
buf_printf(&mod->dev_table_buf, "MODULE_ALIAS(\"%s\");\n", alias);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias %s %s\n", alias, mod->builtin_name);
}
static void do_of_table(void *symval, unsigned long size,
@@ -610,12 +616,18 @@ static void do_pnp_device_entry(void *symval, unsigned long size,
buf_printf(&mod->dev_table_buf,
"MODULE_ALIAS(\"pnp:d%s*\");\n", *id);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias pnp:d%s* %s\n",
+ *id, mod->builtin_name);
/* fix broken pnp bus lowercasing */
for (j = 0; j < sizeof(acpi_id); j++)
acpi_id[j] = toupper((*id)[j]);
buf_printf(&mod->dev_table_buf,
"MODULE_ALIAS(\"acpi*:%s:*\");\n", acpi_id);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias acpi*:%s:* %s\n",
+ acpi_id, mod->builtin_name);
}
}
@@ -637,6 +649,8 @@ static void do_pnp_card_entries(void *symval, unsigned long size,
const char *id = (char *)(*devs)[j].id;
int i2, j2;
int dup = 0;
+ char acpi_id[PNP_ID_LEN];
+ int k;
if (!id[0])
break;
@@ -662,19 +676,23 @@ static void do_pnp_card_entries(void *symval, unsigned long size,
}
/* add an individual alias for every device entry */
- if (!dup) {
- char acpi_id[PNP_ID_LEN];
- int k;
-
- buf_printf(&mod->dev_table_buf,
- "MODULE_ALIAS(\"pnp:d%s*\");\n", id);
-
- /* fix broken pnp bus lowercasing */
- for (k = 0; k < sizeof(acpi_id); k++)
- acpi_id[k] = toupper(id[k]);
- buf_printf(&mod->dev_table_buf,
- "MODULE_ALIAS(\"acpi*:%s:*\");\n", acpi_id);
- }
+ if (dup)
+ continue;
+
+ buf_printf(&mod->dev_table_buf,
+ "MODULE_ALIAS(\"pnp:d%s*\");\n", id);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias pnp:d%s* %s\n",
+ id, mod->builtin_name);
+
+ /* fix broken pnp bus lowercasing */
+ for (k = 0; k < sizeof(acpi_id); k++)
+ acpi_id[k] = toupper(id[k]);
+ buf_printf(&mod->dev_table_buf,
+ "MODULE_ALIAS(\"acpi*:%s:*\");\n", acpi_id);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias acpi*:%s:* %s\n",
+ acpi_id, mod->builtin_name);
}
}
}
@@ -1476,10 +1494,13 @@ static void do_table(void *symval, unsigned long size,
size -= id_size;
for (i = 0; i < size; i += id_size) {
- if (do_entry(mod->name, symval+i, alias)) {
- buf_printf(&mod->dev_table_buf,
- "MODULE_ALIAS(\"%s\");\n", alias);
- }
+ if (!do_entry(mod->name, symval + i, alias))
+ continue;
+ buf_printf(&mod->dev_table_buf, "MODULE_ALIAS(\"%s\");\n", alias);
+ if (!mod->builtin_name)
+ continue;
+ buf_printf(&mod->modalias_buf, "alias %s %s\n", alias,
+ mod->builtin_name);
}
}
--
2.37.3Generate modules.builtin.alias using modpost and install it with the
modules.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
.gitignore | 1 +
Makefile | 1 +
scripts/Makefile.modpost | 17 ++++++++++++++++-
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/.gitignore b/.gitignore
index 47229f98b327..40a90bca8964 100644
--- a/.gitignore
+++ b/.gitignore
@@ -67,6 +67,7 @@ modules.order
/System.map
/Module.markers
/modules.builtin
+/modules.builtin.alias
/modules.builtin.modinfo
/modules.nsdeps
diff --git a/Makefile b/Makefile
index 78525ebea876..572f364f4053 100644
--- a/Makefile
+++ b/Makefile
@@ -1558,6 +1558,7 @@ __modinst_pre:
fi
@sed 's:^:kernel/:' modules.order > $(MODLIB)/modules.order
@cp -f modules.builtin $(MODLIB)/
+ @cp -f modules.builtin.alias $(MODLIB)/
@cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/
endif # CONFIG_MODULES
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index e41dee64d429..94c1d66c7769 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -15,6 +15,7 @@
# 2) modpost is then used to
# 3) create one <module>.mod.c file per module
# 4) create one Module.symvers file with CRC for all exported symbols
+# 5) create modules.builtin.alias the aliases for built-in modules
# Step 3 is used to place certain information in the module's ELF
# section, including information such as:
@@ -51,6 +52,21 @@ ifneq ($(findstring i,$(filter-out --%,$(MAKEFLAGS))),)
modpost-args += -n
endif
+vmlinux.o-if-present := $(wildcard vmlinux.o)
+ifneq ($(vmlinux.o-if-present),)
+output-builtin.alias := modules.builtin.alias
+modpost-args += -b .modules.builtin.alias.in
+.modules.builtin.alias.in: $(output-symdump)
+ @# Building $(output-symdump) generates .modules.builtin.alias.in as a
+ @# side effect.
+ @[ -e $@ ] || $(MODPOST) -b .modules.builtin.alias.in $(vmlinux.o-if-present)
+
+$(output-builtin.alias): .modules.builtin.alias.in
+ sort -o $@ $^
+
+__modpost: $(output-builtin.alias)
+endif
+
ifeq ($(KBUILD_EXTMOD),)
# Generate the list of in-tree objects in vmlinux
@@ -78,7 +94,6 @@ targets += .vmlinux.objs
.vmlinux.objs: vmlinux.a $(KBUILD_VMLINUX_LIBS) FORCE
$(call if_changed,vmlinux_objs)
-vmlinux.o-if-present := $(wildcard vmlinux.o)
output-symdump := vmlinux.symvers
ifdef KBUILD_MODULES
--
2.37.3Update the documentation to include the presense and use case of
modules.builtin.alias.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
Documentation/kbuild/kbuild.rst | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst
index 08f575e6236c..1c7c02040a54 100644
--- a/Documentation/kbuild/kbuild.rst
+++ b/Documentation/kbuild/kbuild.rst
@@ -17,6 +17,12 @@ modules.builtin
This file lists all modules that are built into the kernel. This is used
by modprobe to not fail when trying to load something builtin.
+modules.builtin.alias
+---------------------
+This file lists all match-id based aliases for modules built into the kernel.
+These are intended to enable userspace to make authorization decisions based
+on which modules are likely to be bound to a device after it is authorized.
+
modules.builtin.modinfo
-----------------------
This file contains modinfo from all modules that are built into the kernel.
--
2.37.3Update the documentation to include the presense and use case of
modules.builtin.alias.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
Documentation/kbuild/kbuild.rst | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst
index 08f575e6236c..1c7c02040a54 100644
--- a/Documentation/kbuild/kbuild.rst
+++ b/Documentation/kbuild/kbuild.rst
@@ -17,6 +17,12 @@ modules.builtin
This file lists all modules that are built into the kernel. This is used
by modprobe to not fail when trying to load something builtin.
+modules.builtin.alias
+---------------------
+This file lists all match-id based aliases for modules built into the kernel.
+These are intended to enable userspace to make authorization decisions based
+on which modules are likely to be bound to a device after it is authorized.
+
modules.builtin.modinfo
-----------------------
This file contains modinfo from all modules that are built into the kernel.
--
2.37.3On Mon, Dec 19, 2022 at 02:46:18PM -0600, Allen Webb wrote: > Update the documentation to include the presense and use case of > modules.builtin.alias. > > Signed-off-by: Allen Webb <allenwebb@google.com> > --- > Documentation/kbuild/kbuild.rst | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst > index 08f575e6236c..1c7c02040a54 100644 > --- a/Documentation/kbuild/kbuild.rst > +++ b/Documentation/kbuild/kbuild.rst > @@ -17,6 +17,12 @@ modules.builtin > This file lists all modules that are built into the kernel. This is used > by modprobe to not fail when trying to load something builtin. > > +modules.builtin.alias > +--------------------- > +This file lists all match-id based aliases for modules built into the kernel. > +These are intended to enable userspace to make authorization decisions based > +on which modules are likely to be bound to a device after it is authorized. What is an example? This sounds obscure. Luis
On Mon, Dec 19, 2022 at 3:23 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Mon, Dec 19, 2022 at 02:46:18PM -0600, Allen Webb wrote: > > Update the documentation to include the presense and use case of > > modules.builtin.alias. > > > > Signed-off-by: Allen Webb <allenwebb@google.com> > > --- > > Documentation/kbuild/kbuild.rst | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst > > index 08f575e6236c..1c7c02040a54 100644 > > --- a/Documentation/kbuild/kbuild.rst > > +++ b/Documentation/kbuild/kbuild.rst > > @@ -17,6 +17,12 @@ modules.builtin > > This file lists all modules that are built into the kernel. This is used > > by modprobe to not fail when trying to load something builtin. > > > > +modules.builtin.alias > > +--------------------- > > +This file lists all match-id based aliases for modules built into the kernel. > > +These are intended to enable userspace to make authorization decisions based > > +on which modules are likely to be bound to a device after it is authorized. > > What is an example? This sounds obscure. Many of the devices that match the usb_storage driver only specify the vendor id, product id, and device id (VID:PID:D) and do not match against device class, interface class, etc. Here are some examples from modules.alias: A grep for wildcards in these fields yields 6136 matches: grep 'dc\*dsc\*dp\*ic\*isc\*ip\*in\*' /lib/modules/5.19.11-1rodete1-amd64/modules.alias | wc -l 6136 To write USBGuard policy that only authorizes devices that bind to a particular module the policy needs to be aware of all these VID:PID:D which can change between kernel versions. This is done at runtime rather than excluding modules from the build because some devices are not needed at or before login or when a device is locked. By not authorizing new devices that would bind to a set of modules, these modules become unreachable to an attacker who seeks to exploit kernel bugs in those modules. I could add this detail to the documentation file, but I was trying to keep the description to about the same length as the others around it. > > Luis
On Mon, Dec 19, 2022 at 03:40:42PM -0600, Allen Webb wrote: > On Mon, Dec 19, 2022 at 3:23 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > On Mon, Dec 19, 2022 at 02:46:18PM -0600, Allen Webb wrote: > > > Update the documentation to include the presense and use case of > > > modules.builtin.alias. > > > > > > Signed-off-by: Allen Webb <allenwebb@google.com> > > > --- > > > Documentation/kbuild/kbuild.rst | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst > > > index 08f575e6236c..1c7c02040a54 100644 > > > --- a/Documentation/kbuild/kbuild.rst > > > +++ b/Documentation/kbuild/kbuild.rst > > > @@ -17,6 +17,12 @@ modules.builtin > > > This file lists all modules that are built into the kernel. This is used > > > by modprobe to not fail when trying to load something builtin. > > > > > > +modules.builtin.alias > > > +--------------------- > > > +This file lists all match-id based aliases for modules built into the kernel. > > > +These are intended to enable userspace to make authorization decisions based > > > +on which modules are likely to be bound to a device after it is authorized. > > > > What is an example? This sounds obscure. > > Many of the devices that match the usb_storage driver only specify the > vendor id, product id, and device id (VID:PID:D) and do not match > against device class, interface class, etc. Here are some examples > from modules.alias: A grep for wildcards in these fields yields 6136 > matches: > grep 'dc\*dsc\*dp\*ic\*isc\*ip\*in\*' > /lib/modules/5.19.11-1rodete1-amd64/modules.alias | wc -l > 6136 > > To write USBGuard policy that only authorizes devices that bind to a > particular module the policy needs to be aware of all these VID:PID:D > which can change between kernel versions. > > This is done at runtime rather than excluding modules from the build > because some devices are not needed at or before login or when a > device is locked. By not authorizing new devices that would bind to a > set of modules, these modules become unreachable to an attacker who > seeks to exploit kernel bugs in those modules. > > I could add this detail to the documentation file, but I was trying to > keep the description to about the same length as the others around it. How about the second sentence you wrote say something like: An example usage of the built-in aliases is to enable software such as USBGuard to enable / disable specific devices outside of just the vendor, product and device ID. This allows more flexible security policies in userspace. Luis
On Mon, Dec 19, 2022 at 4:07 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Mon, Dec 19, 2022 at 03:40:42PM -0600, Allen Webb wrote: > > On Mon, Dec 19, 2022 at 3:23 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > > > On Mon, Dec 19, 2022 at 02:46:18PM -0600, Allen Webb wrote: > > > > Update the documentation to include the presense and use case of > > > > modules.builtin.alias. > > > > > > > > Signed-off-by: Allen Webb <allenwebb@google.com> > > > > --- > > > > Documentation/kbuild/kbuild.rst | 6 ++++++ > > > > 1 file changed, 6 insertions(+) > > > > > > > > diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst > > > > index 08f575e6236c..1c7c02040a54 100644 > > > > --- a/Documentation/kbuild/kbuild.rst > > > > +++ b/Documentation/kbuild/kbuild.rst > > > > @@ -17,6 +17,12 @@ modules.builtin > > > > This file lists all modules that are built into the kernel. This is used > > > > by modprobe to not fail when trying to load something builtin. > > > > > > > > +modules.builtin.alias > > > > +--------------------- > > > > +This file lists all match-id based aliases for modules built into the kernel. > > > > +These are intended to enable userspace to make authorization decisions based > > > > +on which modules are likely to be bound to a device after it is authorized. > > > > > > What is an example? This sounds obscure. > > > > Many of the devices that match the usb_storage driver only specify the > > vendor id, product id, and device id (VID:PID:D) and do not match > > against device class, interface class, etc. Here are some examples > > from modules.alias: A grep for wildcards in these fields yields 6136 > > matches: > > grep 'dc\*dsc\*dp\*ic\*isc\*ip\*in\*' > > /lib/modules/5.19.11-1rodete1-amd64/modules.alias | wc -l > > 6136 > > > > To write USBGuard policy that only authorizes devices that bind to a > > particular module the policy needs to be aware of all these VID:PID:D > > which can change between kernel versions. > > > > This is done at runtime rather than excluding modules from the build > > because some devices are not needed at or before login or when a > > device is locked. By not authorizing new devices that would bind to a > > set of modules, these modules become unreachable to an attacker who > > seeks to exploit kernel bugs in those modules. > > > > I could add this detail to the documentation file, but I was trying to > > keep the description to about the same length as the others around it. > > How about the second sentence you wrote say something like: > > An example usage of the built-in aliases is to enable software such as > USBGuard to enable / disable specific devices outside of just the > vendor, product and device ID. This allows more flexible security policies > in userspace. I tweaked it a tiny bit, but that makes the whole description: This file lists all match-id based aliases for modules built into the kernel. An example usage of the built-in aliases is to enable software such as USBGuard to allow or block devices outside of just the vendor, product, and device ID. This enables more flexible security policies in userspace. > > Luis
On Mon, Dec 19, 2022 at 04:20:41PM -0600, Allen Webb wrote: > On Mon, Dec 19, 2022 at 4:07 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > On Mon, Dec 19, 2022 at 03:40:42PM -0600, Allen Webb wrote: > > > On Mon, Dec 19, 2022 at 3:23 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > > > > > > > On Mon, Dec 19, 2022 at 02:46:18PM -0600, Allen Webb wrote: > > > > > Update the documentation to include the presense and use case of > > > > > modules.builtin.alias. > > > > > > > > > > Signed-off-by: Allen Webb <allenwebb@google.com> > > > > > --- > > > > > Documentation/kbuild/kbuild.rst | 6 ++++++ > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst > > > > > index 08f575e6236c..1c7c02040a54 100644 > > > > > --- a/Documentation/kbuild/kbuild.rst > > > > > +++ b/Documentation/kbuild/kbuild.rst > > > > > @@ -17,6 +17,12 @@ modules.builtin > > > > > This file lists all modules that are built into the kernel. This is used > > > > > by modprobe to not fail when trying to load something builtin. > > > > > > > > > > +modules.builtin.alias > > > > > +--------------------- > > > > > +This file lists all match-id based aliases for modules built into the kernel. > > > > > +These are intended to enable userspace to make authorization decisions based > > > > > +on which modules are likely to be bound to a device after it is authorized. > > > > > > > > What is an example? This sounds obscure. > > > > > > Many of the devices that match the usb_storage driver only specify the > > > vendor id, product id, and device id (VID:PID:D) and do not match > > > against device class, interface class, etc. Here are some examples > > > from modules.alias: A grep for wildcards in these fields yields 6136 > > > matches: > > > grep 'dc\*dsc\*dp\*ic\*isc\*ip\*in\*' > > > /lib/modules/5.19.11-1rodete1-amd64/modules.alias | wc -l > > > 6136 > > > > > > To write USBGuard policy that only authorizes devices that bind to a > > > particular module the policy needs to be aware of all these VID:PID:D > > > which can change between kernel versions. > > > > > > This is done at runtime rather than excluding modules from the build > > > because some devices are not needed at or before login or when a > > > device is locked. By not authorizing new devices that would bind to a > > > set of modules, these modules become unreachable to an attacker who > > > seeks to exploit kernel bugs in those modules. > > > > > > I could add this detail to the documentation file, but I was trying to > > > keep the description to about the same length as the others around it. > > > > How about the second sentence you wrote say something like: > > > > An example usage of the built-in aliases is to enable software such as > > USBGuard to enable / disable specific devices outside of just the > > vendor, product and device ID. This allows more flexible security policies > > in userspace. > > I tweaked it a tiny bit, but that makes the whole description: > > This file lists all match-id based aliases for modules built into the kernel. > An example usage of the built-in aliases is to enable software such as > USBGuard to allow or block devices outside of just the vendor, product, and > device ID. This enables more flexible security policies in userspace. Now, without ever readng your patchset and intentions I can easily grasp what your goals are. Looks good. Feel free to add my Reviewed-by tags for this patch. Luis
Please disregard this patch. I updated the commit message and this was hanging around in my outgoing directory afterward. On Mon, Dec 19, 2022 at 2:46 PM Allen Webb <allenwebb@google.com> wrote: > > Update the documentation to include the presense and use case of > modules.builtin.alias. > > Signed-off-by: Allen Webb <allenwebb@google.com> > --- > Documentation/kbuild/kbuild.rst | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst > index 08f575e6236c..1c7c02040a54 100644 > --- a/Documentation/kbuild/kbuild.rst > +++ b/Documentation/kbuild/kbuild.rst > @@ -17,6 +17,12 @@ modules.builtin > This file lists all modules that are built into the kernel. This is used > by modprobe to not fail when trying to load something builtin. > > +modules.builtin.alias > +--------------------- > +This file lists all match-id based aliases for modules built into the kernel. > +These are intended to enable userspace to make authorization decisions based > +on which modules are likely to be bound to a device after it is authorized. > + > modules.builtin.modinfo > ----------------------- > This file contains modinfo from all modules that are built into the kernel. > -- > 2.37.3 >
A one character difference in the name supplied to MODULE_DEVICE_TABLE
breaks a future patch set, so fix the typo.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Allen Webb <allenwebb@google.com>
---
drivers/soc/imx/imx8mp-blk-ctrl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
index 0e3b6ba22f943..344a0a71df14a 100644
--- a/drivers/soc/imx/imx8mp-blk-ctrl.c
+++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
@@ -743,7 +743,7 @@ static const struct of_device_id imx8mp_blk_ctrl_of_match[] = {
/* Sentinel */
}
};
-MODULE_DEVICE_TABLE(of, imx8m_blk_ctrl_of_match);
+MODULE_DEVICE_TABLE(of, imx8mp_blk_ctrl_of_match);
static struct platform_driver imx8mp_blk_ctrl_driver = {
.probe = imx8mp_blk_ctrl_probe,
--
2.37.3On Mon, Dec 19, 2022 at 01:18:47PM -0600, Allen Webb wrote:
> A one character difference in the name supplied to MODULE_DEVICE_TABLE
> breaks a future patch set, so fix the typo.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Allen Webb <allenwebb@google.com>
> ---
> drivers/soc/imx/imx8mp-blk-ctrl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
> index 0e3b6ba22f943..344a0a71df14a 100644
> --- a/drivers/soc/imx/imx8mp-blk-ctrl.c
> +++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
> @@ -743,7 +743,7 @@ static const struct of_device_id imx8mp_blk_ctrl_of_match[] = {
> /* Sentinel */
> }
> };
> -MODULE_DEVICE_TABLE(of, imx8m_blk_ctrl_of_match);
> +MODULE_DEVICE_TABLE(of, imx8mp_blk_ctrl_of_match);
What commit id does this fix? Shouldn't this be also cc: stable to
resolve this issue for older kernels as obviousl the module device table
for auto-loading is not correct?
thanks,
greg k-h
On Mon, Dec 19, 2022 at 1:22 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Dec 19, 2022 at 01:18:47PM -0600, Allen Webb wrote:
> > A one character difference in the name supplied to MODULE_DEVICE_TABLE
> > breaks a future patch set, so fix the typo.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Allen Webb <allenwebb@google.com>
> > ---
> > drivers/soc/imx/imx8mp-blk-ctrl.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
> > index 0e3b6ba22f943..344a0a71df14a 100644
> > --- a/drivers/soc/imx/imx8mp-blk-ctrl.c
> > +++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
> > @@ -743,7 +743,7 @@ static const struct of_device_id imx8mp_blk_ctrl_of_match[] = {
> > /* Sentinel */
> > }
> > };
> > -MODULE_DEVICE_TABLE(of, imx8m_blk_ctrl_of_match);
> > +MODULE_DEVICE_TABLE(of, imx8mp_blk_ctrl_of_match);
>
> What commit id does this fix? Shouldn't this be also cc: stable to
> resolve this issue for older kernels as obviousl the module device table
> for auto-loading is not correct?
I have included Cc stable and Fixes: for the three patches that were
obvious typos and will upload a follow-up series shortly. It is
unlikely these drivers were being built as modules because the build
would have been broken for that configuration.
This seems to be the most recent case so it is the most likely to make
a difference, but I would imagine SOC drivers might not be loadable in
practice if they are needed to bootstrap the system to a point that
loadable modules can be accessed.
>
> thanks,
>
> greg k-h
A one character difference in the name supplied to MODULE_DEVICE_TABLE
breaks a future patch set, so fix the typo.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Allen Webb <allenwebb@google.com>
---
drivers/mailbox/rockchip-mailbox.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mailbox/rockchip-mailbox.c b/drivers/mailbox/rockchip-mailbox.c
index 979acc810f307..ca50f7f176f6a 100644
--- a/drivers/mailbox/rockchip-mailbox.c
+++ b/drivers/mailbox/rockchip-mailbox.c
@@ -159,7 +159,7 @@ static const struct of_device_id rockchip_mbox_of_match[] = {
{ .compatible = "rockchip,rk3368-mailbox", .data = &rk3368_drv_data},
{ },
};
-MODULE_DEVICE_TABLE(of, rockchp_mbox_of_match);
+MODULE_DEVICE_TABLE(of, rockchip_mbox_of_match);
static int rockchip_mbox_probe(struct platform_device *pdev)
{
--
2.37.3A future patch makes use of the device table for built-in modules, so
do not ifdef out the match id table.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Allen Webb <allenwebb@google.com>
---
drivers/scsi/BusLogic.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index f2abffce26599..0c60867c9e7c0 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -3715,7 +3715,6 @@ static void __exit blogic_exit(void)
__setup("BusLogic=", blogic_setup);
-#ifdef MODULE
/*static struct pci_device_id blogic_pci_tbl[] = {
{ PCI_VENDOR_ID_BUSLOGIC, PCI_DEVICE_ID_BUSLOGIC_MULTIMASTER,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
@@ -3731,7 +3730,6 @@ static const struct pci_device_id blogic_pci_tbl[] = {
{PCI_DEVICE(PCI_VENDOR_ID_BUSLOGIC, PCI_DEVICE_ID_BUSLOGIC_FLASHPOINT)},
{0, },
};
-#endif
MODULE_DEVICE_TABLE(pci, blogic_pci_tbl);
module_init(blogic_init);
--
2.37.3A small difference in the name supplied to MODULE_DEVICE_TABLE
breaks a future patch set, so fix the typo.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Allen Webb <allenwebb@google.com>
---
drivers/mfd/stmpe-spi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mfd/stmpe-spi.c b/drivers/mfd/stmpe-spi.c
index ad8055a0e2869..6791a53689777 100644
--- a/drivers/mfd/stmpe-spi.c
+++ b/drivers/mfd/stmpe-spi.c
@@ -129,7 +129,7 @@ static const struct spi_device_id stmpe_spi_id[] = {
{ "stmpe2403", STMPE2403 },
{ }
};
-MODULE_DEVICE_TABLE(spi, stmpe_id);
+MODULE_DEVICE_TABLE(spi, stmpe_spi_id);
static struct spi_driver stmpe_spi_driver = {
.driver = {
--
2.37.3Implement MODULE_DEVICE_TABLE for build-in modules to make it possible
to generate a builtin.alias file to complement modules.alias.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
include/linux/module.h | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index ec61fb53979a9..3d1b04ca63505 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -243,7 +243,20 @@ extern void cleanup_module(void);
extern typeof(name) __mod_##type##__##name##_device_table \
__attribute__ ((unused, alias(__stringify(name))))
#else /* !MODULE */
-#define MODULE_DEVICE_TABLE(type, name)
+/*
+ * The names may not be unique for built-in modules, so include the module name
+ * to guarantee uniqueness.
+ *
+ * Note that extern is needed because modpost reads these symbols to generate
+ * modalias entries for each match id in each device table. They are not used
+ * at runtime.
+ */
+#define MODULE_DEVICE_TABLE(type, name) \
+extern void *CONCATENATE( \
+ CONCATENATE(__mod_##type##__##name##__, \
+ __KBUILD_MODNAME), \
+ _device_table) \
+ __attribute__ ((unused, alias(__stringify(name))))
#endif
/* Version of form [<epoch>:]<version>[-<extra-version>].
--
2.37.3Keep track of the module name when processing match table symbols.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
scripts/mod/file2alias.c | 39 +++++++++++++++++++++++++++++++++++----
scripts/mod/modpost.h | 1 +
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 80d973144fded..e41ff8de7a876 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -28,6 +28,7 @@ typedef Elf64_Addr kernel_ulong_t;
#include <stdint.h>
#endif
+#include <assert.h>
#include <ctype.h>
#include <stdbool.h>
@@ -1540,9 +1541,9 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
Elf_Sym *sym, const char *symname)
{
void *symval;
- char *zeros = NULL;
- const char *name, *identifier;
- unsigned int namelen;
+ char *zeros = NULL, *modname_str = NULL;
+ const char *name, *identifier, *modname;
+ unsigned int namelen, modnamelen;
/* We're looking for a section relative symbol */
if (!sym->st_shndx || get_secindex(info, sym) >= info->num_sections)
@@ -1552,7 +1553,12 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
return;
- /* All our symbols are of form __mod_<name>__<identifier>_device_table. */
+ /*
+ * All our symbols are either of form
+ * __mod_<name>__<identifier>_device_table
+ * or
+ * __mod_<name>__<identifier>__kmod_<builtin-name>_device_table
+ */
if (strncmp(symname, "__mod_", strlen("__mod_")))
return;
name = symname + strlen("__mod_");
@@ -1564,8 +1570,30 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
identifier = strstr(name, "__");
if (!identifier)
return;
+ modnamelen = namelen;
namelen = identifier - name;
+ /*
+ * In the vmlinuz.o case we want to handle __kmod_ so aliases from
+ * builtin modules are attributed correctly.
+ */
+ modname = strstr(identifier + 2, "__kmod_");
+ if (modname) {
+ modname += strlen("__kmod_");
+ modnamelen -= (modname - name) + strlen("_device_table");
+ modname_str = malloc(modnamelen + 1);
+ /* We don't want to continue if the allocation fails. */
+ assert(modname_str);
+ memcpy(modname_str, modname, modnamelen);
+ modname_str[modnamelen] = '\0';
+ }
+
+ if (modname_str)
+ modname = modname_str;
+ else
+ modname = mod->name;
+ mod->builtin_name = modname;
+
/* Handle all-NULL symbols allocated into .bss */
if (info->sechdrs[get_secindex(info, sym)].sh_type & SHT_NOBITS) {
zeros = calloc(1, sym->st_size);
@@ -1597,6 +1625,9 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
}
}
free(zeros);
+ mod->builtin_name = NULL;
+ if (modname_str)
+ free(modname_str);
}
/* Now add out buffered information to the generated C source */
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 1178f40a73f3d..34fe5fc0b02cb 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -128,6 +128,7 @@ struct module {
struct list_head missing_namespaces;
// Actual imported namespaces
struct list_head imported_namespaces;
+ const char *builtin_name;
char name[];
};
--
2.37.3This adds an unimplemented command line flag for writing the built-in
aliases to a file.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
scripts/mod/modpost.c | 23 +++++++++++++++++++++--
scripts/mod/modpost.h | 1 +
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 2c80da0220c32..e38d6b2ceea40 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2165,6 +2165,19 @@ static void write_if_changed(struct buffer *b, const char *fname)
write_buf(b, fname);
}
+/* Write the builtin aliases to the specified file. */
+static void write_builtin(const char *fname)
+{
+ struct buffer buf = { };
+ struct module *mod;
+
+ list_for_each_entry(mod, &modules, list)
+ buf_write(&buf, mod->modalias_buf.p, mod->modalias_buf.pos);
+
+ write_if_changed(&buf, fname);
+ free(buf.p);
+}
+
static void write_vmlinux_export_c_file(struct module *mod)
{
struct buffer buf = { };
@@ -2321,13 +2334,16 @@ int main(int argc, char **argv)
{
struct module *mod;
char *missing_namespace_deps = NULL;
- char *dump_write = NULL, *files_source = NULL;
+ char *builtin_write = NULL, *dump_write = NULL, *files_source = NULL;
int opt;
LIST_HEAD(dump_lists);
struct dump_list *dl, *dl2;
- while ((opt = getopt(argc, argv, "ei:mnT:o:awENd:")) != -1) {
+ while ((opt = getopt(argc, argv, "b:ei:mnT:o:awENd:")) != -1) {
switch (opt) {
+ case 'b':
+ builtin_write = optarg;
+ break;
case 'e':
external_module = true;
break;
@@ -2390,6 +2406,9 @@ int main(int argc, char **argv)
write_mod_c_file(mod);
}
+ if (builtin_write)
+ write_builtin(builtin_write);
+
if (missing_namespace_deps)
write_namespace_deps_files(missing_namespace_deps);
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 34fe5fc0b02cb..c55a6aeb46bfd 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -123,6 +123,7 @@ struct module {
bool has_init;
bool has_cleanup;
struct buffer dev_table_buf;
+ struct buffer modalias_buf;
char srcversion[25];
// Missing namespace dependencies
struct list_head missing_namespaces;
--
2.37.3This populates the mod->modalias_buf with aliases for built-in modules
when modpost is run against vmlinuz.o.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
scripts/mod/file2alias.c | 55 +++++++++++++++++++++++++++-------------
1 file changed, 38 insertions(+), 17 deletions(-)
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index e41ff8de7a876..e840cb51281a4 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -232,6 +232,8 @@ static void do_usb_entry(void *symval,
add_wildcard(alias);
buf_printf(&mod->dev_table_buf,
"MODULE_ALIAS(\"%s\");\n", alias);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias %s %s\n", alias, mod->builtin_name);
}
/* Handles increment/decrement of BCD formatted integers */
@@ -376,9 +378,13 @@ static void do_of_entry_multi(void *symval, struct module *mod)
*tmp = '_';
buf_printf(&mod->dev_table_buf, "MODULE_ALIAS(\"%s\");\n", alias);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias %s %s\n", alias, mod->builtin_name);
strcat(alias, "C");
add_wildcard(alias);
buf_printf(&mod->dev_table_buf, "MODULE_ALIAS(\"%s\");\n", alias);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias %s %s\n", alias, mod->builtin_name);
}
static void do_of_table(void *symval, unsigned long size,
@@ -610,12 +616,18 @@ static void do_pnp_device_entry(void *symval, unsigned long size,
buf_printf(&mod->dev_table_buf,
"MODULE_ALIAS(\"pnp:d%s*\");\n", *id);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias pnp:d%s* %s\n",
+ *id, mod->builtin_name);
/* fix broken pnp bus lowercasing */
for (j = 0; j < sizeof(acpi_id); j++)
acpi_id[j] = toupper((*id)[j]);
buf_printf(&mod->dev_table_buf,
"MODULE_ALIAS(\"acpi*:%s:*\");\n", acpi_id);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias acpi*:%s:* %s\n",
+ acpi_id, mod->builtin_name);
}
}
@@ -637,6 +649,8 @@ static void do_pnp_card_entries(void *symval, unsigned long size,
const char *id = (char *)(*devs)[j].id;
int i2, j2;
int dup = 0;
+ char acpi_id[PNP_ID_LEN];
+ int k;
if (!id[0])
break;
@@ -662,19 +676,23 @@ static void do_pnp_card_entries(void *symval, unsigned long size,
}
/* add an individual alias for every device entry */
- if (!dup) {
- char acpi_id[PNP_ID_LEN];
- int k;
-
- buf_printf(&mod->dev_table_buf,
- "MODULE_ALIAS(\"pnp:d%s*\");\n", id);
-
- /* fix broken pnp bus lowercasing */
- for (k = 0; k < sizeof(acpi_id); k++)
- acpi_id[k] = toupper(id[k]);
- buf_printf(&mod->dev_table_buf,
- "MODULE_ALIAS(\"acpi*:%s:*\");\n", acpi_id);
- }
+ if (dup)
+ continue;
+
+ buf_printf(&mod->dev_table_buf,
+ "MODULE_ALIAS(\"pnp:d%s*\");\n", id);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias pnp:d%s* %s\n",
+ id, mod->builtin_name);
+
+ /* fix broken pnp bus lowercasing */
+ for (k = 0; k < sizeof(acpi_id); k++)
+ acpi_id[k] = toupper(id[k]);
+ buf_printf(&mod->dev_table_buf,
+ "MODULE_ALIAS(\"acpi*:%s:*\");\n", acpi_id);
+ if (mod->builtin_name)
+ buf_printf(&mod->modalias_buf, "alias acpi*:%s:* %s\n",
+ acpi_id, mod->builtin_name);
}
}
}
@@ -1476,10 +1494,13 @@ static void do_table(void *symval, unsigned long size,
size -= id_size;
for (i = 0; i < size; i += id_size) {
- if (do_entry(mod->name, symval+i, alias)) {
- buf_printf(&mod->dev_table_buf,
- "MODULE_ALIAS(\"%s\");\n", alias);
- }
+ if (!do_entry(mod->name, symval + i, alias))
+ continue;
+ buf_printf(&mod->dev_table_buf, "MODULE_ALIAS(\"%s\");\n", alias);
+ if (!mod->builtin_name)
+ continue;
+ buf_printf(&mod->modalias_buf, "alias %s %s\n", alias,
+ mod->builtin_name);
}
}
--
2.37.3Generate modules.builtin.alias using modpost and install it with the
modules.
Signed-off-by: Allen Webb <allenwebb@google.com>
---
.gitignore | 1 +
Makefile | 1 +
scripts/Makefile.modpost | 17 ++++++++++++++++-
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/.gitignore b/.gitignore
index 47229f98b327b..40a90bca89641 100644
--- a/.gitignore
+++ b/.gitignore
@@ -67,6 +67,7 @@ modules.order
/System.map
/Module.markers
/modules.builtin
+/modules.builtin.alias
/modules.builtin.modinfo
/modules.nsdeps
diff --git a/Makefile b/Makefile
index 78525ebea8762..572f364f40538 100644
--- a/Makefile
+++ b/Makefile
@@ -1558,6 +1558,7 @@ __modinst_pre:
fi
@sed 's:^:kernel/:' modules.order > $(MODLIB)/modules.order
@cp -f modules.builtin $(MODLIB)/
+ @cp -f modules.builtin.alias $(MODLIB)/
@cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/
endif # CONFIG_MODULES
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index e41dee64d429c..94c1d66c7769a 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -15,6 +15,7 @@
# 2) modpost is then used to
# 3) create one <module>.mod.c file per module
# 4) create one Module.symvers file with CRC for all exported symbols
+# 5) create modules.builtin.alias the aliases for built-in modules
# Step 3 is used to place certain information in the module's ELF
# section, including information such as:
@@ -51,6 +52,21 @@ ifneq ($(findstring i,$(filter-out --%,$(MAKEFLAGS))),)
modpost-args += -n
endif
+vmlinux.o-if-present := $(wildcard vmlinux.o)
+ifneq ($(vmlinux.o-if-present),)
+output-builtin.alias := modules.builtin.alias
+modpost-args += -b .modules.builtin.alias.in
+.modules.builtin.alias.in: $(output-symdump)
+ @# Building $(output-symdump) generates .modules.builtin.alias.in as a
+ @# side effect.
+ @[ -e $@ ] || $(MODPOST) -b .modules.builtin.alias.in $(vmlinux.o-if-present)
+
+$(output-builtin.alias): .modules.builtin.alias.in
+ sort -o $@ $^
+
+__modpost: $(output-builtin.alias)
+endif
+
ifeq ($(KBUILD_EXTMOD),)
# Generate the list of in-tree objects in vmlinux
@@ -78,7 +94,6 @@ targets += .vmlinux.objs
.vmlinux.objs: vmlinux.a $(KBUILD_VMLINUX_LIBS) FORCE
$(call if_changed,vmlinux_objs)
-vmlinux.o-if-present := $(wildcard vmlinux.o)
output-symdump := vmlinux.symvers
ifdef KBUILD_MODULES
--
2.37.3© 2016 - 2025 Red Hat, Inc.