USB devices support the authorized attribute which can be used by
user-space to implement trust-based systems for enabling USB devices. It
would be helpful when building these systems to be able to know in
advance which kernel drivers (or modules) are reachable from a
particular USB device.
This information is readily available for external modules in
modules.alias. However, builtin kernel modules are not covered. This
patch adds a sys-fs attribute to both builtin and loaded modules
exposing the matching rules in the modalias format for integration
with tools like USBGuard.
Change-Id: I83b6f0c30e06e65cbe223f1606187283fcb13215
Signed-off-by: Allen Webb <[email protected]>
---
drivers/base/Makefile | 2 +-
drivers/base/base.h | 8 ++
drivers/base/bus.c | 42 ++++++
drivers/base/mod_devicetable.c | 249 +++++++++++++++++++++++++++++++++
drivers/usb/core/driver.c | 2 +
include/linux/device/bus.h | 8 ++
include/linux/module.h | 1 +
kernel/module/internal.h | 2 +
kernel/module/sysfs.c | 88 ++++++++++++
kernel/params.c | 2 +
10 files changed, 403 insertions(+), 1 deletion(-)
create mode 100644 drivers/base/mod_devicetable.c
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 83217d243c25b..924d46ae987f4 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -15,7 +15,7 @@ obj-y += firmware_loader/
obj-$(CONFIG_NUMA) += node.o
obj-$(CONFIG_MEMORY_HOTPLUG) += memory.o
ifeq ($(CONFIG_SYSFS),y)
-obj-$(CONFIG_MODULES) += module.o
+obj-$(CONFIG_MODULES) += mod_devicetable.o module.o
endif
obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o
obj-$(CONFIG_REGMAP) += regmap/
diff --git a/drivers/base/base.h b/drivers/base/base.h
index b902d1ecc247f..beaa252c04388 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -173,6 +173,14 @@ static inline void module_add_driver(struct module *mod,
static inline void module_remove_driver(struct device_driver *drv) { }
#endif
+#if defined(CONFIG_SYSFS)
+ssize_t usb_drv_to_modalias(struct device_driver *drv, char *buf,
+ size_t count);
+#else
+static inline ssize_t usb_drv_to_modalias(struct device_driver *drv, char *buf,
+ size_t count) { return -EINVAL; }
+#endif
+
#ifdef CONFIG_DEVTMPFS
extern int devtmpfs_init(void);
#else
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 7ca47e5b3c1f4..4e0c5925545e5 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -178,6 +178,48 @@ static const struct kset_uevent_ops bus_uevent_ops = {
static struct kset *bus_kset;
+/**
+ * bus_for_each - bus iterator.
+ * @start: bus to start iterating from.
+ * @data: data for the callback.
+ * @fn: function to be called for each device.
+ *
+ * Iterate over list of buses, and call @fn for each,
+ * passing it @data. If @start is not NULL, we use that bus to
+ * begin iterating from.
+ *
+ * We check the return of @fn each time. If it returns anything
+ * other than 0, we break out and return that value.
+ *
+ * NOTE: The bus that returns a non-zero value is not retained
+ * in any way, nor is its refcount incremented. If the caller needs
+ * to retain this data, it should do so, and increment the reference
+ * count in the supplied callback.
+ */
+int bus_for_each(void *data, int (*fn)(struct bus_type *, void *))
+{
+ int error = 0;
+ struct bus_type *bus;
+ struct subsys_private *bus_prv;
+ struct kset *subsys;
+ struct kobject *k;
+
+ spin_lock(&bus_kset->list_lock);
+
+ list_for_each_entry(k, &bus_kset->list, entry) {
+ subsys = container_of(k, struct kset, kobj);
+ bus_prv = container_of(subsys, struct subsys_private, subsys);
+ bus = bus_prv->bus;
+ error = fn(bus, data);
+ if (error)
+ break;
+ }
+
+ spin_unlock(&bus_kset->list_lock);
+ return error;
+}
+EXPORT_SYMBOL_GPL(bus_for_each);
+
/* Manually detach a device from its associated driver. */
static ssize_t unbind_store(struct device_driver *drv, const char *buf,
size_t count)
diff --git a/drivers/base/mod_devicetable.c b/drivers/base/mod_devicetable.c
new file mode 100644
index 0000000000000..f1d3de9f111c4
--- /dev/null
+++ b/drivers/base/mod_devicetable.c
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mod_devicetable.c - helpers for displaying modaliases through sysfs.
+ *
+ * This borrows a lot from file2alias.c
+ */
+
+#include <linux/device/bus.h>
+#include <linux/device.h>
+#include <linux/usb.h>
+
+#include "base.h"
+#include "../usb/core/usb.h"
+
+#define ADD(buf, count, len, sep, cond, field) \
+do { \
+ if (cond) \
+ (len) += scnprintf(&(buf)[len], \
+ (count) - (len), \
+ sizeof(field) == 1 ? (sep "%02X") : \
+ sizeof(field) == 2 ? (sep "%04X") : \
+ sizeof(field) == 4 ? (sep "%08X") : "", \
+ (field)); \
+ else \
+ (len) += scnprintf(&(buf)[len], (count) - (len), (sep "*")); \
+} while (0)
+
+#ifdef CONFIG_USB
+/* USB related modaliases can be split because of device number matching, so
+ * this function handles individual modaliases for one segment of the range.
+ *
+ *
+ */
+static ssize_t usb_id_to_modalias(const struct usb_device_id *id,
+ unsigned int bcdDevice_initial,
+ int bcdDevice_initial_digits,
+ unsigned char range_lo,
+ unsigned char range_hi,
+ unsigned char max, const char *mod_name,
+ char *buf, size_t count)
+{
+ ssize_t len = 0;
+
+ ADD(buf, count, len, "alias usb:v",
+ id->match_flags & USB_DEVICE_ID_MATCH_VENDOR, id->idVendor);
+ ADD(buf, count, len, "p", id->match_flags & USB_DEVICE_ID_MATCH_PRODUCT,
+ id->idProduct);
+
+ len += scnprintf(&buf[len], count - len, "d");
+ if (bcdDevice_initial_digits)
+ len += scnprintf(&buf[len], count - len, "%0*X",
+ bcdDevice_initial_digits, bcdDevice_initial);
+ if (range_lo == range_hi) {
+ len += scnprintf(&buf[len], count - len, "%X", range_lo);
+ } else if (range_lo > 0 || range_hi < max) {
+ if (range_lo > 0x9 || range_hi < 0xA) {
+ len += scnprintf(&buf[len], count - len, "[%X-%X]",
+ range_lo, range_hi);
+ } else {
+ len += scnprintf(&buf[len], count - len,
+ range_lo < 0x9 ? "[%X-9" : "[%X",
+ range_lo);
+ len += scnprintf(&buf[len], count - len,
+ range_hi > 0xA ? "A-%X]" : "%X]",
+ range_hi);
+ }
+ }
+ if (bcdDevice_initial_digits < (sizeof(id->bcdDevice_lo) * 2 - 1))
+ len += scnprintf(&buf[len], count - len, "*");
+
+ ADD(buf, count, len, "dc",
+ id->match_flags & USB_DEVICE_ID_MATCH_DEV_CLASS, id->bDeviceClass);
+ ADD(buf, count, len, "dsc",
+ id->match_flags & USB_DEVICE_ID_MATCH_DEV_SUBCLASS,
+ id->bDeviceSubClass);
+ ADD(buf, count, len, "dp",
+ id->match_flags & USB_DEVICE_ID_MATCH_DEV_PROTOCOL,
+ id->bDeviceProtocol);
+ ADD(buf, count, len, "ic",
+ id->match_flags & USB_DEVICE_ID_MATCH_INT_CLASS,
+ id->bInterfaceClass);
+ ADD(buf, count, len, "isc",
+ id->match_flags & USB_DEVICE_ID_MATCH_INT_SUBCLASS,
+ id->bInterfaceSubClass);
+ ADD(buf, count, len, "ip",
+ id->match_flags & USB_DEVICE_ID_MATCH_INT_PROTOCOL,
+ id->bInterfaceProtocol);
+ ADD(buf, count, len, "in",
+ id->match_flags & USB_DEVICE_ID_MATCH_INT_NUMBER,
+ id->bInterfaceNumber);
+
+ len += scnprintf(&buf[len], count - len, " %s\n", mod_name);
+ return len;
+}
+
+/* Handles increment/decrement of BCD formatted integers */
+/* Returns the previous value, so it works like i++ or i-- */
+static unsigned int incbcd(unsigned int *bcd,
+ int inc,
+ unsigned char max,
+ size_t chars)
+{
+ unsigned int init = *bcd, i, j;
+ unsigned long long c, dec = 0, div;
+
+ /* If bcd is not in BCD format, just increment */
+ if (max > 0x9) {
+ *bcd += inc;
+ return init;
+ }
+
+ /* Convert BCD to Decimal */
+ for (i = 0 ; i < chars ; i++) {
+ c = (*bcd >> (i << 2)) & 0xf;
+ c = c > 9 ? 9 : c; /* force to bcd just in case */
+ for (j = 0 ; j < i ; j++)
+ c = c * 10;
+ dec += c;
+ }
+
+ /* Do our increment/decrement */
+ dec += inc;
+ *bcd = 0;
+
+ /* Convert back to BCD */
+ for (i = 0 ; i < chars ; i++) {
+ for (c = 1, j = 0 ; j < i ; j++)
+ c = c * 10;
+ div = dec;
+ (void)do_div(div, c); /* div = div / c */
+ c = do_div(div, 10); /* c = div % 10; div = div / 10 */
+ *bcd += c << (i << 2);
+ }
+ return init;
+}
+
+/* Print the modaliases for the specified struct usb_device_id.
+ */
+static ssize_t usb_id_to_modalias_multi(const struct usb_device_id *id,
+ const char *mod_name, char *buf,
+ size_t count)
+{
+ ssize_t len = 0;
+ unsigned int devlo, devhi;
+ unsigned char chi, clo, max;
+ int ndigits;
+
+ devlo = id->match_flags & USB_DEVICE_ID_MATCH_DEV_LO ?
+ id->bcdDevice_lo : 0x0U;
+ devhi = id->match_flags & USB_DEVICE_ID_MATCH_DEV_HI ?
+ id->bcdDevice_hi : ~0x0U;
+
+ /* Figure out if this entry is in bcd or hex format */
+ max = 0x9; /* Default to decimal format */
+ for (ndigits = 0 ; ndigits < sizeof(id->bcdDevice_lo) * 2 ; ndigits++) {
+ clo = (devlo >> (ndigits << 2)) & 0xf;
+ chi = ((devhi > 0x9999 ? 0x9999 : devhi) >>
+ (ndigits << 2)) & 0xf;
+ if (clo > max || chi > max) {
+ max = 0xf;
+ break;
+ }
+ }
+
+ /*
+ * Some modules (visor) have empty slots as placeholder for
+ * run-time specification that results in catch-all alias
+ */
+ if (!(id->idVendor || id->idProduct || id->bDeviceClass ||
+ id->bInterfaceClass))
+ return len;
+
+ /* Convert numeric bcdDevice range into fnmatch-able pattern(s) */
+ for (ndigits = sizeof(id->bcdDevice_lo) * 2 - 1; devlo <= devhi;
+ ndigits--) {
+ clo = devlo & 0xf;
+ chi = devhi & 0xf;
+ /* If we are in bcd mode, truncate if necessary */
+ if (chi > max)
+ chi = max;
+ devlo >>= 4;
+ devhi >>= 4;
+
+ if (devlo == devhi || !ndigits) {
+ len += usb_id_to_modalias(id, devlo, ndigits, clo, chi,
+ max, mod_name, buf + len,
+ count - len);
+ break;
+ }
+
+ if (clo > 0x0)
+ len += usb_id_to_modalias(id,
+ incbcd(&devlo, 1, max,
+ sizeof(id->bcdDevice_lo) * 2),
+ ndigits, clo, max, max, mod_name, buf + len,
+ count - len);
+
+ if (chi < max)
+ len += usb_id_to_modalias(id,
+ incbcd(&devhi, -1, max,
+ sizeof(id->bcdDevice_lo) * 2),
+ ndigits, 0x0, chi, max, mod_name, buf + len,
+ count - len);
+ }
+ return len;
+}
+
+/* Print the modaliases for the given driver assumed to be an usb_driver or
+ * usb_device_driver.
+ *
+ * "alias" is prepended and the module name is appended to each modalias to
+ * match the format in modules.aliases.
+ *
+ * The modaliases will be written out to @buf with @count being the maximum
+ * bytes to write. The return value is a negative errno on error or the number
+ * of bytes written to @buf on success.
+ */
+ssize_t usb_drv_to_modalias(struct device_driver *drv, char *buf,
+ size_t count)
+{
+ ssize_t len = 0;
+ const struct usb_device_id *id;
+ const char *mod_name;
+
+ if (drv->bus != &usb_bus_type)
+ return -EINVAL;
+
+ if (drv->owner)
+ mod_name = drv->owner->name;
+ else
+ mod_name = drv->mod_name;
+
+ if (is_usb_device_driver(drv))
+ id = to_usb_device_driver(drv)->id_table;
+ else
+ id = to_usb_driver(drv)->id_table;
+ if (!id)
+ return len;
+
+ for (; id->match_flags; id++) {
+ len += usb_id_to_modalias_multi(id, mod_name, buf + len,
+ count - len);
+ }
+ return len;
+}
+#else
+inline ssize_t usb_drv_to_modalias(struct device_driver *drv, char *buf,
+ size_t count){ return 0; }
+#endif
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 7e7e119c253fb..fdbc197b64c9c 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -32,6 +32,7 @@
#include <linux/usb/quirks.h>
#include <linux/usb/hcd.h>
+#include "../../base/base.h"
#include "usb.h"
@@ -2030,4 +2031,5 @@ struct bus_type usb_bus_type = {
.match = usb_device_match,
.uevent = usb_uevent,
.need_parent_lock = true,
+ .drv_to_modalias = usb_drv_to_modalias,
};
diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
index d8b29ccd07e56..cce0bedec63d9 100644
--- a/include/linux/device/bus.h
+++ b/include/linux/device/bus.h
@@ -61,6 +61,10 @@ struct fwnode_handle;
* this bus.
* @dma_cleanup: Called to cleanup DMA configuration on a device on
* this bus.
+ * @drv_to_modalias: Called to convert the matching IDs in a
+ * struct device_driver to their corresponding modaliases.
+ * Note that the struct device_driver is expected to belong
+ * to this bus.
* @pm: Power management operations of this bus, callback the specific
* device driver's pm-ops.
* @iommu_ops: IOMMU specific operations for this bus, used to attach IOMMU
@@ -107,6 +111,9 @@ struct bus_type {
int (*dma_configure)(struct device *dev);
void (*dma_cleanup)(struct device *dev);
+ ssize_t (*drv_to_modalias)(struct device_driver *drv, char *buf,
+ size_t count);
+
const struct dev_pm_ops *pm;
const struct iommu_ops *iommu_ops;
@@ -161,6 +168,7 @@ void subsys_dev_iter_init(struct subsys_dev_iter *iter,
struct device *subsys_dev_iter_next(struct subsys_dev_iter *iter);
void subsys_dev_iter_exit(struct subsys_dev_iter *iter);
+int bus_for_each(void *data, int (*fn)(struct bus_type *, void *));
int bus_for_each_dev(struct bus_type *bus, struct device *start, void *data,
int (*fn)(struct device *dev, void *data));
struct device *bus_find_device(struct bus_type *bus, struct device *start,
diff --git a/include/linux/module.h b/include/linux/module.h
index ec61fb53979a9..0bfa859a21566 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -47,6 +47,7 @@ struct module_kobject {
struct kobject *drivers_dir;
struct module_param_attrs *mp;
struct completion *kobj_completion;
+ struct bin_attribute modalias_attr;
} __randomize_layout;
struct module_attribute {
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 2e2bf236f5582..8d7ae37584868 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -259,11 +259,13 @@ static inline void add_kallsyms(struct module *mod, const struct load_info *info
#endif /* CONFIG_KALLSYMS */
#ifdef CONFIG_SYSFS
+void add_modalias_attr(struct module_kobject *mk);
int mod_sysfs_setup(struct module *mod, const struct load_info *info,
struct kernel_param *kparam, unsigned int num_params);
void mod_sysfs_teardown(struct module *mod);
void init_param_lock(struct module *mod);
#else /* !CONFIG_SYSFS */
+static inline void add_modalias_attr(struct module_kobject *mk) {}
static inline int mod_sysfs_setup(struct module *mod,
const struct load_info *info,
struct kernel_param *kparam,
diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c
index ce68f821dcd12..651c677c4ab96 100644
--- a/kernel/module/sysfs.c
+++ b/kernel/module/sysfs.c
@@ -5,6 +5,8 @@
* Copyright (C) 2008 Rusty Russell
*/
+#include <linux/device/bus.h>
+#include <linux/device/driver.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/fs.h>
@@ -240,6 +242,90 @@ static inline void add_notes_attrs(struct module *mod, const struct load_info *i
static inline void remove_notes_attrs(struct module *mod) { }
#endif /* CONFIG_KALLSYMS */
+/* Track of the buffer and module identity in callbacks when walking the list of
+ * drivers for each bus.
+ */
+struct modalias_bus_print_state {
+ struct module_kobject *mk;
+ char *buf;
+ size_t count;
+ ssize_t len;
+};
+
+static int print_modalias_for_drv(struct device_driver *drv, void *p)
+{
+ struct modalias_bus_print_state *s = p;
+ struct module_kobject *mk = s->mk;
+ ssize_t len;
+ /* Skip drivers that do not match this module. */
+ if (mk->mod) {
+ if (mk->mod != drv->owner)
+ return 0;
+ } else if (!mk->kobj.name || !drv->mod_name ||
+ strcmp(mk->kobj.name, drv->mod_name))
+ return 0;
+
+ if (drv->bus && drv->bus->drv_to_modalias) {
+ len = drv->bus->drv_to_modalias(drv, s->buf + s->len,
+ s->count - s->len);
+ if (len < 0)
+ return len;
+ s->len += len;
+ }
+ return 0;
+}
+
+static int print_modalias_for_bus(struct bus_type *type, void *p)
+{
+ return bus_for_each_drv(type, NULL, p, print_modalias_for_drv);
+}
+
+static ssize_t module_modalias_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t pos, size_t count)
+{
+ struct module_kobject *mk = container_of(kobj, struct module_kobject,
+ kobj);
+ struct modalias_bus_print_state state = {mk, buf, count, 0};
+ int error = 0;
+
+ if (pos != 0)
+ return -EINVAL;
+
+ error = bus_for_each(&state, print_modalias_for_bus);
+ if (error)
+ return error;
+
+ /*
+ * The caller checked the pos and count against our size.
+ */
+ return state.len;
+}
+
+/* Used in kernel/params.c for builtin modules.
+ *
+ * `struct module_kobject` is used instead of `struct module` because for
+ * builtin modules, the `struct module` is not available when this is called.
+ */
+void add_modalias_attr(struct module_kobject *mk)
+{
+ sysfs_bin_attr_init(&mk->modalias_attr);
+ mk->modalias_attr.attr.name = "modalias";
+ mk->modalias_attr.attr.mode = 0444;
+ mk->modalias_attr.read = module_modalias_read;
+ if (sysfs_create_bin_file(&mk->kobj, &mk->modalias_attr)) {
+ /* We shouldn't ignore the return type, but there is nothing to
+ * do.
+ */
+ return;
+ }
+}
+
+static void remove_modalias_attr(struct module_kobject *mk)
+{
+ sysfs_remove_bin_file(&mk->kobj, &mk->modalias_attr);
+}
+
static void del_usage_links(struct module *mod)
{
#ifdef CONFIG_MODULE_UNLOAD
@@ -398,6 +484,7 @@ int mod_sysfs_setup(struct module *mod,
add_sect_attrs(mod, info);
add_notes_attrs(mod, info);
+ add_modalias_attr(&mod->mkobj);
return 0;
@@ -415,6 +502,7 @@ int mod_sysfs_setup(struct module *mod,
static void mod_sysfs_fini(struct module *mod)
{
+ remove_modalias_attr(&mod->mkobj);
remove_notes_attrs(mod);
remove_sect_attrs(mod);
mod_kobject_put(mod);
diff --git a/kernel/params.c b/kernel/params.c
index 5b92310425c50..111024196361a 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -13,6 +13,7 @@
#include <linux/slab.h>
#include <linux/ctype.h>
#include <linux/security.h>
+#include "module/internal.h"
#ifdef CONFIG_SYSFS
/* Protects all built-in parameters, modules use their own param_lock */
@@ -815,6 +816,7 @@ static void __init kernel_add_sysfs_param(const char *name,
BUG_ON(err);
kobject_uevent(&mk->kobj, KOBJ_ADD);
kobject_put(&mk->kobj);
+ add_modalias_attr(mk);
}
/*
--
2.37.3
Removes some debug prints and fixes 64-bit unsigned divide issue
| Reported-by: kernel test robot <[email protected]>
On Tue, Nov 29, 2022 at 04:43:13PM -0600, Allen Webb wrote:
> USB devices support the authorized attribute which can be used by
> user-space to implement trust-based systems for enabling USB devices. It
> would be helpful when building these systems to be able to know in
> advance which kernel drivers (or modules) are reachable from a
> particular USB device.
>
> This information is readily available for external modules in
> modules.alias. However, builtin kernel modules are not covered. This
> patch adds a sys-fs attribute to both builtin and loaded modules
> exposing the matching rules in the modalias format for integration
> with tools like USBGuard.
>
> Change-Id: I83b6f0c30e06e65cbe223f1606187283fcb13215
> Signed-off-by: Allen Webb <[email protected]>
> ---
> drivers/base/Makefile | 2 +-
> drivers/base/base.h | 8 ++
> drivers/base/bus.c | 42 ++++++
> drivers/base/mod_devicetable.c | 249 +++++++++++++++++++++++++++++++++
> drivers/usb/core/driver.c | 2 +
> include/linux/device/bus.h | 8 ++
> include/linux/module.h | 1 +
> kernel/module/internal.h | 2 +
> kernel/module/sysfs.c | 88 ++++++++++++
> kernel/params.c | 2 +
> 10 files changed, 403 insertions(+), 1 deletion(-)
> create mode 100644 drivers/base/mod_devicetable.c
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- Your patch contains warnings and/or errors noticed by the
scripts/checkpatch.pl tool.
- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/SubmittingPatches for what needs to be done
here to properly describe this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
USB devices support the authorized attribute which can be used by
user-space to implement trust-based systems for enabling USB devices. It
would be helpful when building these systems to be able to know in
advance which kernel drivers (or modules) are reachable from a
particular USB device.
This information is readily available for external modules in
modules.alias. However, builtin kernel modules are not covered. This
patch adds a sys-fs attribute to both builtin and loaded modules
exposing the matching rules in the modalias format for integration
with tools like USBGuard.
Signed-off-by: Allen Webb <[email protected]>
---
drivers/base/Makefile | 2 +-
drivers/base/base.h | 8 +
drivers/base/bus.c | 42 ++++++
drivers/base/mod_devicetable.c | 257 +++++++++++++++++++++++++++++++++
drivers/usb/core/driver.c | 2 +
include/linux/device/bus.h | 8 +
include/linux/module.h | 1 +
kernel/module/internal.h | 2 +
kernel/module/sysfs.c | 88 +++++++++++
kernel/params.c | 7 +
10 files changed, 416 insertions(+), 1 deletion(-)
create mode 100644 drivers/base/mod_devicetable.c
Fixed another kernel config incompability identified by:
| Reported-by: kernel test robot <[email protected]>
Fixed a couple CHECK messages from checkpatch.pl.
There are still some CHECK messages that are by design in a macro that
depends on modifying a parameter and string literal concatenation.
There is also a maintainer file update warning for a new file, but that
file is already covered by an existing maintainers entry.
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 83217d243c25b..924d46ae987f4 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -15,7 +15,7 @@ obj-y += firmware_loader/
obj-$(CONFIG_NUMA) += node.o
obj-$(CONFIG_MEMORY_HOTPLUG) += memory.o
ifeq ($(CONFIG_SYSFS),y)
-obj-$(CONFIG_MODULES) += module.o
+obj-$(CONFIG_MODULES) += mod_devicetable.o module.o
endif
obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o
obj-$(CONFIG_REGMAP) += regmap/
diff --git a/drivers/base/base.h b/drivers/base/base.h
index b902d1ecc247f..beaa252c04388 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -173,6 +173,14 @@ static inline void module_add_driver(struct module *mod,
static inline void module_remove_driver(struct device_driver *drv) { }
#endif
+#if defined(CONFIG_SYSFS)
+ssize_t usb_drv_to_modalias(struct device_driver *drv, char *buf,
+ size_t count);
+#else
+static inline ssize_t usb_drv_to_modalias(struct device_driver *drv, char *buf,
+ size_t count) { return -EINVAL; }
+#endif
+
#ifdef CONFIG_DEVTMPFS
extern int devtmpfs_init(void);
#else
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index 7ca47e5b3c1f4..4e0c5925545e5 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -178,6 +178,48 @@ static const struct kset_uevent_ops bus_uevent_ops = {
static struct kset *bus_kset;
+/**
+ * bus_for_each - bus iterator.
+ * @start: bus to start iterating from.
+ * @data: data for the callback.
+ * @fn: function to be called for each device.
+ *
+ * Iterate over list of buses, and call @fn for each,
+ * passing it @data. If @start is not NULL, we use that bus to
+ * begin iterating from.
+ *
+ * We check the return of @fn each time. If it returns anything
+ * other than 0, we break out and return that value.
+ *
+ * NOTE: The bus that returns a non-zero value is not retained
+ * in any way, nor is its refcount incremented. If the caller needs
+ * to retain this data, it should do so, and increment the reference
+ * count in the supplied callback.
+ */
+int bus_for_each(void *data, int (*fn)(struct bus_type *, void *))
+{
+ int error = 0;
+ struct bus_type *bus;
+ struct subsys_private *bus_prv;
+ struct kset *subsys;
+ struct kobject *k;
+
+ spin_lock(&bus_kset->list_lock);
+
+ list_for_each_entry(k, &bus_kset->list, entry) {
+ subsys = container_of(k, struct kset, kobj);
+ bus_prv = container_of(subsys, struct subsys_private, subsys);
+ bus = bus_prv->bus;
+ error = fn(bus, data);
+ if (error)
+ break;
+ }
+
+ spin_unlock(&bus_kset->list_lock);
+ return error;
+}
+EXPORT_SYMBOL_GPL(bus_for_each);
+
/* Manually detach a device from its associated driver. */
static ssize_t unbind_store(struct device_driver *drv, const char *buf,
size_t count)
diff --git a/drivers/base/mod_devicetable.c b/drivers/base/mod_devicetable.c
new file mode 100644
index 0000000000000..d7f198aad430f
--- /dev/null
+++ b/drivers/base/mod_devicetable.c
@@ -0,0 +1,257 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mod_devicetable.c - helpers for displaying modaliases through sysfs.
+ *
+ * This borrows a lot from file2alias.c
+ */
+
+#include <linux/device/bus.h>
+#include <linux/device.h>
+#include <linux/usb.h>
+
+#include "base.h"
+#include "../usb/core/usb.h"
+
+/* Helper macro to add a modalias field to the string buffer associated with
+ * a match id.
+ *
+ * Note that:
+ * + len should be a ssize_t and is modified in the macro
+ * + sep should be a string literal and is concatenated as part of a format
+ * string
+ * + field is the struct field of the match id
+ */
+#define ADD(buf, count, len, sep, cond, field) \
+do { \
+ char *buf_ = buf; \
+ size_t count_ = count; \
+ if (cond) \
+ (len) += scnprintf(&buf_[len], \
+ count_ - (len), \
+ sizeof(field) == 1 ? (sep "%02X") : \
+ sizeof(field) == 2 ? (sep "%04X") : \
+ sizeof(field) == 4 ? (sep "%08X") : "", \
+ (field)); \
+ else \
+ (len) += scnprintf(&buf_[len], count_ - (len), (sep "*")); \
+} while (0)
+
+#ifdef CONFIG_USB
+/* USB related modaliases can be split because of device number matching, so
+ * this function handles individual modaliases for one segment of the range.
+ */
+static ssize_t usb_id_to_modalias(const struct usb_device_id *id,
+ unsigned int bcdDevice_initial,
+ int bcdDevice_initial_digits,
+ unsigned char range_lo,
+ unsigned char range_hi,
+ unsigned char max, const char *mod_name,
+ char *buf, size_t count)
+{
+ ssize_t len = 0;
+
+ ADD(buf, count, len, "alias usb:v",
+ id->match_flags & USB_DEVICE_ID_MATCH_VENDOR, id->idVendor);
+ ADD(buf, count, len, "p", id->match_flags & USB_DEVICE_ID_MATCH_PRODUCT,
+ id->idProduct);
+
+ len += scnprintf(&buf[len], count - len, "d");
+ if (bcdDevice_initial_digits)
+ len += scnprintf(&buf[len], count - len, "%0*X",
+ bcdDevice_initial_digits, bcdDevice_initial);
+ if (range_lo == range_hi) {
+ len += scnprintf(&buf[len], count - len, "%X", range_lo);
+ } else if (range_lo > 0 || range_hi < max) {
+ if (range_lo > 0x9 || range_hi < 0xA) {
+ len += scnprintf(&buf[len], count - len, "[%X-%X]",
+ range_lo, range_hi);
+ } else {
+ len += scnprintf(&buf[len], count - len,
+ range_lo < 0x9 ? "[%X-9" : "[%X",
+ range_lo);
+ len += scnprintf(&buf[len], count - len,
+ range_hi > 0xA ? "A-%X]" : "%X]",
+ range_hi);
+ }
+ }
+ if (bcdDevice_initial_digits < (sizeof(id->bcdDevice_lo) * 2 - 1))
+ len += scnprintf(&buf[len], count - len, "*");
+
+ ADD(buf, count, len, "dc",
+ id->match_flags & USB_DEVICE_ID_MATCH_DEV_CLASS, id->bDeviceClass);
+ ADD(buf, count, len, "dsc",
+ id->match_flags & USB_DEVICE_ID_MATCH_DEV_SUBCLASS,
+ id->bDeviceSubClass);
+ ADD(buf, count, len, "dp",
+ id->match_flags & USB_DEVICE_ID_MATCH_DEV_PROTOCOL,
+ id->bDeviceProtocol);
+ ADD(buf, count, len, "ic",
+ id->match_flags & USB_DEVICE_ID_MATCH_INT_CLASS,
+ id->bInterfaceClass);
+ ADD(buf, count, len, "isc",
+ id->match_flags & USB_DEVICE_ID_MATCH_INT_SUBCLASS,
+ id->bInterfaceSubClass);
+ ADD(buf, count, len, "ip",
+ id->match_flags & USB_DEVICE_ID_MATCH_INT_PROTOCOL,
+ id->bInterfaceProtocol);
+ ADD(buf, count, len, "in",
+ id->match_flags & USB_DEVICE_ID_MATCH_INT_NUMBER,
+ id->bInterfaceNumber);
+
+ len += scnprintf(&buf[len], count - len, " %s\n", mod_name);
+ return len;
+}
+
+/* Handles increment/decrement of BCD formatted integers */
+/* Returns the previous value, so it works like i++ or i-- */
+static unsigned int incbcd(unsigned int *bcd,
+ int inc,
+ unsigned char max,
+ size_t chars)
+{
+ unsigned int init = *bcd, i, j;
+ unsigned long long c, dec = 0, div;
+
+ /* If bcd is not in BCD format, just increment */
+ if (max > 0x9) {
+ *bcd += inc;
+ return init;
+ }
+
+ /* Convert BCD to Decimal */
+ for (i = 0 ; i < chars ; i++) {
+ c = (*bcd >> (i << 2)) & 0xf;
+ c = c > 9 ? 9 : c; /* force to bcd just in case */
+ for (j = 0 ; j < i ; j++)
+ c = c * 10;
+ dec += c;
+ }
+
+ /* Do our increment/decrement */
+ dec += inc;
+ *bcd = 0;
+
+ /* Convert back to BCD */
+ for (i = 0 ; i < chars ; i++) {
+ for (c = 1, j = 0 ; j < i ; j++)
+ c = c * 10;
+ div = dec;
+ (void)do_div(div, c); /* div = div / c */
+ c = do_div(div, 10); /* c = div % 10; div = div / 10 */
+ *bcd += c << (i << 2);
+ }
+ return init;
+}
+
+/* Print the modaliases for the specified struct usb_device_id. */
+static ssize_t usb_id_to_modalias_multi(const struct usb_device_id *id,
+ const char *mod_name, char *buf,
+ size_t count)
+{
+ ssize_t len = 0;
+ unsigned int devlo, devhi;
+ unsigned char chi, clo, max;
+ int ndigits;
+
+ devlo = id->match_flags & USB_DEVICE_ID_MATCH_DEV_LO ?
+ id->bcdDevice_lo : 0x0U;
+ devhi = id->match_flags & USB_DEVICE_ID_MATCH_DEV_HI ?
+ id->bcdDevice_hi : ~0x0U;
+
+ /* Figure out if this entry is in bcd or hex format */
+ max = 0x9; /* Default to decimal format */
+ for (ndigits = 0 ; ndigits < sizeof(id->bcdDevice_lo) * 2 ; ndigits++) {
+ clo = (devlo >> (ndigits << 2)) & 0xf;
+ chi = ((devhi > 0x9999 ? 0x9999 : devhi) >>
+ (ndigits << 2)) & 0xf;
+ if (clo > max || chi > max) {
+ max = 0xf;
+ break;
+ }
+ }
+
+ /*
+ * Some modules (visor) have empty slots as placeholder for
+ * run-time specification that results in catch-all alias
+ */
+ if (!(id->idVendor || id->idProduct || id->bDeviceClass ||
+ id->bInterfaceClass))
+ return len;
+
+ /* Convert numeric bcdDevice range into fnmatch-able pattern(s) */
+ for (ndigits = sizeof(id->bcdDevice_lo) * 2 - 1; devlo <= devhi;
+ ndigits--) {
+ clo = devlo & 0xf;
+ chi = devhi & 0xf;
+ /* If we are in bcd mode, truncate if necessary */
+ if (chi > max)
+ chi = max;
+ devlo >>= 4;
+ devhi >>= 4;
+
+ if (devlo == devhi || !ndigits) {
+ len += usb_id_to_modalias(id, devlo, ndigits, clo, chi,
+ max, mod_name, buf + len,
+ count - len);
+ break;
+ }
+
+ if (clo > 0x0)
+ len += usb_id_to_modalias(id,
+ incbcd(&devlo, 1, max,
+ sizeof(id->bcdDevice_lo) * 2),
+ ndigits, clo, max, max, mod_name, buf + len,
+ count - len);
+
+ if (chi < max)
+ len += usb_id_to_modalias(id,
+ incbcd(&devhi, -1, max,
+ sizeof(id->bcdDevice_lo) * 2),
+ ndigits, 0x0, chi, max, mod_name, buf + len,
+ count - len);
+ }
+ return len;
+}
+
+/* Print the modaliases for the given driver assumed to be an usb_driver or
+ * usb_device_driver.
+ *
+ * "alias" is prepended and the module name is appended to each modalias to
+ * match the format in modules.aliases.
+ *
+ * The modaliases will be written out to @buf with @count being the maximum
+ * bytes to write. The return value is a negative errno on error or the number
+ * of bytes written to @buf on success.
+ */
+ssize_t usb_drv_to_modalias(struct device_driver *drv, char *buf,
+ size_t count)
+{
+ ssize_t len = 0;
+ const struct usb_device_id *id;
+ const char *mod_name;
+
+ if (drv->bus != &usb_bus_type)
+ return -EINVAL;
+
+ if (drv->owner)
+ mod_name = drv->owner->name;
+ else
+ mod_name = drv->mod_name;
+
+ if (is_usb_device_driver(drv))
+ id = to_usb_device_driver(drv)->id_table;
+ else
+ id = to_usb_driver(drv)->id_table;
+ if (!id)
+ return len;
+
+ for (; id->match_flags; id++) {
+ len += usb_id_to_modalias_multi(id, mod_name, buf + len,
+ count - len);
+ }
+ return len;
+}
+#else
+inline ssize_t usb_drv_to_modalias(struct device_driver *drv, char *buf,
+ size_t count){ return 0; }
+#endif
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 7e7e119c253fb..fdbc197b64c9c 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -32,6 +32,7 @@
#include <linux/usb/quirks.h>
#include <linux/usb/hcd.h>
+#include "../../base/base.h"
#include "usb.h"
@@ -2030,4 +2031,5 @@ struct bus_type usb_bus_type = {
.match = usb_device_match,
.uevent = usb_uevent,
.need_parent_lock = true,
+ .drv_to_modalias = usb_drv_to_modalias,
};
diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
index d8b29ccd07e56..cce0bedec63d9 100644
--- a/include/linux/device/bus.h
+++ b/include/linux/device/bus.h
@@ -61,6 +61,10 @@ struct fwnode_handle;
* this bus.
* @dma_cleanup: Called to cleanup DMA configuration on a device on
* this bus.
+ * @drv_to_modalias: Called to convert the matching IDs in a
+ * struct device_driver to their corresponding modaliases.
+ * Note that the struct device_driver is expected to belong
+ * to this bus.
* @pm: Power management operations of this bus, callback the specific
* device driver's pm-ops.
* @iommu_ops: IOMMU specific operations for this bus, used to attach IOMMU
@@ -107,6 +111,9 @@ struct bus_type {
int (*dma_configure)(struct device *dev);
void (*dma_cleanup)(struct device *dev);
+ ssize_t (*drv_to_modalias)(struct device_driver *drv, char *buf,
+ size_t count);
+
const struct dev_pm_ops *pm;
const struct iommu_ops *iommu_ops;
@@ -161,6 +168,7 @@ void subsys_dev_iter_init(struct subsys_dev_iter *iter,
struct device *subsys_dev_iter_next(struct subsys_dev_iter *iter);
void subsys_dev_iter_exit(struct subsys_dev_iter *iter);
+int bus_for_each(void *data, int (*fn)(struct bus_type *, void *));
int bus_for_each_dev(struct bus_type *bus, struct device *start, void *data,
int (*fn)(struct device *dev, void *data));
struct device *bus_find_device(struct bus_type *bus, struct device *start,
diff --git a/include/linux/module.h b/include/linux/module.h
index ec61fb53979a9..0bfa859a21566 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -47,6 +47,7 @@ struct module_kobject {
struct kobject *drivers_dir;
struct module_param_attrs *mp;
struct completion *kobj_completion;
+ struct bin_attribute modalias_attr;
} __randomize_layout;
struct module_attribute {
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 2e2bf236f5582..8d7ae37584868 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -259,11 +259,13 @@ static inline void add_kallsyms(struct module *mod, const struct load_info *info
#endif /* CONFIG_KALLSYMS */
#ifdef CONFIG_SYSFS
+void add_modalias_attr(struct module_kobject *mk);
int mod_sysfs_setup(struct module *mod, const struct load_info *info,
struct kernel_param *kparam, unsigned int num_params);
void mod_sysfs_teardown(struct module *mod);
void init_param_lock(struct module *mod);
#else /* !CONFIG_SYSFS */
+static inline void add_modalias_attr(struct module_kobject *mk) {}
static inline int mod_sysfs_setup(struct module *mod,
const struct load_info *info,
struct kernel_param *kparam,
diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c
index ce68f821dcd12..651c677c4ab96 100644
--- a/kernel/module/sysfs.c
+++ b/kernel/module/sysfs.c
@@ -5,6 +5,8 @@
* Copyright (C) 2008 Rusty Russell
*/
+#include <linux/device/bus.h>
+#include <linux/device/driver.h>
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/fs.h>
@@ -240,6 +242,90 @@ static inline void add_notes_attrs(struct module *mod, const struct load_info *i
static inline void remove_notes_attrs(struct module *mod) { }
#endif /* CONFIG_KALLSYMS */
+/* Track of the buffer and module identity in callbacks when walking the list of
+ * drivers for each bus.
+ */
+struct modalias_bus_print_state {
+ struct module_kobject *mk;
+ char *buf;
+ size_t count;
+ ssize_t len;
+};
+
+static int print_modalias_for_drv(struct device_driver *drv, void *p)
+{
+ struct modalias_bus_print_state *s = p;
+ struct module_kobject *mk = s->mk;
+ ssize_t len;
+ /* Skip drivers that do not match this module. */
+ if (mk->mod) {
+ if (mk->mod != drv->owner)
+ return 0;
+ } else if (!mk->kobj.name || !drv->mod_name ||
+ strcmp(mk->kobj.name, drv->mod_name))
+ return 0;
+
+ if (drv->bus && drv->bus->drv_to_modalias) {
+ len = drv->bus->drv_to_modalias(drv, s->buf + s->len,
+ s->count - s->len);
+ if (len < 0)
+ return len;
+ s->len += len;
+ }
+ return 0;
+}
+
+static int print_modalias_for_bus(struct bus_type *type, void *p)
+{
+ return bus_for_each_drv(type, NULL, p, print_modalias_for_drv);
+}
+
+static ssize_t module_modalias_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t pos, size_t count)
+{
+ struct module_kobject *mk = container_of(kobj, struct module_kobject,
+ kobj);
+ struct modalias_bus_print_state state = {mk, buf, count, 0};
+ int error = 0;
+
+ if (pos != 0)
+ return -EINVAL;
+
+ error = bus_for_each(&state, print_modalias_for_bus);
+ if (error)
+ return error;
+
+ /*
+ * The caller checked the pos and count against our size.
+ */
+ return state.len;
+}
+
+/* Used in kernel/params.c for builtin modules.
+ *
+ * `struct module_kobject` is used instead of `struct module` because for
+ * builtin modules, the `struct module` is not available when this is called.
+ */
+void add_modalias_attr(struct module_kobject *mk)
+{
+ sysfs_bin_attr_init(&mk->modalias_attr);
+ mk->modalias_attr.attr.name = "modalias";
+ mk->modalias_attr.attr.mode = 0444;
+ mk->modalias_attr.read = module_modalias_read;
+ if (sysfs_create_bin_file(&mk->kobj, &mk->modalias_attr)) {
+ /* We shouldn't ignore the return type, but there is nothing to
+ * do.
+ */
+ return;
+ }
+}
+
+static void remove_modalias_attr(struct module_kobject *mk)
+{
+ sysfs_remove_bin_file(&mk->kobj, &mk->modalias_attr);
+}
+
static void del_usage_links(struct module *mod)
{
#ifdef CONFIG_MODULE_UNLOAD
@@ -398,6 +484,7 @@ int mod_sysfs_setup(struct module *mod,
add_sect_attrs(mod, info);
add_notes_attrs(mod, info);
+ add_modalias_attr(&mod->mkobj);
return 0;
@@ -415,6 +502,7 @@ int mod_sysfs_setup(struct module *mod,
static void mod_sysfs_fini(struct module *mod)
{
+ remove_modalias_attr(&mod->mkobj);
remove_notes_attrs(mod);
remove_sect_attrs(mod);
mod_kobject_put(mod);
diff --git a/kernel/params.c b/kernel/params.c
index 5b92310425c50..b7fd5313a3118 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -14,6 +14,12 @@
#include <linux/ctype.h>
#include <linux/security.h>
+#ifdef CONFIG_MODULES
+#include "module/internal.h"
+#else
+static inline void add_modalias_attr(struct module_kobject *mk) {}
+#endif /* !CONFIG_MODULES */
+
#ifdef CONFIG_SYSFS
/* Protects all built-in parameters, modules use their own param_lock */
static DEFINE_MUTEX(param_lock);
@@ -815,6 +821,7 @@ static void __init kernel_add_sysfs_param(const char *name,
BUG_ON(err);
kobject_uevent(&mk->kobj, KOBJ_ADD);
kobject_put(&mk->kobj);
+ add_modalias_attr(mk);
}
/*
--
2.37.3
Hi Allen,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on driver-core/driver-core-testing]
[also build test ERROR on driver-core/driver-core-next driver-core/driver-core-linus mcgrof/modules-next usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.1-rc7 next-20221130]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Allen-Webb/modules-add-modalias-file-to-sysfs-for-modules/20221201-061550
patch link: https://lore.kernel.org/r/20221130221447.1202206-1-allenwebb%40google.com
patch subject: [PATCH v4] modules: add modalias file to sysfs for modules.
config: riscv-randconfig-r042-20221128
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 6e4cea55f0d1104408b26ac574566a0e4de48036)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/8a0305181ba6d360a1aaba1384e672caef1366be
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Allen-Webb/modules-add-modalias-file-to-sysfs-for-modules/20221201-061550
git checkout 8a0305181ba6d360a1aaba1384e672caef1366be
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> ld.lld: error: undefined symbol: usb_drv_to_modalias
>>> referenced by driver.c
>>> drivers/usb/core/driver.o:(usb_bus_type) in archive vmlinux.a
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On Wed, Nov 30, 2022 at 04:14:47PM -0600, Allen Webb wrote:
> USB devices support the authorized attribute which can be used by
> user-space to implement trust-based systems for enabling USB devices. It
> would be helpful when building these systems to be able to know in
> advance which kernel drivers (or modules) are reachable from a
> particular USB device.
>
> This information is readily available for external modules in
> modules.alias. However, builtin kernel modules are not covered. This
> patch adds a sys-fs attribute to both builtin and loaded modules
> exposing the matching rules in the modalias format for integration
> with tools like USBGuard.
>
> Signed-off-by: Allen Webb <[email protected]>
> ---
> drivers/base/Makefile | 2 +-
> drivers/base/base.h | 8 +
> drivers/base/bus.c | 42 ++++++
> drivers/base/mod_devicetable.c | 257 +++++++++++++++++++++++++++++++++
> drivers/usb/core/driver.c | 2 +
> include/linux/device/bus.h | 8 +
> include/linux/module.h | 1 +
> kernel/module/internal.h | 2 +
> kernel/module/sysfs.c | 88 +++++++++++
> kernel/params.c | 7 +
> 10 files changed, 416 insertions(+), 1 deletion(-)
> create mode 100644 drivers/base/mod_devicetable.c
>
> Fixed another kernel config incompability identified by:
> | Reported-by: kernel test robot <[email protected]>
>
> Fixed a couple CHECK messages from checkpatch.pl.
>
> There are still some CHECK messages that are by design in a macro that
> depends on modifying a parameter and string literal concatenation.
>
> There is also a maintainer file update warning for a new file, but that
> file is already covered by an existing maintainers entry.
>
Again, please read the kernel documentation for how to list changes that
have happened since the 1st version you submitted, it's impossible to
know what has changed in each version here, right? Also how has this
changed based on the reviews of the first release where people pointed
you at other alternatives? How did they not work out?
You might need to start including a 0/1 email with all of this
information as it needs an introduction in places, right?
thanks,
greg k-h
Hi Allen,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on driver-core/driver-core-testing]
[also build test ERROR on driver-core/driver-core-next driver-core/driver-core-linus mcgrof/modules-next usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.1-rc7 next-20221201]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Allen-Webb/modules-add-modalias-file-to-sysfs-for-modules/20221201-061550
patch link: https://lore.kernel.org/r/20221130221447.1202206-1-allenwebb%40google.com
patch subject: [PATCH v4] modules: add modalias file to sysfs for modules.
config: arm-randconfig-s043-20221128
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/8a0305181ba6d360a1aaba1384e672caef1366be
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Allen-Webb/modules-add-modalias-file-to-sysfs-for-modules/20221201-061550
git checkout 8a0305181ba6d360a1aaba1384e672caef1366be
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> arm-linux-gnueabi-ld: drivers/usb/core/driver.o:(.data+0x4c): undefined reference to `usb_drv_to_modalias'
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On Tue, Nov 29, 2022 at 04:43:13PM -0600, Allen Webb wrote:
> This information is readily available for external modules in
> modules.alias. However, builtin kernel modules are not covered.
Why add this into the kernel instead of just a modules.builtin.alias
or something like that?
Luis
On Wed, Dec 7, 2022 at 8:34 PM Luis Chamberlain <[email protected]> wrote:
>
> On Tue, Nov 29, 2022 at 04:43:13PM -0600, Allen Webb wrote:
> > This information is readily available for external modules in
> > modules.alias. However, builtin kernel modules are not covered.
>
> Why add this into the kernel instead of just a modules.builtin.alias
> or something like that?
>
> Luis
I am fine with that approach and already have a PoC for it. Here are
some considerations:
* This would add a new file to the kernel packaging requirements.
* It is easier for separate files to get out of sync with the runtime
state (this isn't really a big deal because we already have to deal
with it for modules.alias)
On Thu, Dec 08, 2022 at 08:22:56AM -0600, Allen Webb wrote:
> On Wed, Dec 7, 2022 at 8:34 PM Luis Chamberlain <[email protected]> wrote:
> >
> > On Tue, Nov 29, 2022 at 04:43:13PM -0600, Allen Webb wrote:
> > > This information is readily available for external modules in
> > > modules.alias. However, builtin kernel modules are not covered.
> >
> > Why add this into the kernel instead of just a modules.builtin.alias
> > or something like that?
> >
> > Luis
>
> I am fine with that approach and already have a PoC for it. Here are
> some considerations:
> * This would add a new file to the kernel packaging requirements.
That's easy, you add it to the build process and the tools that pick up
kernels to package them, grab everything that the build process creates.
> * It is easier for separate files to get out of sync with the runtime
> state (this isn't really a big deal because we already have to deal
> with it for modules.alias)
How can it get out of sync if it came directly from the kernel image
itself?
I think this really is the best solution, as it should be much simpler
overall and not require every bus to add special code for it, right?
thanks,
greg k-h
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/[email protected]/
v2 (missing v2 label): https://lore.kernel.org/lkml/[email protected]/
v3: https://lore.kernel.org/lkml/[email protected]/
v4: https://lore.kernel.org/lkml/[email protected]/
v5: https://lore.kernel.org/lkml/[email protected]/
v6: https://lore.kernel.org/lkml/[email protected]/
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 <[email protected]> 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
Keep track of the module name when processing match table symbols.
Signed-off-by: Allen Webb <[email protected]>
---
scripts/mod/file2alias.c | 37 +++++++++++++++++++++++++++++++++----
scripts/mod/modpost.h | 1 +
2 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 80d973144fded..458e41ae0f5f1 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,11 @@ 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 +1569,29 @@ 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 +1623,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.3
This populates the mod->modalias_buf with aliases for built-in modules
when modpost is run against vmlinuz.o.
Signed-off-by: Allen Webb <[email protected]>
---
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 458e41ae0f5f1..8ed08154b3d81 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);
}
}
@@ -662,19 +674,25 @@ 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;
+ char acpi_id[PNP_ID_LEN];
+ int k;
+
+ 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.3
Hi Allen,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on masahiroy-kbuild/fixes]
[also build test WARNING on mcgrof/modules-next westeri-thunderbolt/next linus/master v6.1 next-20221216]
[cannot apply to masahiroy-kbuild/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Allen-Webb/module-h-MODULE_DEVICE_TABLE-for-built-in-modules/20221217-071949
base: https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git fixes
patch link: https://lore.kernel.org/r/20221216221703.294683-5-allenwebb%40google.com
patch subject: [PATCH v7 4/5] file2alias.c: Implement builtin.alias generation
config: powerpc-allnoconfig
compiler: powerpc-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/e65dd221062ee2430d083c7a65400fe1b1fc771f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Allen-Webb/module-h-MODULE_DEVICE_TABLE-for-built-in-modules/20221217-071949
git checkout e65dd221062ee2430d083c7a65400fe1b1fc771f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc prepare
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
scripts/mod/file2alias.c: In function 'do_pnp_card_entries':
>> scripts/mod/file2alias.c:679:25: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
679 | char acpi_id[PNP_ID_LEN];
| ^~~~
--
scripts/mod/file2alias.c: In function 'do_pnp_card_entries':
>> scripts/mod/file2alias.c:679:25: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
679 | char acpi_id[PNP_ID_LEN];
| ^~~~
vim +679 scripts/mod/file2alias.c
^1da177e4c3f41 Linus Torvalds 2005-04-16 633
0c81eed4b9d627 Kay Sievers 2008-02-21 634 /* looks like: "pnp:dD" for every device of the card */
0c81eed4b9d627 Kay Sievers 2008-02-21 635 static void do_pnp_card_entries(void *symval, unsigned long size,
0c81eed4b9d627 Kay Sievers 2008-02-21 636 struct module *mod)
^1da177e4c3f41 Linus Torvalds 2005-04-16 637 {
6543becf26fff6 Andreas Schwab 2013-01-20 638 const unsigned long id_size = SIZE_pnp_card_device_id;
0c81eed4b9d627 Kay Sievers 2008-02-21 639 const unsigned int count = (size / id_size)-1;
0c81eed4b9d627 Kay Sievers 2008-02-21 640 unsigned int i;
0c81eed4b9d627 Kay Sievers 2008-02-21 641
0c81eed4b9d627 Kay Sievers 2008-02-21 642 device_id_check(mod->name, "pnp", size, id_size, symval);
0c81eed4b9d627 Kay Sievers 2008-02-21 643
0c81eed4b9d627 Kay Sievers 2008-02-21 644 for (i = 0; i < count; i++) {
0c81eed4b9d627 Kay Sievers 2008-02-21 645 unsigned int j;
6543becf26fff6 Andreas Schwab 2013-01-20 646 DEF_FIELD_ADDR(symval + i * id_size, pnp_card_device_id, devs);
0c81eed4b9d627 Kay Sievers 2008-02-21 647
0c81eed4b9d627 Kay Sievers 2008-02-21 648 for (j = 0; j < PNP_MAX_DEVICES; j++) {
6543becf26fff6 Andreas Schwab 2013-01-20 649 const char *id = (char *)(*devs)[j].id;
0c81eed4b9d627 Kay Sievers 2008-02-21 650 int i2, j2;
0c81eed4b9d627 Kay Sievers 2008-02-21 651 int dup = 0;
0c81eed4b9d627 Kay Sievers 2008-02-21 652
0c81eed4b9d627 Kay Sievers 2008-02-21 653 if (!id[0])
0c81eed4b9d627 Kay Sievers 2008-02-21 654 break;
0c81eed4b9d627 Kay Sievers 2008-02-21 655
0c81eed4b9d627 Kay Sievers 2008-02-21 656 /* find duplicate, already added value */
0c81eed4b9d627 Kay Sievers 2008-02-21 657 for (i2 = 0; i2 < i && !dup; i2++) {
c2b1a9226fe7c1 Leonardo Bras 2018-10-24 658 DEF_FIELD_ADDR_VAR(symval + i2 * id_size,
c2b1a9226fe7c1 Leonardo Bras 2018-10-24 659 pnp_card_device_id,
c2b1a9226fe7c1 Leonardo Bras 2018-10-24 660 devs, devs_dup);
0c81eed4b9d627 Kay Sievers 2008-02-21 661
0c81eed4b9d627 Kay Sievers 2008-02-21 662 for (j2 = 0; j2 < PNP_MAX_DEVICES; j2++) {
c2b1a9226fe7c1 Leonardo Bras 2018-10-24 663 const char *id2 =
c2b1a9226fe7c1 Leonardo Bras 2018-10-24 664 (char *)(*devs_dup)[j2].id;
^1da177e4c3f41 Linus Torvalds 2005-04-16 665
0c81eed4b9d627 Kay Sievers 2008-02-21 666 if (!id2[0])
0c81eed4b9d627 Kay Sievers 2008-02-21 667 break;
0c81eed4b9d627 Kay Sievers 2008-02-21 668
0c81eed4b9d627 Kay Sievers 2008-02-21 669 if (!strcmp(id, id2)) {
0c81eed4b9d627 Kay Sievers 2008-02-21 670 dup = 1;
^1da177e4c3f41 Linus Torvalds 2005-04-16 671 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 672 }
0c81eed4b9d627 Kay Sievers 2008-02-21 673 }
0c81eed4b9d627 Kay Sievers 2008-02-21 674 }
0c81eed4b9d627 Kay Sievers 2008-02-21 675
0c81eed4b9d627 Kay Sievers 2008-02-21 676 /* add an individual alias for every device entry */
e65dd221062ee2 Allen Webb 2022-12-16 677 if (dup)
e65dd221062ee2 Allen Webb 2022-12-16 678 continue;
6543becf26fff6 Andreas Schwab 2013-01-20 @679 char acpi_id[PNP_ID_LEN];
72638f598ec9f0 Kay Sievers 2009-01-08 680 int k;
72638f598ec9f0 Kay Sievers 2009-01-08 681
0c81eed4b9d627 Kay Sievers 2008-02-21 682 buf_printf(&mod->dev_table_buf,
0c81eed4b9d627 Kay Sievers 2008-02-21 683 "MODULE_ALIAS(\"pnp:d%s*\");\n", id);
e65dd221062ee2 Allen Webb 2022-12-16 684 if (mod->builtin_name)
e65dd221062ee2 Allen Webb 2022-12-16 685 buf_printf(&mod->modalias_buf, "alias pnp:d%s* %s\n",
e65dd221062ee2 Allen Webb 2022-12-16 686 id, mod->builtin_name);
72638f598ec9f0 Kay Sievers 2009-01-08 687
72638f598ec9f0 Kay Sievers 2009-01-08 688 /* fix broken pnp bus lowercasing */
72638f598ec9f0 Kay Sievers 2009-01-08 689 for (k = 0; k < sizeof(acpi_id); k++)
72638f598ec9f0 Kay Sievers 2009-01-08 690 acpi_id[k] = toupper(id[k]);
22454cb99fc39f Kay Sievers 2008-05-28 691 buf_printf(&mod->dev_table_buf,
72638f598ec9f0 Kay Sievers 2009-01-08 692 "MODULE_ALIAS(\"acpi*:%s:*\");\n", acpi_id);
e65dd221062ee2 Allen Webb 2022-12-16 693 if (mod->builtin_name)
e65dd221062ee2 Allen Webb 2022-12-16 694 buf_printf(&mod->modalias_buf, "alias acpi*:%s:* %s\n",
e65dd221062ee2 Allen Webb 2022-12-16 695 acpi_id, mod->builtin_name);
0c81eed4b9d627 Kay Sievers 2008-02-21 696 }
0c81eed4b9d627 Kay Sievers 2008-02-21 697 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 698 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 699
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Hi Allen,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on masahiroy-kbuild/fixes]
[also build test WARNING on mcgrof/modules-next westeri-thunderbolt/next linus/master v6.1 next-20221216]
[cannot apply to masahiroy-kbuild/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Allen-Webb/module-h-MODULE_DEVICE_TABLE-for-built-in-modules/20221217-071949
base: https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git fixes
patch link: https://lore.kernel.org/r/20221216221703.294683-5-allenwebb%40google.com
patch subject: [PATCH v7 4/5] file2alias.c: Implement builtin.alias generation
config: x86_64-randconfig-a001
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/e65dd221062ee2430d083c7a65400fe1b1fc771f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Allen-Webb/module-h-MODULE_DEVICE_TABLE-for-built-in-modules/20221217-071949
git checkout e65dd221062ee2430d083c7a65400fe1b1fc771f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 prepare
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> scripts/mod/file2alias.c:679:9: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
char acpi_id[PNP_ID_LEN];
^
1 warning generated.
--
>> scripts/mod/file2alias.c:679:9: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
char acpi_id[PNP_ID_LEN];
^
1 warning generated.
vim +679 scripts/mod/file2alias.c
^1da177e4c3f41 Linus Torvalds 2005-04-16 633
0c81eed4b9d627 Kay Sievers 2008-02-21 634 /* looks like: "pnp:dD" for every device of the card */
0c81eed4b9d627 Kay Sievers 2008-02-21 635 static void do_pnp_card_entries(void *symval, unsigned long size,
0c81eed4b9d627 Kay Sievers 2008-02-21 636 struct module *mod)
^1da177e4c3f41 Linus Torvalds 2005-04-16 637 {
6543becf26fff6 Andreas Schwab 2013-01-20 638 const unsigned long id_size = SIZE_pnp_card_device_id;
0c81eed4b9d627 Kay Sievers 2008-02-21 639 const unsigned int count = (size / id_size)-1;
0c81eed4b9d627 Kay Sievers 2008-02-21 640 unsigned int i;
0c81eed4b9d627 Kay Sievers 2008-02-21 641
0c81eed4b9d627 Kay Sievers 2008-02-21 642 device_id_check(mod->name, "pnp", size, id_size, symval);
0c81eed4b9d627 Kay Sievers 2008-02-21 643
0c81eed4b9d627 Kay Sievers 2008-02-21 644 for (i = 0; i < count; i++) {
0c81eed4b9d627 Kay Sievers 2008-02-21 645 unsigned int j;
6543becf26fff6 Andreas Schwab 2013-01-20 646 DEF_FIELD_ADDR(symval + i * id_size, pnp_card_device_id, devs);
0c81eed4b9d627 Kay Sievers 2008-02-21 647
0c81eed4b9d627 Kay Sievers 2008-02-21 648 for (j = 0; j < PNP_MAX_DEVICES; j++) {
6543becf26fff6 Andreas Schwab 2013-01-20 649 const char *id = (char *)(*devs)[j].id;
0c81eed4b9d627 Kay Sievers 2008-02-21 650 int i2, j2;
0c81eed4b9d627 Kay Sievers 2008-02-21 651 int dup = 0;
0c81eed4b9d627 Kay Sievers 2008-02-21 652
0c81eed4b9d627 Kay Sievers 2008-02-21 653 if (!id[0])
0c81eed4b9d627 Kay Sievers 2008-02-21 654 break;
0c81eed4b9d627 Kay Sievers 2008-02-21 655
0c81eed4b9d627 Kay Sievers 2008-02-21 656 /* find duplicate, already added value */
0c81eed4b9d627 Kay Sievers 2008-02-21 657 for (i2 = 0; i2 < i && !dup; i2++) {
c2b1a9226fe7c1 Leonardo Bras 2018-10-24 658 DEF_FIELD_ADDR_VAR(symval + i2 * id_size,
c2b1a9226fe7c1 Leonardo Bras 2018-10-24 659 pnp_card_device_id,
c2b1a9226fe7c1 Leonardo Bras 2018-10-24 660 devs, devs_dup);
0c81eed4b9d627 Kay Sievers 2008-02-21 661
0c81eed4b9d627 Kay Sievers 2008-02-21 662 for (j2 = 0; j2 < PNP_MAX_DEVICES; j2++) {
c2b1a9226fe7c1 Leonardo Bras 2018-10-24 663 const char *id2 =
c2b1a9226fe7c1 Leonardo Bras 2018-10-24 664 (char *)(*devs_dup)[j2].id;
^1da177e4c3f41 Linus Torvalds 2005-04-16 665
0c81eed4b9d627 Kay Sievers 2008-02-21 666 if (!id2[0])
0c81eed4b9d627 Kay Sievers 2008-02-21 667 break;
0c81eed4b9d627 Kay Sievers 2008-02-21 668
0c81eed4b9d627 Kay Sievers 2008-02-21 669 if (!strcmp(id, id2)) {
0c81eed4b9d627 Kay Sievers 2008-02-21 670 dup = 1;
^1da177e4c3f41 Linus Torvalds 2005-04-16 671 break;
^1da177e4c3f41 Linus Torvalds 2005-04-16 672 }
0c81eed4b9d627 Kay Sievers 2008-02-21 673 }
0c81eed4b9d627 Kay Sievers 2008-02-21 674 }
0c81eed4b9d627 Kay Sievers 2008-02-21 675
0c81eed4b9d627 Kay Sievers 2008-02-21 676 /* add an individual alias for every device entry */
e65dd221062ee2 Allen Webb 2022-12-16 677 if (dup)
e65dd221062ee2 Allen Webb 2022-12-16 678 continue;
6543becf26fff6 Andreas Schwab 2013-01-20 @679 char acpi_id[PNP_ID_LEN];
72638f598ec9f0 Kay Sievers 2009-01-08 680 int k;
72638f598ec9f0 Kay Sievers 2009-01-08 681
0c81eed4b9d627 Kay Sievers 2008-02-21 682 buf_printf(&mod->dev_table_buf,
0c81eed4b9d627 Kay Sievers 2008-02-21 683 "MODULE_ALIAS(\"pnp:d%s*\");\n", id);
e65dd221062ee2 Allen Webb 2022-12-16 684 if (mod->builtin_name)
e65dd221062ee2 Allen Webb 2022-12-16 685 buf_printf(&mod->modalias_buf, "alias pnp:d%s* %s\n",
e65dd221062ee2 Allen Webb 2022-12-16 686 id, mod->builtin_name);
72638f598ec9f0 Kay Sievers 2009-01-08 687
72638f598ec9f0 Kay Sievers 2009-01-08 688 /* fix broken pnp bus lowercasing */
72638f598ec9f0 Kay Sievers 2009-01-08 689 for (k = 0; k < sizeof(acpi_id); k++)
72638f598ec9f0 Kay Sievers 2009-01-08 690 acpi_id[k] = toupper(id[k]);
22454cb99fc39f Kay Sievers 2008-05-28 691 buf_printf(&mod->dev_table_buf,
72638f598ec9f0 Kay Sievers 2009-01-08 692 "MODULE_ALIAS(\"acpi*:%s:*\");\n", acpi_id);
e65dd221062ee2 Allen Webb 2022-12-16 693 if (mod->builtin_name)
e65dd221062ee2 Allen Webb 2022-12-16 694 buf_printf(&mod->modalias_buf, "alias acpi*:%s:* %s\n",
e65dd221062ee2 Allen Webb 2022-12-16 695 acpi_id, mod->builtin_name);
0c81eed4b9d627 Kay Sievers 2008-02-21 696 }
0c81eed4b9d627 Kay Sievers 2008-02-21 697 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 698 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 699
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Le 16/12/2022 à 23:17, Allen Webb a écrit :
> Keep track of the module name when processing match table symbols.
>
> Signed-off-by: Allen Webb <[email protected]>
> ---
> scripts/mod/file2alias.c | 37 +++++++++++++++++++++++++++++++++----
> scripts/mod/modpost.h | 1 +
> 2 files changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 80d973144fded..458e41ae0f5f1 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,11 @@ 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
> + */
Comments style is wrong. Multiline comments outside net/ start with an
empty /* line.
> if (strncmp(symname, "__mod_", strlen("__mod_")))
> return;
> name = symname + strlen("__mod_");
> @@ -1564,8 +1569,29 @@ 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 +1623,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[];
> };
>
Le 16/12/2022 à 23:17, Allen Webb a écrit :
> This populates the mod->modalias_buf with aliases for built-in modules
> when modpost is run against vmlinuz.o.
>
> Signed-off-by: Allen Webb <[email protected]>
> ---
> 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 458e41ae0f5f1..8ed08154b3d81 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);
> }
> }
>
> @@ -662,19 +674,25 @@ 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;
> + char acpi_id[PNP_ID_LEN];
> + int k;
No declarations in the middle of a block. Put declarations before code.
> +
> + 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);
> }
> }
>
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/[email protected]/
v2 (missing v2 label): https://lore.kernel.org/lkml/[email protected]/
v3: https://lore.kernel.org/lkml/[email protected]/
v4: https://lore.kernel.org/lkml/[email protected]/
v5: https://lore.kernel.org/lkml/[email protected]/
v6: https://lore.kernel.org/lkml/[email protected]/
v7: https://lore.kernel.org/lkml/[email protected]/
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 <[email protected]> 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
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 <[email protected]>
---
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.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 <[email protected]>
Signed-off-by: Allen Webb <[email protected]>
---
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.3
A small difference in the name supplied to MODULE_DEVICE_TABLE
breaks a future patch set, so fix the typo.
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Allen Webb <[email protected]>
---
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.3
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 <[email protected]>
> Signed-off-by: Allen Webb <[email protected]>
> ---
> 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
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 <[email protected]>
Signed-off-by: Allen Webb <[email protected]>
---
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.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 <[email protected]>
Signed-off-by: Allen Webb <[email protected]>
---
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.3
Generate modules.builtin.alias using modpost and install it with the
modules.
Signed-off-by: Allen Webb <[email protected]>
---
.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
This adds an unimplemented command line flag for writing the built-in
aliases to a file.
Signed-off-by: Allen Webb <[email protected]>
---
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.3
On Mon, Dec 19, 2022 at 1:22 PM Greg Kroah-Hartman
<[email protected]> 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 <[email protected]>
> > Signed-off-by: Allen Webb <[email protected]>
> > ---
> > 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
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 <[email protected]> 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/[email protected]/
v2 (missing v2 label): https://lore.kernel.org/lkml/[email protected]/
v3: https://lore.kernel.org/lkml/[email protected]/
v4: https://lore.kernel.org/lkml/[email protected]/
v5: https://lore.kernel.org/lkml/[email protected]/
v6: https://lore.kernel.org/lkml/[email protected]/
v7: https://lore.kernel.org/lkml/[email protected]/
v8: https://lore.kernel.org/lkml/[email protected]/
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 <[email protected]> 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
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 <[email protected]>
Signed-off-by: Allen Webb <[email protected]>
---
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.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/[email protected]/
v2 (missing v2 label): https://lore.kernel.org/lkml/[email protected]/
v3: https://lore.kernel.org/lkml/[email protected]/
v4: https://lore.kernel.org/lkml/[email protected]/
v5: https://lore.kernel.org/lkml/[email protected]/
v6: https://lore.kernel.org/lkml/[email protected]/
v7: https://lore.kernel.org/lkml/[email protected]/
v8: https://lore.kernel.org/lkml/[email protected]/
v9: https://lore.kernel.org/lkml/[email protected]/
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 <[email protected]> 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 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 <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Allen Webb <[email protected]>
---
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.2
Keep track of the module name when processing match table symbols.
Signed-off-by: Allen Webb <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Allen Webb <[email protected]>
---
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.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 <[email protected]>
---
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.2
Generate modules.builtin.alias using modpost and install it with the
modules.
Signed-off-by: Allen Webb <[email protected]>
---
.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.2
Built-in modules that set id_table need to set MODULE_DEVICE_TABLE so
update the documentation accordingly.
Signed-off-by: Allen Webb <[email protected]>
---
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.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 <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Allen Webb <[email protected]>
---
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.2
Update the documentation to include the presense and use case of
modules.builtin.alias.
Reviewed-by: Luis Chamberlain <[email protected]>
Signed-off-by: Allen Webb <[email protected]>
---
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.2
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 <[email protected]>
---
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.2
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.
Signed-off-by: Allen Webb <[email protected]>
---
.../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.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
On 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 <[email protected]>
> ---
> .../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
On 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 <[email protected]>
> ---
> 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
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 <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]/
> Signed-off-by: Allen Webb <[email protected]>
> ---
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: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 <[email protected]>
> ---
> 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: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 <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]/
> Signed-off-by: Allen Webb <[email protected]>
> ---
Oh feel free to add Reviewed-by: Luis Chamberlain <[email protected]>
on patches 1-3.
Luis
On 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 <[email protected]>
> ---
> 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
>
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 <[email protected]>
> ---
> .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
>
On 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 <[email protected]>
> ---
> 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
>
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 <[email protected]> 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 <[email protected]>
> > ---
> > .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 <[email protected]> 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 <[email protected]>
> > > ---
> > > .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