.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(-)
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
© 2016 - 2025 Red Hat, Inc.