linux-kernel - Re: [PATCH 3/3] percpu: Cast percpu pointer in PERCPU

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]

Message-ID: <bf189e58-710f-49c2-a419-e3d3e71576ca@nvidia.com>
Date: Wed, 18 Dec 2024 10:46:13 +0200
From: Gal Pressman <gal@...dia.com>
To: Uros Bizjak <ubizjak@...il.com>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
 Dennis Zhou <dennis@...nel.org>, Tejun Heo <tj@...nel.org>,
 Christoph Lameter <cl@...ux.com>, Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 3/3] percpu: Cast percpu pointer in PERCPU_PTR() via
 unsigned long

On 18/12/2024 10:40, Uros Bizjak wrote:
> On Wed, Dec 18, 2024 at 8:54 AM Gal Pressman <gal@...dia.com> wrote:
>>
>> On 21/10/2024 11:07, Uros Bizjak wrote:
>>> Cast pointer from percpu address space to generic (kernel) address
>>> space in PERCPU_PTR() macro via unsigned long intermediate cast [1].
>>> This intermediate cast is also required to avoid build failure
>>> when GCC's strict named address space checks for x86 targets [2]
>>> are enabled.
>>>
>>> Found by GCC's named address space checks.
>>>
>>> [1] https://sparse.docs.kernel.org/en/latest/annotations.html#address-space-name
>>> [2] https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html#x86-Named-Address-Spaces
>>>
>>> Signed-off-by: Uros Bizjak <ubizjak@...il.com>
>>> Cc: Dennis Zhou <dennis@...nel.org>
>>> Cc: Tejun Heo <tj@...nel.org>
>>> Cc: Christoph Lameter <cl@...ux.com>
>>> Cc: Andrew Morton <akpm@...ux-foundation.org>
>>> ---
>>>  include/linux/percpu-defs.h | 5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h
>>> index e1cf7982424f..35842d1e3879 100644
>>> --- a/include/linux/percpu-defs.h
>>> +++ b/include/linux/percpu-defs.h
>>> @@ -221,7 +221,10 @@ do {                                                                     \
>>>  } while (0)
>>>
>>>  #define PERCPU_PTR(__p)                                                      \
>>> -     (typeof(*(__p)) __force __kernel *)(__p);
>>> +({                                                                   \
>>> +     unsigned long __pcpu_ptr = (__force unsigned long)(__p);        \
>>> +     (typeof(*(__p)) __force __kernel *)(__pcpu_ptr);                \
>>> +})
>>>
>>>  #ifdef CONFIG_SMP
>>>
>>
>> Hello Uros,
>>
>> We've encountered a kernel panic on boot [1] bisected to this patch.
>> I believe the patch is fine and the issue is caused by a compiler bug.
>> The panic reproduces when compiling the kernel with gcc 11.3.1, but does
>> not reproduce with latest gcc/clang.
>>
>> I have a patch that workarounds the issue by ditching the intermediate
>> variable and does the casting in a single line. Will that be enough to
>> solve the sparse/build issues?
> 
> Yes, single line like:
> 
> (typeof(*(__p)) __force __kernel *)(__force unsigned long)(__pcpu_ptr);
> 
> should be OK.
> 
> Thanks,
> Uros.

Awesome, I'll submit a patch unless I hear any objections.
Thanks Uros!