[2/3] PCI: don't use snprintf() in driver_override_show()

Message ID 20170911074542.16777-3-nstange@suse.de
State New
Headers show
Series
  • make PCI's and platform's driver_override_store()/show() converge
Related show

Commit Message

Nicolai Stange Sept. 11, 2017, 7:45 a.m.
Quote from Documentation/filesystems/sysfs.txt:

  show() must not use snprintf() when formatting the value to be
  returned to user space. If you can guarantee that an overflow
  will never happen you can use sprintf() otherwise you must use
  scnprintf().

Commit 4efe874aace5 ("PCI: Don't read past the end of sysfs
"driver_override" buffer") introduced such a snprintf() usage from
driver_override_show() while at the same time tweaking
driver_override_store() such that the write buffer can't ever get
overflowed.

Reasoning:
Since aforementioned commit, driver_override_store() only accepts to be
written buffers less than PAGE_SIZE - 1 in size.

The then kstrndup()'ed driver_override string will be at most PAGE_SIZE - 1
in length, including the trailing '\0'.

After the addition of a '\n' in driver_override_show(), the result won't
exceed PAGE_SIZE characters in length, again including the trailing '\0'.

Hence, snprintf(buf, PAGE_SIZE, ...) and sprintf(buf, ...) are equivalent
at this point.

Replace the former by the latter in order to adhere to the rules in
Documentation/filesystems/sysfs.txt.

This is a style fix only and there's no change in functionality.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 drivers/pci/pci-sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Greg Kroah-Hartman Sept. 11, 2017, 11:55 a.m. | #1
On Mon, Sep 11, 2017 at 09:45:41AM +0200, Nicolai Stange wrote:
> Quote from Documentation/filesystems/sysfs.txt:
> 
>   show() must not use snprintf() when formatting the value to be
>   returned to user space. If you can guarantee that an overflow
>   will never happen you can use sprintf() otherwise you must use
>   scnprintf().
> 
> Commit 4efe874aace5 ("PCI: Don't read past the end of sysfs
> "driver_override" buffer") introduced such a snprintf() usage from
> driver_override_show() while at the same time tweaking
> driver_override_store() such that the write buffer can't ever get
> overflowed.
> 
> Reasoning:
> Since aforementioned commit, driver_override_store() only accepts to be
> written buffers less than PAGE_SIZE - 1 in size.
> 
> The then kstrndup()'ed driver_override string will be at most PAGE_SIZE - 1
> in length, including the trailing '\0'.
> 
> After the addition of a '\n' in driver_override_show(), the result won't
> exceed PAGE_SIZE characters in length, again including the trailing '\0'.
> 
> Hence, snprintf(buf, PAGE_SIZE, ...) and sprintf(buf, ...) are equivalent
> at this point.
> 
> Replace the former by the latter in order to adhere to the rules in
> Documentation/filesystems/sysfs.txt.
> 
> This is a style fix only and there's no change in functionality.
> 
> Signed-off-by: Nicolai Stange <nstange@suse.de>
> ---
>  drivers/pci/pci-sysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 8e075ea2743e..43f7fbede448 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -722,7 +722,7 @@ static ssize_t driver_override_show(struct device *dev,
>  	ssize_t len;
>  
>  	device_lock(dev);
> -	len = snprintf(buf, PAGE_SIZE, "%s\n", pdev->driver_override);
> +	len = sprintf(buf, "%s\n", pdev->driver_override);

While I'm all for changes like this, it's an uphill battle to change
them, usually it's best to just catch them before they go into the tree.

Anyway, nice summary, very good job with that.

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Patch

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 8e075ea2743e..43f7fbede448 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -722,7 +722,7 @@  static ssize_t driver_override_show(struct device *dev,
 	ssize_t len;
 
 	device_lock(dev);
-	len = snprintf(buf, PAGE_SIZE, "%s\n", pdev->driver_override);
+	len = sprintf(buf, "%s\n", pdev->driver_override);
 	device_unlock(dev);
 	return len;
 }