From 95e83e219d68956ba4fed9326683e4d2bd3e39a9 Mon Sep 17 00:00:00 2001 From: Krzysztof Wilczyński Date: Wed, 15 Sep 2021 23:01:25 +0000 Subject: PCI/sysfs: Check CAP_SYS_ADMIN before parsing user input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check if the "CAP_SYS_ADMIN" capability flag is set before parsing user input as it makes more sense to first check whether the current user actually has the right permissions before accepting any input from such user. This will also make order in which enable_store() and msi_bus_store() perform the "CAP_SYS_ADMIN" capability check consistent with other PCI-related sysfs objects that first verify whether user has this capability set. Link: https://lore.kernel.org/r/20210915230127.2495723-1-kw@linux.com Signed-off-by: Krzysztof Wilczyński Signed-off-by: Bjorn Helgaas --- drivers/pci/pci-sysfs.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'drivers/pci/pci-sysfs.c') diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c index 7fb5cd17cc98..6832e161be1c 100644 --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c @@ -273,15 +273,16 @@ static ssize_t enable_store(struct device *dev, struct device_attribute *attr, { struct pci_dev *pdev = to_pci_dev(dev); unsigned long val; - ssize_t result = kstrtoul(buf, 0, &val); - - if (result < 0) - return result; + ssize_t result; /* this can crash the machine when done on the "wrong" device */ if (!capable(CAP_SYS_ADMIN)) return -EPERM; + result = kstrtoul(buf, 0, &val); + if (result < 0) + return result; + device_lock(dev); if (dev->driver) result = -EBUSY; @@ -378,12 +379,12 @@ static ssize_t msi_bus_store(struct device *dev, struct device_attribute *attr, struct pci_bus *subordinate = pdev->subordinate; unsigned long val; - if (kstrtoul(buf, 0, &val) < 0) - return -EINVAL; - if (!capable(CAP_SYS_ADMIN)) return -EPERM; + if (kstrtoul(buf, 0, &val) < 0) + return -EINVAL; + /* * "no_msi" and "bus_flags" only affect what happens when a driver * requests MSI or MSI-X. They don't affect any drivers that have -- cgit v1.2.3 From 36f354ec7bf92f8aaf09eaf3b261ea06c25ec337 Mon Sep 17 00:00:00 2001 From: Krzysztof Wilczyński Date: Wed, 15 Sep 2021 23:01:26 +0000 Subject: PCI/sysfs: Return -EINVAL consistently from "store" functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Most of the "store" functions that handle userspace input via sysfs return -EINVAL should the value fail validation and/or type conversion. This error code is a clear message to userspace that the value is not a valid input. However, some of the "show" functions return input parsing error codes as-is, which may be either -EINVAL or -ERANGE. The former would often be from kstrtobool(), and the latter typically from other kstr*() functions such as kstrtou8(), kstrtou32(), kstrtoint(), etc. -EINVAL is commonly returned as the error code to indicate that the value provided is invalid, but -ERANGE is not very useful in userspace. Therefore, normalize the return error code to be -EINVAL for when the validation and/or type conversion fails. Link: https://lore.kernel.org/r/20210915230127.2495723-2-kw@linux.com Signed-off-by: Krzysztof Wilczyński Signed-off-by: Bjorn Helgaas --- drivers/pci/endpoint/functions/pci-epf-ntb.c | 18 +++++--------- drivers/pci/endpoint/pci-ep-cfs.c | 35 ++++++++++------------------ drivers/pci/iov.c | 14 +++++------ drivers/pci/pci-sysfs.c | 20 +++++++--------- 4 files changed, 33 insertions(+), 54 deletions(-) (limited to 'drivers/pci/pci-sysfs.c') diff --git a/drivers/pci/endpoint/functions/pci-epf-ntb.c b/drivers/pci/endpoint/functions/pci-epf-ntb.c index 8b4756159f15..e67489144349 100644 --- a/drivers/pci/endpoint/functions/pci-epf-ntb.c +++ b/drivers/pci/endpoint/functions/pci-epf-ntb.c @@ -1947,11 +1947,9 @@ static ssize_t epf_ntb_##_name##_store(struct config_item *item, \ struct config_group *group = to_config_group(item); \ struct epf_ntb *ntb = to_epf_ntb(group); \ u32 val; \ - int ret; \ \ - ret = kstrtou32(page, 0, &val); \ - if (ret) \ - return ret; \ + if (kstrtou32(page, 0, &val) < 0) \ + return -EINVAL; \ \ ntb->_name = val; \ \ @@ -1980,11 +1978,9 @@ static ssize_t epf_ntb_##_name##_store(struct config_item *item, \ struct device *dev = &ntb->epf->dev; \ int win_no; \ u64 val; \ - int ret; \ \ - ret = kstrtou64(page, 0, &val); \ - if (ret) \ - return ret; \ + if (kstrtou64(page, 0, &val) < 0) \ + return -EINVAL; \ \ if (sscanf(#_name, "mw%d", &win_no) != 1) \ return -EINVAL; \ @@ -2005,11 +2001,9 @@ static ssize_t epf_ntb_num_mws_store(struct config_item *item, struct config_group *group = to_config_group(item); struct epf_ntb *ntb = to_epf_ntb(group); u32 val; - int ret; - ret = kstrtou32(page, 0, &val); - if (ret) - return ret; + if (kstrtou32(page, 0, &val) < 0) + return -EINVAL; if (val > MAX_MW) return -EINVAL; diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c index 999911801877..19bc0e828c0c 100644 --- a/drivers/pci/endpoint/pci-ep-cfs.c +++ b/drivers/pci/endpoint/pci-ep-cfs.c @@ -175,9 +175,8 @@ static ssize_t pci_epc_start_store(struct config_item *item, const char *page, epc = epc_group->epc; - ret = kstrtobool(page, &start); - if (ret) - return ret; + if (kstrtobool(page, &start) < 0) + return -EINVAL; if (!start) { pci_epc_stop(epc); @@ -329,13 +328,11 @@ static ssize_t pci_epf_##_name##_store(struct config_item *item, \ const char *page, size_t len) \ { \ u32 val; \ - int ret; \ struct pci_epf *epf = to_pci_epf_group(item)->epf; \ if (WARN_ON_ONCE(!epf->header)) \ return -EINVAL; \ - ret = kstrtou32(page, 0, &val); \ - if (ret) \ - return ret; \ + if (kstrtou32(page, 0, &val) < 0) \ + return -EINVAL; \ epf->header->_name = val; \ return len; \ } @@ -345,13 +342,11 @@ static ssize_t pci_epf_##_name##_store(struct config_item *item, \ const char *page, size_t len) \ { \ u16 val; \ - int ret; \ struct pci_epf *epf = to_pci_epf_group(item)->epf; \ if (WARN_ON_ONCE(!epf->header)) \ return -EINVAL; \ - ret = kstrtou16(page, 0, &val); \ - if (ret) \ - return ret; \ + if (kstrtou16(page, 0, &val) < 0) \ + return -EINVAL; \ epf->header->_name = val; \ return len; \ } @@ -361,13 +356,11 @@ static ssize_t pci_epf_##_name##_store(struct config_item *item, \ const char *page, size_t len) \ { \ u8 val; \ - int ret; \ struct pci_epf *epf = to_pci_epf_group(item)->epf; \ if (WARN_ON_ONCE(!epf->header)) \ return -EINVAL; \ - ret = kstrtou8(page, 0, &val); \ - if (ret) \ - return ret; \ + if (kstrtou8(page, 0, &val) < 0) \ + return -EINVAL; \ epf->header->_name = val; \ return len; \ } @@ -376,11 +369,9 @@ static ssize_t pci_epf_msi_interrupts_store(struct config_item *item, const char *page, size_t len) { u8 val; - int ret; - ret = kstrtou8(page, 0, &val); - if (ret) - return ret; + if (kstrtou8(page, 0, &val) < 0) + return -EINVAL; to_pci_epf_group(item)->epf->msi_interrupts = val; @@ -398,11 +389,9 @@ static ssize_t pci_epf_msix_interrupts_store(struct config_item *item, const char *page, size_t len) { u16 val; - int ret; - ret = kstrtou16(page, 0, &val); - if (ret) - return ret; + if (kstrtou16(page, 0, &val) < 0) + return -EINVAL; to_pci_epf_group(item)->epf->msix_interrupts = val; diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index dafdc652fcd0..0267977c9f17 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c @@ -183,11 +183,10 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev, { struct pci_dev *vf_dev = to_pci_dev(dev); struct pci_dev *pdev = pci_physfn(vf_dev); - int val, ret; + int val, ret = 0; - ret = kstrtoint(buf, 0, &val); - if (ret) - return ret; + if (kstrtoint(buf, 0, &val) < 0) + return -EINVAL; if (val < 0) return -EINVAL; @@ -376,12 +375,11 @@ static ssize_t sriov_numvfs_store(struct device *dev, const char *buf, size_t count) { struct pci_dev *pdev = to_pci_dev(dev); - int ret; + int ret = 0; u16 num_vfs; - ret = kstrtou16(buf, 0, &num_vfs); - if (ret < 0) - return ret; + if (kstrtou16(buf, 0, &num_vfs) < 0) + return -EINVAL; if (num_vfs > pci_sriov_get_totalvfs(pdev)) return -ERANGE; diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c index 6832e161be1c..a092fc0c665d 100644 --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c @@ -273,15 +273,14 @@ static ssize_t enable_store(struct device *dev, struct device_attribute *attr, { struct pci_dev *pdev = to_pci_dev(dev); unsigned long val; - ssize_t result; + ssize_t result = 0; /* this can crash the machine when done on the "wrong" device */ if (!capable(CAP_SYS_ADMIN)) return -EPERM; - result = kstrtoul(buf, 0, &val); - if (result < 0) - return result; + if (kstrtoul(buf, 0, &val) < 0) + return -EINVAL; device_lock(dev); if (dev->driver) @@ -313,14 +312,13 @@ static ssize_t numa_node_store(struct device *dev, size_t count) { struct pci_dev *pdev = to_pci_dev(dev); - int node, ret; + int node; if (!capable(CAP_SYS_ADMIN)) return -EPERM; - ret = kstrtoint(buf, 0, &node); - if (ret) - return ret; + if (kstrtoint(buf, 0, &node) < 0) + return -EINVAL; if ((node < 0 && node != NUMA_NO_NODE) || node >= MAX_NUMNODES) return -EINVAL; @@ -1340,10 +1338,10 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr, { struct pci_dev *pdev = to_pci_dev(dev); unsigned long val; - ssize_t result = kstrtoul(buf, 0, &val); + ssize_t result; - if (result < 0) - return result; + if (kstrtoul(buf, 0, &val) < 0) + return -EINVAL; if (val != 1) return -EINVAL; -- cgit v1.2.3 From ac8e3cef588c5affc6dfa7b693ec64bbf3cead6a Mon Sep 17 00:00:00 2001 From: Barry Song Date: Wed, 25 Aug 2021 18:26:35 +0800 Subject: PCI/sysfs: Explicitly show first MSI IRQ for 'irq' The sysfs "irq" file contains the legacy INTx IRQ. Or, if the device has MSI enabled, it contains the first MSI IRQ instead. Previously this file showed the pci_dev.irq value directly. But we'd prefer to use pci_dev.irq only for the INTx IRQ and decouple that from any MSI or MSI-X IRQs. If the device has MSI enabled, explicitly look up and show the first MSI IRQ in the sysfs "irq" file. Otherwise, show the INTx IRQ. This removes the requirement that msi_capability_init() set pci_dev.irq to the first MSI IRQ when enabling MSI and pci_msi_shutdown() restore the INTx IRQ when disabling MSI. [bhelgaas: commit log] Link: https://lore.kernel.org/r/20210825102636.52757-3-21cnbao@gmail.com Signed-off-by: Barry Song Signed-off-by: Bjorn Helgaas --- drivers/pci/pci-sysfs.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) (limited to 'drivers/pci/pci-sysfs.c') diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c index 7fb5cd17cc98..b50fe7ce234b 100644 --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include "pci.h" @@ -49,7 +50,28 @@ pci_config_attr(subsystem_vendor, "0x%04x\n"); pci_config_attr(subsystem_device, "0x%04x\n"); pci_config_attr(revision, "0x%02x\n"); pci_config_attr(class, "0x%06x\n"); -pci_config_attr(irq, "%u\n"); + +static ssize_t irq_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct pci_dev *pdev = to_pci_dev(dev); + +#ifdef CONFIG_PCI_MSI + /* + * For MSI, show the first MSI IRQ; for all other cases including + * MSI-X, show the legacy INTx IRQ. + */ + if (pdev->msi_enabled) { + struct msi_desc *desc = first_pci_msi_entry(pdev); + + return sysfs_emit(buf, "%u\n", desc->irq); + } +#endif + + return sysfs_emit(buf, "%u\n", pdev->irq); +} +static DEVICE_ATTR_RO(irq); static ssize_t broken_parity_status_show(struct device *dev, struct device_attribute *attr, -- cgit v1.2.3