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@xxxxxxxxx> > --- > 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 >
