Hi Nathan, On Thu, Jan 4, 2024 at 2:59 PM Nathan Chancellor <nathan@xxxxxxxxxx> wrote: > > When booting a kernel with CONFIG_CFI_CLANG, there is a CFI failure when > accessing any of the values under > /sys/devices/system/cpu/intel_uncore_frequency/package_00_die_00: > > $ cat /sys/devices/system/cpu/intel_uncore_frequency/package_00_die_00/max_freq_khz > fish: Job 1, 'cat /sys/devices/system/cpu/int…' terminated by signal SIGSEGV (Address boundary error) > > $ sudo dmesg &| grep 'CFI failure' > [ 170.953925] CFI failure at kobj_attr_show+0x19/0x30 (target: show_max_freq_khz+0x0/0xc0 [intel_uncore_frequency_common]; expected type: 0xd34078c5 > > The sysfs callback functions such as show_domain_id() are written as if > they are going to be called by dev_attr_show() but as the above message > shows, they are instead called by kobj_attr_show(). kCFI checks that the > destination of an indirect jump has the exact same type as the prototype > of the function pointer it is called through and fails when they do not. > > These callbacks are called through kobj_attr_show() because > uncore_root_kobj was initialized with kobject_create_and_add(), which > means uncore_root_kobj has a ->sysfs_ops of kobj_sysfs_ops from > kobject_create(), which uses kobj_attr_show() as its ->show() value. > > The only reason there has not been a more noticeable problem until this > point is that 'struct kobj_attribute' and 'struct device_attribute' have > the same layout, so getting the callback from container_of() works the > same with either value. > > Change all the callbacks and their uses to be compatible with > kobj_attr_show() and kobj_attr_store(), which resolves the kCFI failure > and allows the sysfs files to work properly. > > Closes: https://github.com/ClangBuiltLinux/linux/issues/1974 > Fixes: ae7b2ce57851 ("platform/x86/intel/uncore-freq: Use sysfs API to create attributes") > Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx> > --- > I think I got the fixes tag right. I only started seeing this because of > commit 27f2b08735c9 ("platform/x86: intel-uncore-freq: Add additional > client processors"), which allows this driver to load on my i7-11700 > test system but I think the commit in the Fixes tag incorrectly changes > the types of the callbacks. > --- > .../uncore-frequency/uncore-frequency-common.c | 82 +++++++++++----------- > .../uncore-frequency/uncore-frequency-common.h | 32 ++++----- > 2 files changed, 57 insertions(+), 57 deletions(-) Looks good to me. Thanks for the patch! Reviewed-by: Sami Tolvanen <samitolvanen@xxxxxxxxxx> Sami
