linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] percpu: Remove intermediate variable in PERCPU_PTR()
@ 2024-12-19 12:18 Gal Pressman
  2024-12-19 12:30 ` Uros Bizjak
  0 siblings, 1 reply; 5+ messages in thread
From: Gal Pressman @ 2024-12-19 12:18 UTC (permalink / raw)
  To: Dennis Zhou, Tejun Heo, Christoph Lameter, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Uros Bizjak,
	Andrew Morton
  Cc: linux-mm, linux-kernel, llvm, netdev, Gal Pressman

The intermediate variable in the PERCPU_PTR() macro results in a kernel
panic on boot [1] due to a compiler bug seen when compiling the kernel
(+ KASAN) with gcc 11.3.1, but not when compiling with latest gcc
(v14.2)/clang(v18.1).

To solve it, remove the intermediate variable (which is not needed) and
keep the casting that resolves the address space checks.

[1]
  Oops: general protection fault, probably for non-canonical address 0xdffffc0000000003: 0000 [#1] SMP KASAN
  KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f]
  CPU: 0 UID: 0 PID: 547 Comm: iptables Not tainted 6.13.0-rc1_external_tested-master #1
  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
  RIP: 0010:nf_ct_netns_do_get+0x139/0x540
  Code: 03 00 00 48 81 c4 88 00 00 00 5b 5d 41 5c 41 5d 41 5e 41 5f c3 4d 8d 75 08 48 b8 00 00 00 00 00 fc ff df 4c 89 f2 48 c1 ea 03 <0f> b6 04 02 84 c0 74 08 3c 03 0f 8e 27 03 00 00 41 8b 45 08 83 c0
  RSP: 0018:ffff888116df75e8 EFLAGS: 00010207
  RAX: dffffc0000000000 RBX: 1ffff11022dbeebe RCX: ffffffff839a2382
  RDX: 0000000000000003 RSI: 0000000000000008 RDI: ffff88842ec46d10
  RBP: 0000000000000002 R08: 0000000000000000 R09: fffffbfff0b0860c
  R10: ffff888116df75e8 R11: 0000000000000001 R12: ffffffff879d6a80
  R13: 0000000000000016 R14: 000000000000001e R15: ffff888116df7908
  FS:  00007fba01646740(0000) GS:ffff88842ec00000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 000055bd901800d8 CR3: 00000001205f0003 CR4: 0000000000172eb0
  Call Trace:
   <TASK>
   ? die_addr+0x3d/0xa0
   ? exc_general_protection+0x144/0x220
   ? asm_exc_general_protection+0x22/0x30
   ? __mutex_lock+0x2c2/0x1d70
   ? nf_ct_netns_do_get+0x139/0x540
   ? nf_ct_netns_do_get+0xb5/0x540
   ? net_generic+0x1f0/0x1f0
   ? __create_object+0x5e/0x80
   xt_check_target+0x1f0/0x930
   ? textify_hooks.constprop.0+0x110/0x110
   ? pcpu_alloc_noprof+0x7cd/0xcf0
   ? xt_find_target+0x148/0x1e0
   find_check_entry.constprop.0+0x6c0/0x920
   ? get_info+0x380/0x380
   ? __virt_addr_valid+0x1df/0x3b0
   ? kasan_quarantine_put+0xe3/0x200
   ? kfree+0x13e/0x3d0
   ? translate_table+0xaf5/0x1750
   translate_table+0xbd8/0x1750
   ? ipt_unregister_table_exit+0x30/0x30
   ? __might_fault+0xbb/0x170
   do_ipt_set_ctl+0x408/0x1340
   ? nf_sockopt_find.constprop.0+0x17b/0x1f0
   ? lock_downgrade+0x680/0x680
   ? lockdep_hardirqs_on_prepare+0x284/0x400
   ? ipt_register_table+0x440/0x440
   ? bit_wait_timeout+0x160/0x160
   nf_setsockopt+0x6f/0xd0
   raw_setsockopt+0x7e/0x200
   ? raw_bind+0x590/0x590
   ? do_user_addr_fault+0x812/0xd20
   do_sock_setsockopt+0x1e2/0x3f0
   ? move_addr_to_user+0x90/0x90
   ? lock_downgrade+0x680/0x680
   __sys_setsockopt+0x9e/0x100
   __x64_sys_setsockopt+0xb9/0x150
   ? do_syscall_64+0x33/0x140
   do_syscall_64+0x6d/0x140
   entry_SYSCALL_64_after_hwframe+0x4b/0x53
  RIP: 0033:0x7fba015134ce
  Code: 0f 1f 40 00 48 8b 15 59 69 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b1 0f 1f 00 f3 0f 1e fa 49 89 ca b8 36 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 0a c3 66 0f 1f 84 00 00 00 00 00 48 8b 15 21
  RSP: 002b:00007ffd9de6f388 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
  RAX: ffffffffffffffda RBX: 000055bd9017f490 RCX: 00007fba015134ce
  RDX: 0000000000000040 RSI: 0000000000000000 RDI: 0000000000000004
  RBP: 0000000000000500 R08: 0000000000000560 R09: 0000000000000052
  R10: 000055bd901800e0 R11: 0000000000000246 R12: 000055bd90180140
  R13: 000055bd901800e0 R14: 000055bd9017f498 R15: 000055bd9017ff10
   </TASK>
  Modules linked in: xt_MASQUERADE nf_conntrack_netlink nfnetlink xt_addrtype iptable_nat nf_nat br_netfilter rpcsec_gss_krb5 auth_rpcgss oid_registry overlay zram zsmalloc mlx4_ib mlx4_en mlx4_core rpcrdma rdma_ucm ib_uverbs ib_iser libiscsi scsi_transport_iscsi fuse ib_umad rdma_cm ib_ipoib iw_cm ib_cm ib_core
  ---[ end trace 0000000000000000 ]---

Fixes: dabddd687c9e ("percpu: cast percpu pointer in PERCPU_PTR() via unsigned long")
Closes: https://lore.kernel.org/all/7590f546-4021-4602-9252-0d525de35b52@nvidia.com
Cc: Uros Bizjak <ubizjak@gmail.com>
Signed-off-by: Gal Pressman <gal@nvidia.com>
---
 include/linux/percpu-defs.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h
index 35842d1e3879..573adb643d90 100644
--- a/include/linux/percpu-defs.h
+++ b/include/linux/percpu-defs.h
@@ -222,8 +222,7 @@ do {									\
 
 #define PERCPU_PTR(__p)							\
 ({									\
-	unsigned long __pcpu_ptr = (__force unsigned long)(__p);	\
-	(typeof(*(__p)) __force __kernel *)(__pcpu_ptr);		\
+	(typeof(*(__p)) __force __kernel *)((__force unsigned long)(__p)); \
 })
 
 #ifdef CONFIG_SMP
-- 
2.40.1



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] percpu: Remove intermediate variable in PERCPU_PTR()
  2024-12-19 12:18 [PATCH] percpu: Remove intermediate variable in PERCPU_PTR() Gal Pressman
@ 2024-12-19 12:30 ` Uros Bizjak
  2024-12-19 16:02   ` Uros Bizjak
  0 siblings, 1 reply; 5+ messages in thread
From: Uros Bizjak @ 2024-12-19 12:30 UTC (permalink / raw)
  To: Gal Pressman
  Cc: Dennis Zhou, Tejun Heo, Christoph Lameter, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Andrew Morton,
	linux-mm, linux-kernel, llvm, netdev

On Thu, Dec 19, 2024 at 1:18 PM Gal Pressman <gal@nvidia.com> wrote:
>
> The intermediate variable in the PERCPU_PTR() macro results in a kernel
> panic on boot [1] due to a compiler bug seen when compiling the kernel
> (+ KASAN) with gcc 11.3.1, but not when compiling with latest gcc
> (v14.2)/clang(v18.1).
>
> To solve it, remove the intermediate variable (which is not needed) and
> keep the casting that resolves the address space checks.
>
> [1]
>   Oops: general protection fault, probably for non-canonical address 0xdffffc0000000003: 0000 [#1] SMP KASAN
>   KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f]
>   CPU: 0 UID: 0 PID: 547 Comm: iptables Not tainted 6.13.0-rc1_external_tested-master #1
>   Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
>   RIP: 0010:nf_ct_netns_do_get+0x139/0x540
>   Code: 03 00 00 48 81 c4 88 00 00 00 5b 5d 41 5c 41 5d 41 5e 41 5f c3 4d 8d 75 08 48 b8 00 00 00 00 00 fc ff df 4c 89 f2 48 c1 ea 03 <0f> b6 04 02 84 c0 74 08 3c 03 0f 8e 27 03 00 00 41 8b 45 08 83 c0
>   RSP: 0018:ffff888116df75e8 EFLAGS: 00010207
>   RAX: dffffc0000000000 RBX: 1ffff11022dbeebe RCX: ffffffff839a2382
>   RDX: 0000000000000003 RSI: 0000000000000008 RDI: ffff88842ec46d10
>   RBP: 0000000000000002 R08: 0000000000000000 R09: fffffbfff0b0860c
>   R10: ffff888116df75e8 R11: 0000000000000001 R12: ffffffff879d6a80
>   R13: 0000000000000016 R14: 000000000000001e R15: ffff888116df7908
>   FS:  00007fba01646740(0000) GS:ffff88842ec00000(0000) knlGS:0000000000000000
>   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>   CR2: 000055bd901800d8 CR3: 00000001205f0003 CR4: 0000000000172eb0
>   Call Trace:
>    <TASK>
>    ? die_addr+0x3d/0xa0
>    ? exc_general_protection+0x144/0x220
>    ? asm_exc_general_protection+0x22/0x30
>    ? __mutex_lock+0x2c2/0x1d70
>    ? nf_ct_netns_do_get+0x139/0x540
>    ? nf_ct_netns_do_get+0xb5/0x540
>    ? net_generic+0x1f0/0x1f0
>    ? __create_object+0x5e/0x80
>    xt_check_target+0x1f0/0x930
>    ? textify_hooks.constprop.0+0x110/0x110
>    ? pcpu_alloc_noprof+0x7cd/0xcf0
>    ? xt_find_target+0x148/0x1e0
>    find_check_entry.constprop.0+0x6c0/0x920
>    ? get_info+0x380/0x380
>    ? __virt_addr_valid+0x1df/0x3b0
>    ? kasan_quarantine_put+0xe3/0x200
>    ? kfree+0x13e/0x3d0
>    ? translate_table+0xaf5/0x1750
>    translate_table+0xbd8/0x1750
>    ? ipt_unregister_table_exit+0x30/0x30
>    ? __might_fault+0xbb/0x170
>    do_ipt_set_ctl+0x408/0x1340
>    ? nf_sockopt_find.constprop.0+0x17b/0x1f0
>    ? lock_downgrade+0x680/0x680
>    ? lockdep_hardirqs_on_prepare+0x284/0x400
>    ? ipt_register_table+0x440/0x440
>    ? bit_wait_timeout+0x160/0x160
>    nf_setsockopt+0x6f/0xd0
>    raw_setsockopt+0x7e/0x200
>    ? raw_bind+0x590/0x590
>    ? do_user_addr_fault+0x812/0xd20
>    do_sock_setsockopt+0x1e2/0x3f0
>    ? move_addr_to_user+0x90/0x90
>    ? lock_downgrade+0x680/0x680
>    __sys_setsockopt+0x9e/0x100
>    __x64_sys_setsockopt+0xb9/0x150
>    ? do_syscall_64+0x33/0x140
>    do_syscall_64+0x6d/0x140
>    entry_SYSCALL_64_after_hwframe+0x4b/0x53
>   RIP: 0033:0x7fba015134ce
>   Code: 0f 1f 40 00 48 8b 15 59 69 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b1 0f 1f 00 f3 0f 1e fa 49 89 ca b8 36 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 0a c3 66 0f 1f 84 00 00 00 00 00 48 8b 15 21
>   RSP: 002b:00007ffd9de6f388 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
>   RAX: ffffffffffffffda RBX: 000055bd9017f490 RCX: 00007fba015134ce
>   RDX: 0000000000000040 RSI: 0000000000000000 RDI: 0000000000000004
>   RBP: 0000000000000500 R08: 0000000000000560 R09: 0000000000000052
>   R10: 000055bd901800e0 R11: 0000000000000246 R12: 000055bd90180140
>   R13: 000055bd901800e0 R14: 000055bd9017f498 R15: 000055bd9017ff10
>    </TASK>
>   Modules linked in: xt_MASQUERADE nf_conntrack_netlink nfnetlink xt_addrtype iptable_nat nf_nat br_netfilter rpcsec_gss_krb5 auth_rpcgss oid_registry overlay zram zsmalloc mlx4_ib mlx4_en mlx4_core rpcrdma rdma_ucm ib_uverbs ib_iser libiscsi scsi_transport_iscsi fuse ib_umad rdma_cm ib_ipoib iw_cm ib_cm ib_core
>   ---[ end trace 0000000000000000 ]---
>
> Fixes: dabddd687c9e ("percpu: cast percpu pointer in PERCPU_PTR() via unsigned long")
> Closes: https://lore.kernel.org/all/7590f546-4021-4602-9252-0d525de35b52@nvidia.com
> Cc: Uros Bizjak <ubizjak@gmail.com>
> Signed-off-by: Gal Pressman <gal@nvidia.com>

Reviewed-by: Uros Bizjak <ubizjak@gmail.com>

> ---
>  include/linux/percpu-defs.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h
> index 35842d1e3879..573adb643d90 100644
> --- a/include/linux/percpu-defs.h
> +++ b/include/linux/percpu-defs.h
> @@ -222,8 +222,7 @@ do {                                                                        \
>
>  #define PERCPU_PTR(__p)                                                        \
>  ({                                                                     \
> -       unsigned long __pcpu_ptr = (__force unsigned long)(__p);        \
> -       (typeof(*(__p)) __force __kernel *)(__pcpu_ptr);                \
> +       (typeof(*(__p)) __force __kernel *)((__force unsigned long)(__p)); \
>  })
>
>  #ifdef CONFIG_SMP
> --
> 2.40.1
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] percpu: Remove intermediate variable in PERCPU_PTR()
  2024-12-19 12:30 ` Uros Bizjak
