All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	John Ogness <john.ogness@linutronix.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	Steven Rostedt <rostedt@goodmis.org>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH printk 2/3] printk: move dictionary keys to dev_printk_info
Date: Fri, 18 Sep 2020 14:13:42 +0200	[thread overview]
Message-ID: <20200918121342.GD14605@alley> (raw)
In-Reply-To: <316cf147-0ae2-1c88-7375-07aedc58bd15@rasmusvillemoes.dk>

On Fri 2020-09-18 08:16:37, Rasmus Villemoes wrote:
> On 17/09/2020 15.16, John Ogness wrote:
> 
> >  	if (dev->class)
> >  		subsys = dev->class->name;
> >  	else if (dev->bus)
> >  		subsys = dev->bus->name;
> >  	else
> > -		return 0;
> > +		return;
> >  
> > -	pos += snprintf(hdr + pos, hdrlen - pos, "SUBSYSTEM=%s", subsys);
> > -	if (pos >= hdrlen)
> > -		goto overflow;
> > +	snprintf(dev_info->subsystem, sizeof(dev_info->subsystem), subsys);
> 
> It's unlikely that subsys would contain a %, but this will be yet
> another place to spend brain cycles ignoring if doing static analysis.
> So can we not do this. Either of strXcpy() for X=s,l will do the same
> thing, and likely faster.

Good point! Better be on the safe size in a generic printk() API.

Well, I am afraid that this would be only small drop in a huge lake.
class->name and bus->name seems to be passed to %s in so many
*print*() calls all over the kernel code.

IMHO, this is not the right place to prevent the problem. Dangerous
names must be prevented when a new bus, class, device is added.

Best Rergards,
Petr

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: Petr Mladek <pmladek@suse.com>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: John Ogness <john.ogness@linutronix.de>,
	Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH printk 2/3] printk: move dictionary keys to dev_printk_info
Date: Fri, 18 Sep 2020 14:13:42 +0200	[thread overview]
Message-ID: <20200918121342.GD14605@alley> (raw)
In-Reply-To: <316cf147-0ae2-1c88-7375-07aedc58bd15@rasmusvillemoes.dk>

On Fri 2020-09-18 08:16:37, Rasmus Villemoes wrote:
> On 17/09/2020 15.16, John Ogness wrote:
> 
> >  	if (dev->class)
> >  		subsys = dev->class->name;
> >  	else if (dev->bus)
> >  		subsys = dev->bus->name;
> >  	else
> > -		return 0;
> > +		return;
> >  
> > -	pos += snprintf(hdr + pos, hdrlen - pos, "SUBSYSTEM=%s", subsys);
> > -	if (pos >= hdrlen)
> > -		goto overflow;
> > +	snprintf(dev_info->subsystem, sizeof(dev_info->subsystem), subsys);
> 
> It's unlikely that subsys would contain a %, but this will be yet
> another place to spend brain cycles ignoring if doing static analysis.
> So can we not do this. Either of strXcpy() for X=s,l will do the same
> thing, and likely faster.

Good point! Better be on the safe size in a generic printk() API.

Well, I am afraid that this would be only small drop in a huge lake.
class->name and bus->name seems to be passed to %s in so many
*print*() calls all over the kernel code.

IMHO, this is not the right place to prevent the problem. Dangerous
names must be prevented when a new bus, class, device is added.

Best Rergards,
Petr

  reply	other threads:[~2020-09-18 12:13 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-17 13:16 [PATCH printk 0/3] printk: move dictionaries to meta data John Ogness
2020-09-17 13:16 ` John Ogness
2020-09-17 13:16 ` [PATCH printk 1/3] printk: move printk_info into separate array John Ogness
2020-09-17 13:16   ` John Ogness
2020-09-18 10:06   ` Petr Mladek
2020-09-18 10:06     ` Petr Mladek
2020-09-18 11:30     ` John Ogness
2020-09-18 11:30       ` John Ogness
2020-09-17 13:16 ` [PATCH printk 2/3] printk: move dictionary keys to dev_printk_info John Ogness
2020-09-17 13:16   ` John Ogness
2020-09-18  6:16   ` Rasmus Villemoes
2020-09-18  6:16     ` Rasmus Villemoes
2020-09-18 12:13     ` Petr Mladek [this message]
2020-09-18 12:13       ` Petr Mladek
2020-09-18 12:32       ` Rasmus Villemoes
2020-09-18 12:32         ` Rasmus Villemoes
2020-09-18 14:24         ` Petr Mladek
2020-09-18 14:24           ` Petr Mladek
2020-09-18 11:39   ` Petr Mladek
2020-09-18 11:39     ` Petr Mladek
2020-09-17 13:16 ` [PATCH printk 3/3] printk: remove dict ring John Ogness
2020-09-17 13:16   ` John Ogness
2020-09-17 17:59   ` Linus Torvalds
2020-09-17 17:59     ` Linus Torvalds
2020-09-18 14:56   ` Petr Mladek
2020-09-18 14:56     ` Petr Mladek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200918121342.GD14605@alley \
    --to=pmladek@suse.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=john.ogness@linutronix.de \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.