linux-kernel - Re: arch/x86/include/asm/processor.h:698:16: sparse: sparse: incorrect type in initializer (different address spaces)

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: Mon, 04 Mar 2024 00:49:18 +0100
From: Thomas Gleixner <tglx@...utronix.de>
To: Uros Bizjak <ubizjak@...il.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, kernel test robot
 <lkp@...el.com>, oe-kbuild-all@...ts.linux.dev,
 linux-kernel@...r.kernel.org, Arjan van de Ven <arjan@...ux.intel.com>,
 x86@...nel.org, Luc Van Oostenryck <luc.vanoostenryck@...il.com>, Sparse
 Mailing-list <linux-sparse@...r.kernel.org>, "Paul E. McKenney"
 <paulmck@...nel.org>
Subject: Re: arch/x86/include/asm/processor.h:698:16: sparse: sparse:
 incorrect type in initializer (different address spaces)

On Sun, Mar 03 2024 at 21:24, Uros Bizjak wrote:
> On Sun, Mar 3, 2024 at 9:21 PM Uros Bizjak <ubizjak@...il.com> wrote:
>> On Sun, Mar 3, 2024 at 9:10 PM Thomas Gleixner <tglx@...utronix.de> wrote:
>> > That's so sad because it would provide us compiler based __percpu
>> > validation.
>>
>> Unfortunately, the c compiler can't strip qualifiers, so typeof() is
>> of limited use also when const and volatile qualifiers are used.
>> Perhaps some extension could be introduced to c standard to provide an
>> unqualified type, e.g. typeof_unqual().
>
> Oh, there is one in C23 [1].

Yes. I found it right after ranting.

gcc >= 14 and clang >= 16 have support for it of course only when adding
-std=c2x to the command line.

Sigh. The name space qualifiers are non standard and then the thing
which makes them more useful is hidden behind a standard.

Why can't we have useful tools?

Though the whole thing looks worthwhile:

#define verify_per_cpu_ptr(ptr)						\
do {									\
	const void __seg_gs *__vpp_verify = (typeof((ptr) + 0))NULL;    \
	(void)__vpp_verify;						\
} while (0)

#define per_cpu_ptr(ptr, cpu)						\
({									\
	verify_per_cpu_ptr(ptr);					\
	(typeof_unqual(*(ptr)) *)(uintptr_t)ptr + per_cpu_offset(cpu);	\
})

unsigned int __seg_gs test;

unsigned int foo1(unsigned int cpu)
{
	return *per_cpu_ptr(&test, cpu);
}

unsigned int foo2(unsigned int cpu)
{
	unsigned int x, *p = per_cpu_ptr(&x, cpu);

	return *p;
}

x.c:29:23: error: initializing 'const __attribute__((address_space(256))) void *' with an expression of type 'typeof ((&x) + 0)' (aka 'unsigned int *') changes address space of pointer
        unsigned int x, *p = per_cpu_ptr(&x, cpu);

That's exactly what we want. It would have caught all the long standing
and ignored __percpu sparse warnings right away.

This also simplifies all the other per cpu accessors. The most trivial
is read()

#define verify_per_cpu(variable)					\
{									\
	const unsigned int __s = sizeof(variable);			\
									\
	verify_per_cpu_ptr(&(variable));				\
	BUILD_BUG_ON(__s == 1 || __s == 2 || __s == 4 || __s == 8,	\
		     "Wrong size for per CPU variable");		\
}

#define __pcpu_read(variable)						\
({									\
	verify_per_cpu(variable);					\
	READ_ONCE(variable);						\
})

which in turn catches all the mistakes, i.e. wrong namespace and wrong
size.

I'm really tempted to implement this as an alternative to the current
pile of macro horrors. Of course this requires to figure out first what
kind of damage -std=c2x will do.

I get to that in my copious spare time some day.

Thanks,

        tglx