[PATCH] Staging: fbtbt: show() methods should use scnprintf()

[PATCH] Staging: fbtbt: show() methods should use scnprintf()

[Ksenija Stanojevic]

snprintf() may return a value that exceeds its second argument,
show() methods should use scnprintf() instead of snprintf().

Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
---
 drivers/staging/fbtft/fbtft-sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/fbtft/fbtft-sysfs.c b/drivers/staging/fbtft/fbtft-sysfs.c
index 8d8bd12..36a1f4e 100644
--- a/drivers/staging/fbtft/fbtft-sysfs.c
+++ b/drivers/staging/fbtft/fbtft-sysfs.c
@@ -198,7 +198,7 @@ static ssize_t show_debug(struct device *device,
 	struct fb_info *fb_info = dev_get_drvdata(device);
 	struct fbtft_par *par = fb_info->par;
 
-	return snprintf(buf, PAGE_SIZE, "%lu\n", par->debug);
+	return scnprintf(buf, PAGE_SIZE, "%lu\n", par->debug);
 }
 
 static struct device_attribute debug_device_attr = \
-- 
1.9.1

Re: [Outreachy kernel] [PATCH] Staging: fbtbt: show() methods should use scnprintf()

[Greg KH]

On Tue, Sep 29, 2015 at 09:15:23PM +0200, Ksenija Stanojevic wrote:
> snprintf() may return a value that exceeds its second argument,
> show() methods should use scnprintf() instead of snprintf().
> 
> Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
> ---
>  drivers/staging/fbtft/fbtft-sysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/fbtft/fbtft-sysfs.c b/drivers/staging/fbtft/fbtft-sysfs.c
> index 8d8bd12..36a1f4e 100644
> --- a/drivers/staging/fbtft/fbtft-sysfs.c
> +++ b/drivers/staging/fbtft/fbtft-sysfs.c
> @@ -198,7 +198,7 @@ static ssize_t show_debug(struct device *device,
>  	struct fb_info *fb_info = dev_get_drvdata(device);
>  	struct fbtft_par *par = fb_info->par;
>  
> -	return snprintf(buf, PAGE_SIZE, "%lu\n", par->debug);
> +	return scnprintf(buf, PAGE_SIZE, "%lu\n", par->debug);

They should?  Why?  This shouldn't be needed at all, and really, a
simple sprintf() is all that is necessary.  I would just leave this
alone, it's fine as-is.

thanks,

greg k-h

Re: [Outreachy kernel] [PATCH] Staging: fbtbt: show() methods should use scnprintf()

[Ksenija Stanojević]

Hi,

On Tue, Sep 29, 2015 at 9:38 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Sep 29, 2015 at 09:15:23PM +0200, Ksenija Stanojevic wrote:
>> snprintf() may return a value that exceeds its second argument,
>> show() methods should use scnprintf() instead of snprintf().
>>
>> Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
>> ---
>>  drivers/staging/fbtft/fbtft-sysfs.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/fbtft/fbtft-sysfs.c b/drivers/staging/fbtft/fbtft-sysfs.c
>> index 8d8bd12..36a1f4e 100644
>> --- a/drivers/staging/fbtft/fbtft-sysfs.c
>> +++ b/drivers/staging/fbtft/fbtft-sysfs.c
>> @@ -198,7 +198,7 @@ static ssize_t show_debug(struct device *device,
>>       struct fb_info *fb_info = dev_get_drvdata(device);
>>       struct fbtft_par *par = fb_info->par;
>>
>> -     return snprintf(buf, PAGE_SIZE, "%lu\n", par->debug);
>> +     return scnprintf(buf, PAGE_SIZE, "%lu\n", par->debug);
>
> They should?  Why?  This shouldn't be needed at all, and really, a
> simple sprintf() is all that is necessary.  I would just leave this
> alone, it's fine as-is.

In kernel documentation :
https://www.kernel.org/doc/Documentation/filesystems/sysfs.txt
it states:

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()

Should I just use sprintf instead?

Thanks,
Ksenija


> thanks,
>
> greg k-h
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20150929193803.GA23361%40kroah.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [Outreachy kernel] [PATCH] Staging: fbtbt: show() methods should use scnprintf()

[Greg KH]

On Tue, Sep 29, 2015 at 09:50:58PM +0200, Ksenija Stanojević wrote:
> Hi,
> 
> On Tue, Sep 29, 2015 at 9:38 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Tue, Sep 29, 2015 at 09:15:23PM +0200, Ksenija Stanojevic wrote:
> >> snprintf() may return a value that exceeds its second argument,
> >> show() methods should use scnprintf() instead of snprintf().
> >>
> >> Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
> >> ---
> >>  drivers/staging/fbtft/fbtft-sysfs.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/staging/fbtft/fbtft-sysfs.c b/drivers/staging/fbtft/fbtft-sysfs.c
> >> index 8d8bd12..36a1f4e 100644
> >> --- a/drivers/staging/fbtft/fbtft-sysfs.c
> >> +++ b/drivers/staging/fbtft/fbtft-sysfs.c
> >> @@ -198,7 +198,7 @@ static ssize_t show_debug(struct device *device,
> >>       struct fb_info *fb_info = dev_get_drvdata(device);
> >>       struct fbtft_par *par = fb_info->par;
> >>
> >> -     return snprintf(buf, PAGE_SIZE, "%lu\n", par->debug);
> >> +     return scnprintf(buf, PAGE_SIZE, "%lu\n", par->debug);
> >
> > They should?  Why?  This shouldn't be needed at all, and really, a
> > simple sprintf() is all that is necessary.  I would just leave this
> > alone, it's fine as-is.
> 
> In kernel documentation :
> https://www.kernel.org/doc/Documentation/filesystems/sysfs.txt
> it states:
> 
> 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()
> 
> Should I just use sprintf instead?

If you are writing new code for sysfs attributes like these, yes, use
sprintf().  Don't modify existing code that is working just fine with no
errors at all by trying to convert snprintf() to scnprintf() please.

thanks,

greg k-h