@ 2024-12-19 16:02   ` Uros Bizjak
  2024-12-19 17:03     ` Uros Bizjak
  0 siblings, 1 reply; 5+ messages in thread
From: Uros Bizjak @ 2024-12-19 16:02 UTC (permalink / raw)
  To: Gal Pressman
  Cc: Dennis Zhou, Tejun Heo, Christoph Lameter, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Andrew Morton,
	linux-mm, linux-kernel, llvm, netdev

On Thu, Dec 19, 2024 at 1:30 PM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> On Thu, Dec 19, 2024 at 1:18 PM Gal Pressman <gal@nvidia.com> wrote:
> >
> > The intermediate variable in the PERCPU_PTR() macro results in a kernel
> > panic on boot [1] due to a compiler bug seen when compiling the kernel
> > (+ KASAN) with gcc 11.3.1, but not when compiling with latest gcc
> > (v14.2)/clang(v18.1).
> >
> > To solve it, remove the intermediate variable (which is not needed) and
> > keep the casting that resolves the address space checks.
> >
> > [1]
> >   Oops: general protection fault, probably for non-canonical address 0xdffffc0000000003: 0000 [#1] SMP KASAN
> >   KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f]
> >   CPU: 0 UID: 0 PID: 547 Comm: iptables Not tainted 6.13.0-rc1_external_tested-master #1
> >   Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
> >   RIP: 0010:nf_ct_netns_do_get+0x139/0x540
> >   Code: 03 00 00 48 81 c4 88 00 00 00 5b 5d 41 5c 41 5d 41 5e 41 5f c3 4d 8d 75 08 48 b8 00 00 00 00 00 fc ff df 4c 89 f2 48 c1 ea 03 <0f> b6 04 02 84 c0 74 08 3c 03 0f 8e 27 03 00 00 41 8b 45 08 83 c0
> >   RSP: 0018:ffff888116df75e8 EFLAGS: 00010207
> >   RAX: dffffc0000000000 RBX: 1ffff11022dbeebe RCX: ffffffff839a2382
> >   RDX: 0000000000000003 RSI: 0000000000000008 RDI: ffff88842ec46d10
> >   RBP: 0000000000000002 R08: 0000000000000000 R09: fffffbfff0b0860c
> >   R10: ffff888116df75e8 R11: 0000000000000001 R12: ffffffff879d6a80
> >   R13: 0000000000000016 R14: 000000000000001e R15: ffff888116df7908
> >   FS:  00007fba01646740(0000) GS:ffff88842ec00000(0000) knlGS:0000000000000000
> >   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >   CR2: 000055bd901800d8 CR3: 00000001205f0003 CR4: 0000000000172eb0
> >   Call Trace:
> >    <TASK>
> >    ? die_addr+0x3d/0xa0
> >    ? exc_general_protection+0x144/0x220
> >    ? asm_exc_general_protection+0x22/0x30
> >    ? __mutex_lock+0x2c2/0x1d70
> >    ? nf_ct_netns_do_get+0x139/0x540
> >    ? nf_ct_netns_do_get+0xb5/0x540
> >    ? net_generic+0x1f0/0x1f0
> >    ? __create_object+0x5e/0x80
> >    xt_check_target+0x1f0/0x930
> >    ? textify_hooks.constprop.0+0x110/0x110
> >    ? pcpu_alloc_noprof+0x7cd/0xcf0
> >    ? xt_find_target+0x148/0x1e0
> >    find_check_entry.constprop.0+0x6c0/0x920
> >    ? get_info+0x380/0x380
> >    ? __virt_addr_valid+0x1df/0x3b0
> >    ? kasan_quarantine_put+0xe3/0x200
> >    ? kfree+0x13e/0x3d0
> >    ? translate_table+0xaf5/0x1750
> >    translate_table+0xbd8/0x1750
> >    ? ipt_unregister_table_exit+0x30/0x30
> >    ? __might_fault+0xbb/0x170
> >    do_ipt_set_ctl+0x408/0x1340
> >    ? nf_sockopt_find.constprop.0+0x17b/0x1f0
> >    ? lock_downgrade+0x680/0x680
> >    ? lockdep_hardirqs_on_prepare+0x284/0x400
> >    ? ipt_register_table+0x440/0x440
> >    ? bit_wait_timeout+0x160/0x160
> >    nf_setsockopt+0x6f/0xd0
> >    raw_setsockopt+0x7e/0x200
> >    ? raw_bind+0x590/0x590
> >    ? do_user_addr_fault+0x812/0xd20
> >    do_sock_setsockopt+0x1e2/0x3f0
> >    ? move_addr_to_user+0x90/0x90
> >    ? lock_downgrade+0x680/0x680
> >    __sys_setsockopt+0x9e/0x100
> >    __x64_sys_setsockopt+0xb9/0x150
> >    ? do_syscall_64+0x33/0x140
> >    do_syscall_64+0x6d/0x140
> >    entry_SYSCALL_64_after_hwframe+0x4b/0x53
> >   RIP: 0033:0x7fba015134ce
> >   Code: 0f 1f 40 00 48 8b 15 59 69 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b1 0f 1f 00 f3 0f 1e fa 49 89 ca b8 36 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 0a c3 66 0f 1f 84 00 00 00 00 00 48 8b 15 21
> >   RSP: 002b:00007ffd9de6f388 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
> >   RAX: ffffffffffffffda RBX: 000055bd9017f490 RCX: 00007fba015134ce
> >   RDX: 0000000000000040 RSI: 0000000000000000 RDI: 0000000000000004
> >   RBP: 0000000000000500 R08: 0000000000000560 R09: 0000000000000052
> >   R10: 000055bd901800e0 R11: 0000000000000246 R12: 000055bd90180140
> >   R13: 000055bd901800e0 R14: 000055bd9017f498 R15: 000055bd9017ff10
> >    </TASK>
> >   Modules linked in: xt_MASQUERADE nf_conntrack_netlink nfnetlink xt_addrtype iptable_nat nf_nat br_netfilter rpcsec_gss_krb5 auth_rpcgss oid_registry overlay zram zsmalloc mlx4_ib mlx4_en mlx4_core rpcrdma rdma_ucm ib_uverbs ib_iser libiscsi scsi_transport_iscsi fuse ib_umad rdma_cm ib_ipoib iw_cm ib_cm ib_core
> >   ---[ end trace 0000000000000000 ]---
> >
> > Fixes: dabddd687c9e ("percpu: cast percpu pointer in PERCPU_PTR() via unsigned long")
> > Closes: https://lore.kernel.org/all/7590f546-4021-4602-9252-0d525de35b52@nvidia.com
> > Cc: Uros Bizjak <ubizjak@gmail.com>
> > Signed-off-by: Gal Pressman <gal@nvidia.com>
>
> Reviewed-by: Uros Bizjak <ubizjak@gmail.com>
>
> > ---
> >  include/linux/percpu-defs.h | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h
> > index 35842d1e3879..573adb643d90 100644
> > --- a/include/linux/percpu-defs.h
> > +++ b/include/linux/percpu-defs.h
> > @@ -222,8 +222,7 @@ do {                                                                        \
> >
> >  #define PERCPU_PTR(__p)                                                        \
> >  ({                                                                     \
> > -       unsigned long __pcpu_ptr = (__force unsigned long)(__p);        \
> > -       (typeof(*(__p)) __force __kernel *)(__pcpu_ptr);                \
> > +       (typeof(*(__p)) __force __kernel *)((__force unsigned long)(__p)); \
> >  })

Actually, you can simplify the above a bit by writing it as:

#define PERCPU_PTR(__p)                            \
    ((typeof(*(__p)) __force __kernel *)(__force unsigned long)(__p)) \

Uros.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] percpu: Remove intermediate variable in PERCPU_PTR()
  2024-12-19 16:02   ` Uros Bizjak
@ 2024-12-19 17:03     ` Uros Bizjak
  2024-12-19 22:51       ` Andrew Morton
  0 siblings, 1 reply; 5+ messages in thread
From: Uros Bizjak @ 2024-12-19 17:03 UTC (permalink / raw)
  To: Gal Pressman
  Cc: Dennis Zhou, Tejun Heo, Christoph Lameter, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Andrew Morton,
	linux-mm, linux-kernel, llvm, netdev

[-- Attachment #1: Type: text/plain, Size: 1886 bytes --]

On Thu, Dec 19, 2024 at 5:02 PM Uros Bizjak <ubizjak@gmail.com> wrote:

> > > The intermediate variable in the PERCPU_PTR() macro results in a kernel
> > > panic on boot [1] due to a compiler bug seen when compiling the kernel
> > > (+ KASAN) with gcc 11.3.1, but not when compiling with latest gcc
> > > (v14.2)/clang(v18.1).
> > >
> > > To solve it, remove the intermediate variable (which is not needed) and
> > > keep the casting that resolves the address space checks.

[...]

> > >  include/linux/percpu-defs.h | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h
> > > index 35842d1e3879..573adb643d90 100644
> > > --- a/include/linux/percpu-defs.h
> > > +++ b/include/linux/percpu-defs.h
> > > @@ -222,8 +222,7 @@ do {                                                                        \
> > >
> > >  #define PERCPU_PTR(__p)                                                        \
> > >  ({                                                                     \
> > > -       unsigned long __pcpu_ptr = (__force unsigned long)(__p);        \
> > > -       (typeof(*(__p)) __force __kernel *)(__pcpu_ptr);                \
> > > +       (typeof(*(__p)) __force __kernel *)((__force unsigned long)(__p)); \
> > >  })
>
> Actually, you can simplify the above a bit by writing it as:
>
> #define PERCPU_PTR(__p)                            \
>     ((typeof(*(__p)) __force __kernel *)(__force unsigned long)(__p)) \

Andrew, please find attached a substitute patch "[PATCH 4/6] percpu:
Use TYPEOF_UNQUAL() in *_cpu_ptr() accessors" for your MM tree
relative to the above hotfix. The whole patch series (+ hotfix) has
been re-tested against the current mainline defconfig (+ KASAN),
compiled once with gcc-11.4.1 and once with gcc-14.2.1.

Uros.

[-- Attachment #2: 0004-percpu-Use-TYPEOF_UNQUAL-in-_cpu_ptr-accessors.patch --]
[-- Type: text/x-patch, Size: 2284 bytes --]

From fc933c978446d6aae977b540f00cfcfbfc65b755 Mon Sep 17 00:00:00 2001
From: Uros Bizjak <ubizjak@gmail.com>
Date: Tue, 26 Nov 2024 16:46:42 +0100
Subject: [PATCH 4/6] percpu: Use TYPEOF_UNQUAL() in *_cpu_ptr() accessors

Use TYPEOF_UNQUAL() macro to declare the return type of *_cpu_ptr()
accessors in the generic named address space to avoid access to
data from pointer to non-enclosed address space type of errors.

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Acked-by: Nadav Amit <nadav.amit@gmail.com>
Acked-by: Christoph Lameter <cl@linux.com>
Cc: Dennis Zhou <dennis@kernel.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 arch/x86/include/asm/percpu.h | 8 ++++++--
 include/linux/percpu-defs.h   | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 666e4137b09f..27f668660abe 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -73,10 +73,14 @@
 	unsigned long tcp_ptr__ = raw_cpu_read_long(this_cpu_off);	\
 									\
 	tcp_ptr__ += (__force unsigned long)(_ptr);			\
-	(typeof(*(_ptr)) __kernel __force *)tcp_ptr__;			\
+	(TYPEOF_UNQUAL(*(_ptr)) __force __kernel *)tcp_ptr__;		\
 })
 #else
-#define arch_raw_cpu_ptr(_ptr) ({ BUILD_BUG(); (typeof(_ptr))0; })
+#define arch_raw_cpu_ptr(_ptr)						\
+({									\
+	BUILD_BUG();							\
+	(TYPEOF_UNQUAL(*(_ptr)) __force __kernel *)0;			\
+})
 #endif
 
 #define PER_CPU_VAR(var)	%__percpu_seg:(var)__percpu_rel
diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h
index 4fde93334ac3..8a7c8d2d570d 100644
--- a/include/linux/percpu-defs.h
+++ b/include/linux/percpu-defs.h
@@ -221,7 +221,7 @@ do {									\
 } while (0)
 
 #define PERCPU_PTR(__p)							\
-	((typeof(*(__p)) __force __kernel *)(__force unsigned long)(__p)) \
+	((TYPEOF_UNQUAL(*(__p)) __force __kernel *)(__force unsigned long)(__p)) \
 
 #ifdef CONFIG_SMP
 
-- 
2.42.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] percpu: Remove intermediate variable in PERCPU_PTR()
  2024-12-19 17:03     ` Uros Bizjak
