* [Cocci] [PATCH] coccinelle: api: add device_attr_show script
@ 2020-06-15 13:02 Denis Efremov
2020-06-17 20:27 ` Julia Lawall
0 siblings, 1 reply; 6+ messages in thread
From: Denis Efremov @ 2020-06-15 13:02 UTC (permalink / raw)
To: Julia Lawall; +Cc: linux-kernel, cocci
According to the documentation[1] show() methods of device attributes
should return the number of bytes printed into the buffer. This is
the return value of scnprintf(). show() must not use snprintf()
when formatting the value to be returned to user space. snprintf()
returns the length the resulting string would be, assuming it all
fit into the destination array[2]. scnprintf() return the length of
the string actually created in buf. If one can guarantee that an
overflow will never happen sprintf() can be used otherwise scnprintf().
[1] Documentation/filesystems/sysfs.txt
[2] "snprintf() confusion" https://lwn.net/Articles/69419/
Signed-off-by: Denis Efremov <efremov@linux.com>
---
scripts/coccinelle/api/device_attr_show.cocci | 55 +++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100644 scripts/coccinelle/api/device_attr_show.cocci
diff --git a/scripts/coccinelle/api/device_attr_show.cocci b/scripts/coccinelle/api/device_attr_show.cocci
new file mode 100644
index 000000000000..d8ec4bb8ac41
--- /dev/null
+++ b/scripts/coccinelle/api/device_attr_show.cocci
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// 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().
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers
+//
+
+virtual report
+virtual org
+virtual context
+virtual patch
+
+@r depends on !patch@
+identifier show, dev, attr, buf;
+position p;
+@@
+
+ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ <...
+* return snprintf@p(...);
+ ...>
+}
+
+@rp depends on patch@
+identifier show, dev, attr, buf;
+@@
+
+ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+ <...
+ return
+- snprintf
++ scnprintf
+ (...);
+ ...>
+}
+
+@script: python depends on report@
+p << r.p;
+@@
+
+coccilib.report.print_report(p[0], "WARNING: use scnprintf or sprintf")
+
+@script: python depends on org@
+p << r.p;
+@@
+
+coccilib.org.print_todo(p[0], "WARNING: use scnprintf or sprintf")
--
2.26.2
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Cocci] [PATCH] coccinelle: api: add device_attr_show script
2020-06-15 13:02 [Cocci] [PATCH] coccinelle: api: add device_attr_show script Denis Efremov
@ 2020-06-17 20:27 ` Julia Lawall
2020-06-17 20:41 ` Denis Efremov
0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2020-06-17 20:27 UTC (permalink / raw)
To: Denis Efremov; +Cc: linux-kernel, cocci
On Mon, 15 Jun 2020, Denis Efremov wrote:
> According to the documentation[1] show() methods of device attributes
> should return the number of bytes printed into the buffer. This is
> the return value of scnprintf(). show() must not use snprintf()
> when formatting the value to be returned to user space. snprintf()
> returns the length the resulting string would be, assuming it all
> fit into the destination array[2]. scnprintf() return the length of
> the string actually created in buf. If one can guarantee that an
> overflow will never happen sprintf() can be used otherwise scnprintf().
The semantic patch looks fine. Do you have any accepted patches from
this?
julia
>
> [1] Documentation/filesystems/sysfs.txt
> [2] "snprintf() confusion" https://lwn.net/Articles/69419/
>
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
> scripts/coccinelle/api/device_attr_show.cocci | 55 +++++++++++++++++++
> 1 file changed, 55 insertions(+)
> create mode 100644 scripts/coccinelle/api/device_attr_show.cocci
>
> diff --git a/scripts/coccinelle/api/device_attr_show.cocci b/scripts/coccinelle/api/device_attr_show.cocci
> new file mode 100644
> index 000000000000..d8ec4bb8ac41
> --- /dev/null
> +++ b/scripts/coccinelle/api/device_attr_show.cocci
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +///
> +/// 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().
> +///
> +// Confidence: High
> +// Copyright: (C) 2020 Denis Efremov ISPRAS
> +// Options: --no-includes --include-headers
> +//
> +
> +virtual report
> +virtual org
> +virtual context
> +virtual patch
> +
> +@r depends on !patch@
> +identifier show, dev, attr, buf;
> +position p;
> +@@
> +
> +ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + <...
> +* return snprintf@p(...);
> + ...>
> +}
> +
> +@rp depends on patch@
> +identifier show, dev, attr, buf;
> +@@
> +
> +ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + <...
> + return
> +- snprintf
> ++ scnprintf
> + (...);
> + ...>
> +}
> +
> +@script: python depends on report@
> +p << r.p;
> +@@
> +
> +coccilib.report.print_report(p[0], "WARNING: use scnprintf or sprintf")
> +
> +@script: python depends on org@
> +p << r.p;
> +@@
> +
> +coccilib.org.print_todo(p[0], "WARNING: use scnprintf or sprintf")
> --
> 2.26.2
>
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Cocci] [PATCH] coccinelle: api: add device_attr_show script
2020-06-17 20:27 ` Julia Lawall
@ 2020-06-17 20:41 ` Denis Efremov
2020-06-17 20:46 ` Julia Lawall
0 siblings, 1 reply; 6+ messages in thread
From: Denis Efremov @ 2020-06-17 20:41 UTC (permalink / raw)
To: Julia Lawall; +Cc: linux-kernel, cocci
On 6/17/20 11:27 PM, Julia Lawall wrote:
>
>
> On Mon, 15 Jun 2020, Denis Efremov wrote:
>
>> According to the documentation[1] show() methods of device attributes
>> should return the number of bytes printed into the buffer. This is
>> the return value of scnprintf(). show() must not use snprintf()
>> when formatting the value to be returned to user space. snprintf()
>> returns the length the resulting string would be, assuming it all
>> fit into the destination array[2]. scnprintf() return the length of
>> the string actually created in buf. If one can guarantee that an
>> overflow will never happen sprintf() can be used otherwise scnprintf().
>
> The semantic patch looks fine. Do you have any accepted patches from
> this?
It's not my patches, but:
3f9f8daad342 cpuidle: sysfs: Fix the overlap for showing available governors
117e2cb3eeee sparc: use scnprintf() in show_pciobppath_attr() in vio.c
03a1b56f501e sparc: use scnprintf() in show_pciobppath_attr() in pci.c
3dee04262898 iio: tsl2772: Use scnprintf() for avoiding potential buffer overflow
dbdd24eaac4e edd: Use scnprintf() for avoiding potential buffer overflow
abdd9feb45ed btrfs: sysfs: Use scnprintf() instead of snprintf()
f21431f2de33 thermal: int340x_thermal: Use scnprintf() for avoiding potential buffer overflow
40501c70e3f0 s390/zcrypt: replace snprintf/sprintf with scnprintf
eb3e064b8dd1 s390/zcrypt: Use scnprintf() for avoiding potential buffer overflow
06b522d6de9d video: uvesafb: Use scnprintf() for avoiding potential buffer overflow
bf1b615ad97e video: omapfb: Use scnprintf() for avoiding potential buffer overflow
b40e288bfb53 platform/x86: sony-laptop: Use scnprintf() for avoiding potential buffer overflow
ef21e1750158 ALSA: Use scnprintf() instead of snprintf() for show
and many more
Thanks,
Denis
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Cocci] [PATCH] coccinelle: api: add device_attr_show script
2020-06-17 20:41 ` Denis Efremov
@ 2020-06-17 20:46 ` Julia Lawall
0 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2020-06-17 20:46 UTC (permalink / raw)
To: Denis Efremov; +Cc: linux-kernel, cocci
On Wed, 17 Jun 2020, Denis Efremov wrote:
>
>
> On 6/17/20 11:27 PM, Julia Lawall wrote:
> >
> >
> > On Mon, 15 Jun 2020, Denis Efremov wrote:
> >
> >> According to the documentation[1] show() methods of device attributes
> >> should return the number of bytes printed into the buffer. This is
> >> the return value of scnprintf(). show() must not use snprintf()
> >> when formatting the value to be returned to user space. snprintf()
> >> returns the length the resulting string would be, assuming it all
> >> fit into the destination array[2]. scnprintf() return the length of
> >> the string actually created in buf. If one can guarantee that an
> >> overflow will never happen sprintf() can be used otherwise scnprintf().
> >
> > The semantic patch looks fine. Do you have any accepted patches from
> > this?
>
> It's not my patches, but:
>
> 3f9f8daad342 cpuidle: sysfs: Fix the overlap for showing available governors
> 117e2cb3eeee sparc: use scnprintf() in show_pciobppath_attr() in vio.c
> 03a1b56f501e sparc: use scnprintf() in show_pciobppath_attr() in pci.c
> 3dee04262898 iio: tsl2772: Use scnprintf() for avoiding potential buffer overflow
> dbdd24eaac4e edd: Use scnprintf() for avoiding potential buffer overflow
> abdd9feb45ed btrfs: sysfs: Use scnprintf() instead of snprintf()
> f21431f2de33 thermal: int340x_thermal: Use scnprintf() for avoiding potential buffer overflow
> 40501c70e3f0 s390/zcrypt: replace snprintf/sprintf with scnprintf
> eb3e064b8dd1 s390/zcrypt: Use scnprintf() for avoiding potential buffer overflow
> 06b522d6de9d video: uvesafb: Use scnprintf() for avoiding potential buffer overflow
> bf1b615ad97e video: omapfb: Use scnprintf() for avoiding potential buffer overflow
> b40e288bfb53 platform/x86: sony-laptop: Use scnprintf() for avoiding potential buffer overflow
> ef21e1750158 ALSA: Use scnprintf() instead of snprintf() for show
Thanks.
julia
>
> and many more
>
> Thanks,
> Denis
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Cocci] [PATCH] coccinelle: api: add device_attr_show script
@ 2020-06-15 14:04 Markus Elfring
2020-06-15 15:43 ` Julia Lawall
0 siblings, 1 reply; 6+ messages in thread
From: Markus Elfring @ 2020-06-15 14:04 UTC (permalink / raw)
To: Denis Efremov, Coccinelle, Gilles Muller, Julia Lawall,
Masahiro Yamada, Michal Marek, Nicolas Palix
Cc: kernel-janitors, linux-kernel
> +// Confidence: High
Would you like to add any suggestion for a possible patch message?
…
> +virtual report
> +virtual org
> +virtual context
> +virtual patch
+virtual report, org, context, patch
Is such a SmPL code variant more succinct?
…
> +ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + <...
> +* return snprintf@p(...);
> + ...>
> +}
I suggest to reconsider the selection of the SmPL nest construct.
https://github.com/coccinelle/coccinelle/blob/e06b9156dfa02a28cf3cbf0913a10513f3d163ab/docs/manual/cocci_syntax.tex#L783
Can the construct “<+... … ...+>” become relevant here?
Would you like to consider any further software design consequences
around the safe application of the asterisk functionality in rules
for the semantic patch language?
Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Cocci] [PATCH] coccinelle: api: add device_attr_show script
2020-06-15 14:04 Markus Elfring
@ 2020-06-15 15:43 ` Julia Lawall
0 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2020-06-15 15:43 UTC (permalink / raw)
To: Markus Elfring
Cc: Michal Marek, Gilles Muller, kernel-janitors, Nicolas Palix,
linux-kernel, Coccinelle
[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]
On Mon, 15 Jun 2020, Markus Elfring wrote:
> > +// Confidence: High
>
> Would you like to add any suggestion for a possible patch message?
>
>
> …
> > +virtual report
> > +virtual org
> > +virtual context
> > +virtual patch
>
> +virtual report, org, context, patch
>
> Is such a SmPL code variant more succinct?
This doens't matter.
>
>
> …
> > +ssize_t show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > + <...
> > +* return snprintf@p(...);
> > + ...>
> > +}
>
> I suggest to reconsider the selection of the SmPL nest construct.
> https://github.com/coccinelle/coccinelle/blob/e06b9156dfa02a28cf3cbf0913a10513f3d163ab/docs/manual/cocci_syntax.tex#L783
>
> Can the construct “<+... … ...+>” become relevant here?
<... ...> is fine if the only thing that will be used afterwards is what
is inside the <... ...>
julia
>
>
> Would you like to consider any further software design consequences
> around the safe application of the asterisk functionality in rules
> for the semantic patch language?
>
> Regards,
> Markus
>
[-- Attachment #2: Type: text/plain, Size: 136 bytes --]
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-17 20:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-15 13:02 [Cocci] [PATCH] coccinelle: api: add device_attr_show script Denis Efremov
2020-06-17 20:27 ` Julia Lawall
2020-06-17 20:41 ` Denis Efremov
2020-06-17 20:46 ` Julia Lawall
2020-06-15 14:04 Markus Elfring
2020-06-15 15:43 ` Julia Lawall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).