lkml.org 
[lkml]   [2022]   [Apr]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC] PCI: sysfs: add bypass for config read admin check


On April 6, 2022 4:17:51 AM PDT, Bjorn Helgaas <helgaas@kernel.org> wrote:
>[+cc Kees]
>
>On Wed, Apr 06, 2022 at 04:11:31PM +0900, David Stevens wrote:
>> From: David Stevens <stevensd@chromium.org>
>>
>> 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@chromium.org>
>> ---
>> 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

\
 
 \ /
  Last update: 2022-04-08 17:51    [W:0.116 / U:0.564 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site