A crash was reported in amd-sfh related to hid core initialization before SFH initialization has run.
``` amdtp_hid_request+0x36/0x50 [amd_sfh 2e3095779aada9fdb1764f08ca578ccb14e41fe4] sensor_hub_get_feature+0xad/0x170 [hid_sensor_hub d6157999c9d260a1bfa6f27d4a0dc2c3e2c5654e] hid_sensor_parse_common_attributes+0x217/0x310 [hid_sensor_iio_common 07a7935272aa9c7a28193b574580b3e953a64ec4] hid_gyro_3d_probe+0x7f/0x2e0 [hid_sensor_gyro_3d 9f2eb51294a1f0c0315b365f335617cbaef01eab] platform_probe+0x44/0xa0 really_probe+0x19e/0x3e0 ```
Ensure that sensors have been set up before calling into amd_sfh_get_report() or amd_sfh_set_report().
Cc: stable@vger.kernel.org Cc: Linux regression tracking (Thorsten Leemhuis) regressions@leemhuis.info Fixes: 7bcfdab3f0c6 ("HID: amd_sfh: if no sensors are enabled, clean up") Reported-by: Haochen Tong linux@hexchain.org Link: https://lore.kernel.org/all/3250319.ancTxkQ2z5@zen/T/ Signed-off-by: Mario Limonciello mario.limonciello@amd.com --- drivers/hid/amd-sfh-hid/amd_sfh_client.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c index d9b7b01900b5..88f3d913eaa1 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c @@ -25,6 +25,9 @@ void amd_sfh_set_report(struct hid_device *hid, int report_id, struct amdtp_cl_data *cli_data = hid_data->cli_data; int i;
+ if (!cli_data->is_any_sensor_enabled) + return; + for (i = 0; i < cli_data->num_hid_devices; i++) { if (cli_data->hid_sensor_hubs[i] == hid) { cli_data->cur_hid_dev = i; @@ -41,6 +44,9 @@ int amd_sfh_get_report(struct hid_device *hid, int report_id, int report_type) struct request_list *req_list = &cli_data->req_list; int i;
+ if (!cli_data->is_any_sensor_enabled) + return -ENODEV; + for (i = 0; i < cli_data->num_hid_devices; i++) { if (cli_data->hid_sensor_hubs[i] == hid) { struct request_list *new = kzalloc(sizeof(*new), GFP_KERNEL);
On 6/21/2023 1:09 AM, Mario Limonciello wrote:
A crash was reported in amd-sfh related to hid core initialization before SFH initialization has run.
amdtp_hid_request+0x36/0x50 [amd_sfh 2e3095779aada9fdb1764f08ca578ccb14e41fe4] sensor_hub_get_feature+0xad/0x170 [hid_sensor_hub d6157999c9d260a1bfa6f27d4a0dc2c3e2c5654e] hid_sensor_parse_common_attributes+0x217/0x310 [hid_sensor_iio_common 07a7935272aa9c7a28193b574580b3e953a64ec4] hid_gyro_3d_probe+0x7f/0x2e0 [hid_sensor_gyro_3d 9f2eb51294a1f0c0315b365f335617cbaef01eab] platform_probe+0x44/0xa0 really_probe+0x19e/0x3e0
Ensure that sensors have been set up before calling into amd_sfh_get_report() or amd_sfh_set_report().
Cc: stable@vger.kernel.org Cc: Linux regression tracking (Thorsten Leemhuis) regressions@leemhuis.info Fixes: 7bcfdab3f0c6 ("HID: amd_sfh: if no sensors are enabled, clean up") Reported-by: Haochen Tong linux@hexchain.org Link: https://lore.kernel.org/all/3250319.ancTxkQ2z5@zen/T/ Signed-off-by: Mario Limonciello mario.limonciello@amd.com
drivers/hid/amd-sfh-hid/amd_sfh_client.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c index d9b7b01900b5..88f3d913eaa1 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c @@ -25,6 +25,9 @@ void amd_sfh_set_report(struct hid_device *hid, int report_id, struct amdtp_cl_data *cli_data = hid_data->cli_data; int i;
- if (!cli_data->is_any_sensor_enabled)
return;
can we check below patch series which solves this issue by initializing HID only if is_any_sensor_enabled https://lore.kernel.org/all/nycvar.YFH.7.76.2305231559000.29760@cbobk.fhfr.p...
for (i = 0; i < cli_data->num_hid_devices; i++) { if (cli_data->hid_sensor_hubs[i] == hid) { cli_data->cur_hid_dev = i; @@ -41,6 +44,9 @@ int amd_sfh_get_report(struct hid_device *hid, int report_id, int report_type) struct request_list *req_list = &cli_data->req_list; int i;
- if (!cli_data->is_any_sensor_enabled)
return -ENODEV;
can we check below patch series which solves this issue by initializing HID only if is_any_sensor_enabled. https://lore.kernel.org/all/nycvar.YFH.7.76.2305231559000.29760@cbobk.fhfr.p...
Thanks, -- Basavaraj
for (i = 0; i < cli_data->num_hid_devices; i++) { if (cli_data->hid_sensor_hubs[i] == hid) { struct request_list *new = kzalloc(sizeof(*new), GFP_KERNEL);
[Public]
can we check below patch series which solves this issue by initializing HID only if is_any_sensor_enabled. https://lore.kernel.org/all/nycvar.YFH.7.76.2305231559000.29760@cbobk. fhfr.pm/
The original reporter won't be able to test it because they've upgraded their firmware and SFH is disabled in the new firmware.
But yeah it seems plausible this series could help. If it comes back up again we should point anyone affected to this series.
Thanks!
On 05.07.23 18:07, Limonciello, Mario wrote:
[Public]
can we check below patch series which solves this issue by initializing HID only if is_any_sensor_enabled. https://lore.kernel.org/all/nycvar.YFH.7.76.2305231559000.29760@cbobk. fhfr.pm/
The original reporter won't be able to test it because they've upgraded their firmware and SFH is disabled in the new firmware.
But yeah it seems plausible this series could help. If it comes back up again we should point anyone affected to this series.
Thanks!
Hmmm. So this won't be fixed in 6.3.y. and 6.4.y then, as none of those patches afaics looks like they will be picked up by the stable team?
Hmmm. That doesn't completely feel right to me, unless we consider the problem Haochen Ton ran into an extremely unlikely bug (reminder: only a few of those that encounter a problem will report it). Do we? If not: is backporting that patch-set to 6.4.y an option once this was in mainline for a while without causing trouble?
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page.
On 7/6/2023 06:28, Thorsten Leemhuis wrote:
On 05.07.23 18:07, Limonciello, Mario wrote:
[Public]
can we check below patch series which solves this issue by initializing HID only if is_any_sensor_enabled. https://lore.kernel.org/all/nycvar.YFH.7.76.2305231559000.29760@cbobk. fhfr.pm/
The original reporter won't be able to test it because they've upgraded their firmware and SFH is disabled in the new firmware.
But yeah it seems plausible this series could help. If it comes back up again we should point anyone affected to this series.
Thanks!
Hmmm. So this won't be fixed in 6.3.y. and 6.4.y then, as none of those patches afaics looks like they will be picked up by the stable team?
Hmmm. That doesn't completely feel right to me, unless we consider the problem Haochen Ton ran into an extremely unlikely bug (reminder: only a few of those that encounter a problem will report it). Do we? If not: is backporting that patch-set to 6.4.y an option once this was in mainline for a while without causing trouble?
So the problem that was found is the following set of circumstances: 1) System that doesn't have sensors connected to SFH 2) System that has SFH enabled in BIOS
As this is already fixed in BIOS update for affected laptop that disabled SFH and the vendor publishes the BIOS to LVFS, I think it's unlikely to crop back up on this model.
If anything it might crop up on a different model that meets what I said above. If that happens, I suggest we ask any potential future reporters to test that series.
We can always backport it to remaining stable channels if it comes back.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page.
linux-stable-mirror@lists.linaro.org