On Wed, Sep 15, 2021 at 11:01:25PM +0000, Krzysztof Wilczyński wrote:
> 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.
>
> Signed-off-by: Krzysztof Wilczyński <kw@xxxxxxxxx>
Applied all three to pci/sysfs for v5.16, thanks!
> ---
> drivers/pci/pci-sysfs.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> 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
> --
> 2.33.0
>
