Re: [PATCH 2/4] PCI/sysfs: Check CAP_SYS_ADMIN before parsing user input

[Bjorn Helgaas <helgaas@kernel.org>]

On Mon, Jul 05, 2021 at 09:23:06PM +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.

I like this one.  Can you rebase it to skip patch 1/4 (unless you
convince me that 1/4 is safe)?

> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
> ---
>  drivers/pci/pci-sysfs.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 0f98c4843764..bc4c141e4c1c 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -275,13 +275,13 @@ static ssize_t enable_store(struct device *dev, struct device_attribute *attr,
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	ssize_t result = 0;
>  
> -	if (kstrtobool(buf, &enable) < 0)
> -		return -EINVAL;
> -
>  	/* this can crash the machine when done on the "wrong" device */
>  	if (!capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
> +	if (kstrtobool(buf, &enable) < 0)
> +		return -EINVAL;
> +
>  	device_lock(dev);
>  	if (dev->driver)
>  		result = -EBUSY;
> @@ -377,12 +377,12 @@ static ssize_t msi_bus_store(struct device *dev, struct device_attribute *attr,
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct pci_bus *subordinate = pdev->subordinate;
>  
> -	if (kstrtobool(buf, &enable) < 0)
> -		return -EINVAL;
> -
>  	if (!capable(CAP_SYS_ADMIN))
>  		return -EPERM;
>  
> +	if (kstrtobool(buf, &enable) < 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.32.0
>