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:	Wed, 29 Jun 2016 09:12:07 +0200
From:	Jiri Olsa <jolsa@...hat.com>
To:	Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:	Jiri Olsa <jolsa@...nel.org>, Steven Rostedt <rostedt@...dmis.org>,
	lkml <linux-kernel@...r.kernel.org>,
	Frederic Weisbecker <fweisbec@...il.com>
Subject: Re: [RFC/PATCH] lib/vsprintf: Add support to store cpumask

On Tue, Jun 28, 2016 at 11:19:59PM +0200, Rasmus Villemoes wrote:

SNIP

> > -			save_arg(void *);
> > +			if (spec.cpumask) {
> 
> As I hinted in the other mail, I think it's better just to put the
> fmt[1]=='b' here and not change struct printf_spec.
> 
> > +				/*
> > +				 * Store entire cpumask directly to buffer
> > +				 * instead of storing just a pointer.
> > +				 */
> > +				struct cpumask *mask = va_arg(args, void *);
> > +
> > +				str = PTR_ALIGN(str, sizeof(u32));
> > +
> > +				if (str + sizeof(*mask) <= end)
> > +					cpumask_copy((struct cpumask *) str, mask);
> 
> A cpumask is an array of longs. Why is u32-alignment enough for that?
> cpumask_copy may end up compiling to a simple "*dst = *src", and even if
> this is a memcpy(), the same 4-but-possibly-not-8 byte aligned
> pointer is created below in bstr_printf which is then passed on to
> pointer() and then bitmap_* which certainly expects an unsigned long*. 

hum, the binary buffer is copied after to trace buffer which
has 32bit alignment I think, so bigger alignment breaks the
data.. also the save_arg macro does 32bit alignment for 8 bytes

anyway, I haven't checked deeply on this, was just hunting
wrong cpumask, now with Steven's changes I'm ok to drop it ;-)

thanks,
jirka