On April 6, 2022 4:17:51 AM PDT, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: >[+cc Kees] > >On Wed, Apr 06, 2022 at 04:11:31PM +0900, David Stevens wrote: >> From: David Stevens <stevensd@xxxxxxxxxxxx> >> >> Add a moduleparam that can be set to bypass the check that limits users >> without CAP_SYS_ADMIN to only being able to read the first 64 bytes of >> the config space. This allows systems without problematic hardware to be >> configured to allow users without CAP_SYS_ADMIN to read PCI >> capabilities. > >Can you expand this a bit to explain the purpose of this? I guess it >makes "lspci -v" work without having to be root? How much of a >problem is that? Is there some specific use case that needs this >change? Maybe there's some way to address that without having to add >a new parameter that bypasses CAP_SYS_ADMIN. Yeah, this doesn't seem right to me. There are tons of ways in userspace to deal with these permissions (e.g. sudo with lspci, suid wrapper, etc). -Kees > >> Signed-off-by: David Stevens <stevensd@xxxxxxxxxxxx> >> --- >> drivers/pci/pci-sysfs.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c >> index 602f0fb0b007..162423b3c052 100644 >> --- a/drivers/pci/pci-sysfs.c >> +++ b/drivers/pci/pci-sysfs.c >> @@ -28,10 +28,17 @@ >> #include <linux/pm_runtime.h> >> #include <linux/msi.h> >> #include <linux/of.h> >> +#include <linux/moduleparam.h> >> #include "pci.h" >> >> static int sysfs_initialized; /* = 0 */ >> >> +static bool allow_unsafe_config_reads; >> +module_param_named(allow_unsafe_config_reads, >> + allow_unsafe_config_reads, bool, 0644); >> +MODULE_PARM_DESC(allow_unsafe_config_reads, >> + "Enable full read access to config space without CAP_SYS_ADMIN."); >> + >> /* show configuration fields */ >> #define pci_config_attr(field, format_string) \ >> static ssize_t \ >> @@ -696,7 +703,8 @@ static ssize_t pci_read_config(struct file *filp, struct kobject *kobj, >> u8 *data = (u8 *) buf; >> >> /* Several chips lock up trying to read undefined config space */ >> - if (file_ns_capable(filp, &init_user_ns, CAP_SYS_ADMIN)) >> + if (allow_unsafe_config_reads || >> + file_ns_capable(filp, &init_user_ns, CAP_SYS_ADMIN)) >> size = dev->cfg_size; >> else if (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) >> size = 128; >> -- >> 2.35.1.1094.g7c7d902a7c-goog >> -- Kees Cook
