Re: [PATCH v2] drm/i915: Fix CFI violations in gt_sysfs
On 12/01/2023 16:56, Nathan Chancellor wrote:
Hi Jocelyn,
On Thu, Jan 12, 2023 at 11:08:17AM +0100, Jocelyn Falempe wrote:
This patch does also solve a kernel crash when reading
/sys/class/drm/card1/gt/gt0/* on a skylake machine:
https://bugzilla.redhat.com/show_bug.cgi?id=2154880
Interesting, I wonder what aspect of this patch fixes this because I am
not sure that is an intended consequence of this change but that is
still good to hear!
I wasn't able to find the root cause, but basically the kobj pointer
given in the sysfs callback, is not the good one on this machine, so it
either return garbage or crash.
By chance I found this patch, tried it, and it solves the issue.
For the record, this is commit a8a4f0467d70 ("drm/i915: Fix CFI
violations in gt_sysfs") in mainline.
Do you think it can be backported to stable releases ?
Conflicts are trivial on top of v6.0 at least.
I had a report from another user of this crash affecting them with kCFI
so it is on my TODO to backport it to 6.1 (6.0 just went EOL) but I am
currently out of the office until next Wednesday so I won't be able to
get to it until then (as I would like to test the backport on affected
hardware). If someone wants to beat me to it, I won't complain ;)
Thanks for taking care of it, I will wait next week then.
Cheers,
Nathan
--
Jocelyn
Re: [PATCH v2] drm/i915: Fix CFI violations in gt_sysfs
Hi Jocelyn,
On Thu, Jan 12, 2023 at 11:08:17AM +0100, Jocelyn Falempe wrote:
> This patch does also solve a kernel crash when reading
> /sys/class/drm/card1/gt/gt0/* on a skylake machine:
> https://bugzilla.redhat.com/show_bug.cgi?id=2154880
Interesting, I wonder what aspect of this patch fixes this because I am
not sure that is an intended consequence of this change but that is
still good to hear!
For the record, this is commit a8a4f0467d70 ("drm/i915: Fix CFI
violations in gt_sysfs") in mainline.
> Do you think it can be backported to stable releases ?
> Conflicts are trivial on top of v6.0 at least.
I had a report from another user of this crash affecting them with kCFI
so it is on my TODO to backport it to 6.1 (6.0 just went EOL) but I am
currently out of the office until next Wednesday so I won't be able to
get to it until then (as I would like to test the backport on affected
hardware). If someone wants to beat me to it, I won't complain ;)
Cheers,
Nathan
Re: [PATCH v2] drm/i915: Fix CFI violations in gt_sysfs
Hi,
This patch does also solve a kernel crash when reading
/sys/class/drm/card1/gt/gt0/* on a skylake machine:
https://bugzilla.redhat.com/show_bug.cgi?id=2154880
Do you think it can be backported to stable releases ?
Conflicts are trivial on top of v6.0 at least.
Thanks,
--
Jocelyn
On 13/10/2022 22:59, Nathan Chancellor wrote:
When booting with CONFIG_CFI_CLANG, there are numerous violations when
accessing the files under
/sys/devices/pci:00/:00:02.0/drm/card0/gt/gt0:
$ cd /sys/devices/pci:00/: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/pci:00/: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:
Re: [PATCH v2] drm/i915: Fix CFI violations in gt_sysfs
Hi Nathan,
pushed in drm-intel-gt-next.
Thanks!
Andi
On Thu, Oct 13, 2022 at 01:59:10PM -0700, Nathan Chancellor wrote:
> When booting with CONFIG_CFI_CLANG, there are numerous violations when
> accessing the files under
> /sys/devices/pci:00/:00:02.0/drm/card0/gt/gt0:
>
> $ cd /sys/devices/pci:00/: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/pci:00/: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
>