@ 2024-12-19 22:51       ` Andrew Morton
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2024-12-19 22:51 UTC (permalink / raw)
  To: Uros Bizjak
  Cc: Gal Pressman, Dennis Zhou, Tejun Heo, Christoph Lameter,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	linux-mm, linux-kernel, llvm, netdev

On Thu, 19 Dec 2024 18:03:47 +0100 Uros Bizjak <ubizjak@gmail.com> wrote:

> > Actually, you can simplify the above a bit by writing it as:
> >
> > #define PERCPU_PTR(__p)                            \
> >     ((typeof(*(__p)) __force __kernel *)(__force unsigned long)(__p)) \
> 
> Andrew, please find attached a substitute patch "[PATCH 4/6] percpu:
> Use TYPEOF_UNQUAL() in *_cpu_ptr() accessors" for your MM tree
> relative to the above hotfix. The whole patch series (+ hotfix) has
> been re-tested against the current mainline defconfig (+ KASAN),
> compiled once with gcc-11.4.1 and once with gcc-14.2.1.

Updated, thanks.

>  #define PERCPU_PTR(__p)							\
> -	((typeof(*(__p)) __force __kernel *)(__force unsigned long)(__p)) \
> +	((TYPEOF_UNQUAL(*(__p)) __force __kernel *)(__force unsigned long)(__p)) \
>

I removed that final " \".



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-12-19 22:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-12-19 12:18 [PATCH] percpu: Remove intermediate variable in PERCPU_PTR() Gal Pressman
2024-12-19 12:30 ` Uros Bizjak
2024-12-19 16:02   ` Uros Bizjak
2024-12-19 17:03     ` Uros Bizjak
2024-12-19 22:51       ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox