Re: [PATCH] afs: fix tracepoint string placement with built-in AFS
From: Andrew Morton <akpm@linux-foundation.org>
Date: 2021-06-14 23:47:55
On Fri, 28 May 2021 01:04:46 +0300 Alexey Dobriyan [off-list ref] wrote:
I was adding custom tracepoint to the kernel, grabbed full F34 kernel .config, disabled modules and booted whole shebang as VM kernel. Then did perf record -a -e ... It crashed: general protection fault, probably for non-canonical address 0x435f5346592e4243: 0000 [#1] SMP PTI CPU: 1 PID: 842 Comm: cat Not tainted 5.12.6+ #26 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1.fc33 04/01/2014 RIP: 0010:t_show+0x22/0xd0 Then reproducer was narrowed to # cat /sys/kernel/tracing/printk_formats Original F34 kernel with modules didn't crash. So I started to disable options and after disabling AFS everything started working again. The root cause is that AFS was placing char arrays content into a section full of _pointers_ to strings with predictable consequences. Non canonical address 435f5346592e4243 is "CB.YFS_" which came from CM_NAME macro. The fix is to create char array and pointer to it separatedly. Steps to reproduce: CONFIG_AFS=y CONFIG_TRACING=y # cat /sys/kernel/tracing/printk_formats
I'll add
Fixes: 8e8d7f13b6d5a9 ("afs: Add some tracepoints")
although Andi's d2abfa86ff373 gets in the way a bit.
quoted hunk ↗ jump to hunk
--- a/fs/afs/cmservice.c +++ b/fs/afs/cmservice.c@@ -30,8 +30,9 @@ static void SRXAFSCB_TellMeAboutYourself(struct work_struct *); static int afs_deliver_yfs_cb_callback(struct afs_call *); #define CM_NAME(name) \ - char afs_SRXCB##name##_name[] __tracepoint_string = \ - "CB." #name + const char afs_SRXCB##name##_name[] = "CB." #name; \ + static const char *_afs_SRXCB##name##_name __tracepoint_string =\ + afs_SRXCB##name##_name
Should/can afs_SRXCB##name##_name[] be static? __tracepoint_string is very rarely used. I wonder if there's much point in it existing? kernel/rcu/tree.h does static char rcu_name[] = RCU_NAME_RAW; static const char *tp_rcu_varname __used __tracepoint_string = rcu_name; which is asking the compiler to place a copy of these into each compilation unit which includes tree.h, which probably isn't what was intended.