linux-kernel - Re: [PATCH] fix __percpu annotation in asm-generic

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, 27 Nov 2019 23:54:32 +0100
From:   Luc Van Oostenryck <luc.vanoostenryck@...il.com>
To:     Dennis Zhou <dennis@...nel.org>
Cc:     Christopher Lameter <cl@...ux.com>,
        Ben Dooks <ben.dooks@...ethink.co.uk>,
        linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org,
        Tejun Heo <tj@...nel.org>, Nicholas Piggin <npiggin@...il.com>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH] fix __percpu annotation in asm-generic

On Wed, Nov 27, 2019 at 12:53:50PM -0500, Dennis Zhou wrote:
> On Wed, Nov 27, 2019 at 03:55:19PM +0000, Christopher Lameter wrote:
> > On Tue, 26 Nov 2019, Luc Van Oostenryck wrote:
> > 
> > > So, fix the declaration of the 'pcp' variable to its correct type:
> > > the plain (non-percpu) pointer corresponding to its address.
> > > Same for raw_cpu_generic_xchg(), raw_cpu_generic_cmpxchg() &
> > > raw_cpu_generic_cmpxchg_double().
> > 
> > Acked-by: Christoph Lameter <cl@...ux.com>
> > 
> > Maybe a better fix is to come up with a typeof_strip_percu() or so
> > macro for all the places where this needs to be done?
> 
> I like the idea of typeof_strip_percpu(). Luc do you mind spinning v2
> with a macro for this instead?

I wouldn't mind at all (I already thought about doing something
like this several times) but:
1) it would strip any address space, not just __percpu, so:
   it would need to be combined with __verify_pcpu_ptr() or,
   * a better name should be used,
   * it should be defined in a generic header, any idea where?
   * I fear it would be abused to escape sloppy typing
     (like __force and casts are already often used).
2) while I find the current solution:
	typeof(T) __kernel __force *ptr = ...;

   quite readable and relatively easy to understand, the solution
   I have for doing it with a macro (that behaves like typeof) is,
   IMO, much much less readable and understandable:
	#define typeof_strip_percpu(T) \
		typeof(({ typeof(T) __kernel __force __fakename; __fakename; }))

	typeof_strip_perpcu(T) * ptr = ...;

So, if you insist I can do it but I would really prefer not.


Best regards,
-- Luc