On Sun, Jan 15, 2023 at 08:35:52PM -0700, Nathan Chancellor wrote:
> commit a8a4f0467d706fc22d286dfa973946e5944b793c upstream.
>
> When booting with CONFIG_CFI_CLANG, there are numerous violations when
> accessing the files under
> /sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt/gt0:
>
> $ cd /sys/devices/pci0000:00/0000:00:02.0/drm/card0/gt/gt0
>
> $ grep . *
> id:0
> punit_req_freq_mhz:350
> rc6_enable:1
> rc6_residency_ms:214934
> rps_act_freq_mhz:1300
> rps_boost_freq_mhz:1300
> rps_cur_freq_mhz:350
> rps_max_freq_mhz:1300
> rps_min_freq_mhz:350
> rps_RP0_freq_mhz:1300
> rps_RP1_freq_mhz:350
> rps_RPn_freq_mhz:350
> throttle_reason_pl1:0
> throttle_reason_pl2:0
> throttle_reason_pl4:0
> throttle_reason_prochot:0
> throttle_reason_ratl:0
> throttle_reason_status:0
> throttle_reason_thermal:0
> throttle_reason_vr_tdc:0
> throttle_reason_vr_thermalert:0
>
> $ sudo dmesg &| grep "CFI failure at"
> [ 214.595903] CFI failure at kobj_attr_show+0x19/0x30 (target: id_show+0x0/0x70 [i915]; expected type: 0xc527b809)
> [ 214.596064] CFI failure at kobj_attr_show+0x19/0x30 (target: punit_req_freq_mhz_show+0x0/0x40 [i915]; expected type: 0xc527b809)
> [ 214.596407] CFI failure at kobj_attr_show+0x19/0x30 (target: rc6_enable_show+0x0/0x40 [i915]; expected type: 0xc527b809)
> [ 214.596528] CFI failure at kobj_attr_show+0x19/0x30 (target: rc6_residency_ms_show+0x0/0x270 [i915]; expected type: 0xc527b809)
> [ 214.596682] CFI failure at kobj_attr_show+0x19/0x30 (target: act_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.596792] CFI failure at kobj_attr_show+0x19/0x30 (target: boost_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.596893] CFI failure at kobj_attr_show+0x19/0x30 (target: cur_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.596996] CFI failure at kobj_attr_show+0x19/0x30 (target: max_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597099] CFI failure at kobj_attr_show+0x19/0x30 (target: min_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597198] CFI failure at kobj_attr_show+0x19/0x30 (target: RP0_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597301] CFI failure at kobj_attr_show+0x19/0x30 (target: RP1_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597405] CFI failure at kobj_attr_show+0x19/0x30 (target: RPn_freq_mhz_show+0x0/0xe0 [i915]; expected type: 0xc527b809)
> [ 214.597538] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.597701] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.597836] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.597952] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598071] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598177] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598307] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598439] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
> [ 214.598542] CFI failure at kobj_attr_show+0x19/0x30 (target: throttle_reason_bool_show+0x0/0x50 [i915]; expected type: 0xc527b809)
>
> With kCFI, indirect calls are validated against their expected type
> versus actual type and failures occur when the two types do not match.
> The ultimate issue is that these sysfs functions are expecting to be
> called via dev_attr_show() but they may also be called via
> kobj_attr_show(), as certain files are created under two different
> kobjects that have two different sysfs_ops in intel_gt_sysfs_register(),
> hence the warnings above. When accessing the gt_ files under
> /sys/devices/pci0000:00/0000:00:02.0/drm/card0, which are using the same
> sysfs functions, there are no violations, meaning the functions are
> being called with the proper type.
>
> To make everything work properly, adjust certain functions to match the
> type of the ->show() and ->store() members in 'struct kobj_attribute'.
> Add a macro to generate functions for that can be called via both
> dev_attr_{show,store}() or kobj_attr_{show,store}() so that they can be
> called through both kobject locations without violating kCFI and adjust
> the attribute groups to account for this.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1716
> Reviewed-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
> Reviewed-by: Andrzej Hajda <andrzej.hajda@xxxxxxxxx>
> Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>
> Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
> Link: https://patchwork.freedesktop.org/patch/msgid/20221013205909.1282545-1-nathan@xxxxxxxxxx
> Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> ---
>
> Hi Greg and Sasha,
>
> I received a report from a user of kCFI in 6.1 that the violation fixed
> by this patch is visible there (which is expected, since this was only
> merged in 6.2-rc1). This has been boot tested on real hardware and
> confirmed to fix that crash.
>
> Additionally, Jocelyn reports that this patch also fixes a separate
> crash:
>
> https://lore.kernel.org/4dcf830e-62a5-837b-7590-ac5395f84c14@xxxxxxxxxx/
>
> The patch is a little on the larger side of things but it should be
> pretty safe (the i915 folks can override me if they feel this is out of
> place).
>
Seems sane, now queued up, thanks.
greg k-h
