Re: [PATCH v3 1/3] PCI/sysfs: Check CAP_SYS_ADMIN before parsing user input

[Bjorn Helgaas <helgaas@kernel.org>]

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@linux.com>

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
>