linux-kernel - Re: [RFC/PATCH] lib/vsprintf: Add support to store cpumask

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 28 Jun 2016 15:56:14 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:	Jiri Olsa <jolsa@...nel.org>, Lai Jiangshan <laijs@...fujitsu.com>,
	lkml <linux-kernel@...r.kernel.org>,
	Frederic Weisbecker <fweisbec@...il.com>
Subject: Re: [RFC/PATCH] lib/vsprintf: Add support to store cpumask

On Tue, 28 Jun 2016 21:27:29 +0200
Rasmus Villemoes <linux@...musvillemoes.dk> wrote:


> > I probably should make a trace_printk() that doesn't default to the
> > binary print, to handle things like this.
> >
> >   trace_printk_ptr()?
> >
> > Or even just see if I can find a way that detects this in the fmt
> > string. Hmm, that probably can't be done at compile time :-/  
> 
> Well, not with gcc itself, but it wouldn't be too hard to make smatch
> complain loudly if trace_printk is used on a format string with any %p
> extension (directing people to use trace_printk_ptr()) - the format
> parsing (and type checking) is already there.

But people don't usually run "make smatch" on debug kernels.
trace_printk() currently is not allowed to be used in the kernel. When
it is used, a big ugly banner is posted on boot up saying that the
kernel is in "debug mode" and is "unstable" (even though it isn't) just
to scare people enough to never compile with a trace_printk() in their
code. If they need a permanent trace_printk() then they need to use
tracepoints.

> 
> >> Cc: Steven Rostedt <rostedt@...dmis.org>
> >> Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> >> ---
> >>  lib/vsprintf.c | 41 ++++++++++++++++++++++++++++++++++++-----
> >>  1 file changed, 36 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> >> index 0967771d8f7f..f21d68e1b5fc 100644
> >> --- a/lib/vsprintf.c
> >> +++ b/lib/vsprintf.c
> >> @@ -388,7 +388,8 @@ enum format_type {
> >>  struct printf_spec {
> >>  	unsigned int	type:8;		/* format_type enum */
> >>  	signed int	field_width:24;	/* width of output field */
> >> -	unsigned int	flags:8;	/* flags to number() */
> >> +	unsigned int	flags:7;	/* flags to number() */
> >> +	unsigned int	cpumask:1;	/* pointer to cpumask flag */  
> >
> > Why not just add this as another flag? There's one left. I'm not sure
> > gcc does nice things with bit fields not a multiple of 8.  
> 
> I really don't think we should pollute the common printf code with this
> stuff, partly because of the code generation issues, but also: what
> should we do the next time someone decides to handle a %p extension more
> correctly in vbin_printf?


Perhaps we should add an WARN_ON is these are used in vbin_printf().

I'll go and make a trace_printk_ptr() patch.

-- Steve