* [PATCH 2/2] proc: use set_puts() at /proc/*/wchan
@ 2018-02-17 7:20 Alexey Dobriyan
2018-02-17 14:06 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Dobriyan @ 2018-02-17 7:20 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
fs/proc/base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -396,7 +396,7 @@ static int proc_pid_wchan(struct seq_file *m, struct pid_namespace *ns,
wchan = get_wchan(task);
if (wchan && !lookup_symbol_name(wchan, symname)) {
- seq_printf(m, "%s", symname);
+ seq_puts(m, symname);
return 0;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] proc: use set_puts() at /proc/*/wchan
2018-02-17 7:20 [PATCH 2/2] proc: use set_puts() at /proc/*/wchan Alexey Dobriyan
@ 2018-02-17 14:06 ` Andy Shevchenko
2018-02-17 15:35 ` Alexey Dobriyan
2018-02-21 0:02 ` Andrew Morton
0 siblings, 2 replies; 6+ messages in thread
From: Andy Shevchenko @ 2018-02-17 14:06 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: Andrew Morton, Linux Kernel Mailing List
On Sat, Feb 17, 2018 at 9:20 AM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> - seq_printf(m, "%s", symname);
> + seq_puts(m, symname);
While this might have no security concerns, the pattern might be
brainlessly used by some janitors and there would have security
implications.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] proc: use set_puts() at /proc/*/wchan
2018-02-17 14:06 ` Andy Shevchenko
@ 2018-02-17 15:35 ` Alexey Dobriyan
2018-02-21 0:02 ` Andrew Morton
1 sibling, 0 replies; 6+ messages in thread
From: Alexey Dobriyan @ 2018-02-17 15:35 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Andrew Morton, Linux Kernel Mailing List
On Sat, Feb 17, 2018 at 04:06:42PM +0200, Andy Shevchenko wrote:
> On Sat, Feb 17, 2018 at 9:20 AM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> > - seq_printf(m, "%s", symname);
> > + seq_puts(m, symname);
>
> While this might have no security concerns, the pattern might be
> brainlessly used by some janitors and there would have security
> implications.
Unless there is some kind of preprocessor which seamlessly converts one
to another I'd continue converting.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] proc: use set_puts() at /proc/*/wchan
2018-02-17 14:06 ` Andy Shevchenko
2018-02-17 15:35 ` Alexey Dobriyan
@ 2018-02-21 0:02 ` Andrew Morton
2018-02-21 8:57 ` Rasmus Villemoes
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2018-02-21 0:02 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Alexey Dobriyan, Linux Kernel Mailing List
On Sat, 17 Feb 2018 16:06:42 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Sat, Feb 17, 2018 at 9:20 AM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
>
>
> > - seq_printf(m, "%s", symname);
> > + seq_puts(m, symname);
>
> While this might have no security concerns, the pattern might be
> brainlessly used by some janitors and there would have security
> implications.
And I'd like to see a changelog, please. One which explains why
`symname' cannot have a %s (etc) in it, and never will.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] proc: use set_puts() at /proc/*/wchan
2018-02-21 0:02 ` Andrew Morton
@ 2018-02-21 8:57 ` Rasmus Villemoes
2018-02-21 20:27 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Rasmus Villemoes @ 2018-02-21 8:57 UTC (permalink / raw)
To: Andrew Morton, Andy Shevchenko; +Cc: Alexey Dobriyan, Linux Kernel Mailing List
On 2018-02-21 01:02, Andrew Morton wrote:
> On Sat, 17 Feb 2018 16:06:42 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>
>> On Sat, Feb 17, 2018 at 9:20 AM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>>> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
>>
>>
>>> - seq_printf(m, "%s", symname);
>>> + seq_puts(m, symname);
>>
>> While this might have no security concerns, the pattern might be
>> brainlessly used by some janitors and there would have security
>> implications.
>
> And I'd like to see a changelog, please. One which explains why
> `symname' cannot have a %s (etc) in it, and never will.
OK, since #youtoo: It doesn't _matter_ if symname is "%pHAHAHA %fooled
you <unicode for evil grin emoji>", seq_puts does not interpret it at
all. There are _never_ security implications with the above replacement.
Sure, seq_printf(m, symname) would be bad, but that's not what is being
done.
AFAICT, this should always lead to slightly smaller code (one less
parameter passed) and in all likelyhood also slightly faster (no format
interpretation, no slow char-by-char handling by the string() function
etc.). So the only case where I'd think this should not necessarily be
done would be in a long sequence of seq_printf, where only one or two
could be replaced by seq_puts/seq_putc.
Rasmus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] proc: use set_puts() at /proc/*/wchan
2018-02-21 8:57 ` Rasmus Villemoes
@ 2018-02-21 20:27 ` Andrew Morton
0 siblings, 0 replies; 6+ messages in thread
From: Andrew Morton @ 2018-02-21 20:27 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Andy Shevchenko, Alexey Dobriyan, Linux Kernel Mailing List
On Wed, 21 Feb 2018 09:57:49 +0100 Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
> On 2018-02-21 01:02, Andrew Morton wrote:
> > On Sat, 17 Feb 2018 16:06:42 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> >
> >> On Sat, Feb 17, 2018 at 9:20 AM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> >>> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> >>
> >>
> >>> - seq_printf(m, "%s", symname);
> >>> + seq_puts(m, symname);
> >>
> >> While this might have no security concerns, the pattern might be
> >> brainlessly used by some janitors and there would have security
> >> implications.
> >
> > And I'd like to see a changelog, please. One which explains why
> > `symname' cannot have a %s (etc) in it, and never will.
>
> OK, since #youtoo: It doesn't _matter_ if symname is "%pHAHAHA %fooled
> you <unicode for evil grin emoji>", seq_puts does not interpret it at
> all. There are _never_ security implications with the above replacement.
> Sure, seq_printf(m, symname) would be bad, but that's not what is being
> done.
doh, OK, sorry. RTFP, Andrew.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-02-21 20:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-17 7:20 [PATCH 2/2] proc: use set_puts() at /proc/*/wchan Alexey Dobriyan
2018-02-17 14:06 ` Andy Shevchenko
2018-02-17 15:35 ` Alexey Dobriyan
2018-02-21 0:02 ` Andrew Morton
2018-02-21 8:57 ` Rasmus Villemoes
2018-02-21 20:27 ` Andrew Morton
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).