[PATCH 1/3] HID: nvidia-shield: Remove led_classdev_unregister in thunderstrike_create
From: Rahul Rameshbabu
Date: Mon Aug 07 2023 - 12:36:59 EST
Avoid calling thunderstrike_led_set_brightness from thunderstrike_create
when led_classdev_unregister is called. led_classdev_unregister was called
from thunderstrike_create in the error path. Calling
thunderstrike_led_set_brightness in this situation is unsafe.
Fixes: f88af60e74a5 ("HID: nvidia-shield: Support LED functionality for Thunderstrike")
Signed-off-by: Rahul Rameshbabu <rrameshbabu@xxxxxxxxxx>
---
Notes:
Discussion:
An alternative approach that could be used here is setting the
LED_RETAIN_AT_SHUTDOWN flag. To me, this felt like a less appropriate
solution since in other contexts in the driver, calling
led_classdev_unregister where it then tries to set the led to the LED_OFF
state is safe.
Example backtrace of problem when led_classdev_unregister is called from
thunderstrike_create.
[ +0.000061] thermal_sys: Thermal zone name (thunderstrike_0_battery) too long, should be under 20 chars
[ +0.000096] shield 0005:0955:7214.001B: Failed to register Thunderstrike battery device
[ +0.000001] shield 0005:0955:7214.001B: Failed to create Thunderstrike power supply instance
[ +0.000024] shield 0005:0955:7214.001B: Failed to create SHIELD device
[ +0.000003] shield: probe of 0005:0955:7214.001B failed with error -22
[ +0.121671] BUG: unable to handle page fault for address: 000000046474e550
[ +0.000009] #PF: supervisor read access in kernel mode
[ +0.000003] #PF: error_code(0x0000) - not-present page
[ +0.000003] PGD 0 P4D 0
[ +0.000005] Oops: 0000 [#1] PREEMPT SMP NOPTI
[ +0.000004] CPU: 14 PID: 36436 Comm: kworker/14:3 Tainted: P O 6.4.7 #1-NixOS
[ +0.000005] Hardware name: Dell Inc. Precision 5760/0WP4FK, BIOS 1.16.1 11/22/2022
[ +0.000002] Workqueue: events thunderstrike_hostcmd_req_work_handler [hid_nvidia_shield]
[ +0.000017] RIP: 0010:thunderstrike_hostcmd_req_work_handler+0x1b3/0x390 [hid_nvidia_shield]
[ +0.000010] Code: 09 00 00 00 41 b8 01 00 00 00 48 c7 45 08 00 00 00 00 48 c7 45 17 00 00 00 00 66 41 89 04 24 48 8b 53 98 48 8b bb 90 fd ff ff <0f> b6 32 e8 b5 5e 99 fa 85 c0 0f 88 9d 01 00 00 0f b7 05 cc 05 02
[ +0.000003] RSP: 0018:ffffa43e8f66fe78 EFLAGS: 00010207
[ +0.000004] RAX: 0000000000000704 RBX: ffff93edf8498a98 RCX: 0000000000000021
[ +0.000003] RDX: 000000046474e550 RSI: 0000000000000206 RDI: 00000000000002d8
[ +0.000003] RBP: ffff93eca5485e6a R08: 0000000000000001 R09: 0000000000000009
[ +0.000002] R10: 0000000000000001 R11: 0000000000000000 R12: ffff93eca5485e68
[ +0.000002] R13: 0000000000000000 R14: ffff93eca8c34540 R15: ffff93edf8498aa0
[ +0.000003] FS: 0000000000000000(0000) GS:ffff93f3ef780000(0000) knlGS:0000000000000000
[ +0.000002] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ +0.000003] CR2: 000000046474e550 CR3: 000000027d820003 CR4: 0000000000f70ee0
[ +0.000003] PKRU: 55555554
[ +0.000001] Call Trace:
[ +0.000004] <TASK>
[ +0.000005] ? __die+0x23/0x70
[ +0.000009] ? page_fault_oops+0x17d/0x4b0
[ +0.000005] ? lock_timer_base+0x61/0x80
[ +0.000005] ? exc_page_fault+0x6d/0x150
[ +0.000008] ? asm_exc_page_fault+0x26/0x30
[ +0.000012] ? thunderstrike_hostcmd_req_work_handler+0x1b3/0x390 [hid_nvidia_shield]
[ +0.000008] ? thunderstrike_hostcmd_req_work_handler+0x13b/0x390 [hid_nvidia_shield]
[ +0.000009] process_one_work+0x1c5/0x3c0
[ +0.000005] worker_thread+0x51/0x390
[ +0.000004] ? __pfx_worker_thread+0x10/0x10
[ +0.000003] kthread+0xe5/0x120
[ +0.000005] ? __pfx_kthread+0x10/0x10
[ +0.000004] ret_from_fork+0x29/0x50
[ +0.000008] </TASK>
[ +0.000002] Modules linked in: hid_nvidia_shield(O) hidp rfcomm snd_seq_dummy snd_hrtimer snd_seq nf_conntrack_netlink xfrm_user xfrm_algo xt_addrtype ccm cmac algif_hash algif_skcipher af_alg xt_CHECKSUM xt_MASQUERADE af_packet ipt_REJECT nf_reject_ipv4 nft_chain_nat bnep msr xt_conntrack ip6t_rpfilter ipt_rpfilter xt_pkttype xt_LOG nf_log_syslog xt_tcpudp nft_compat nf_tables nfnetlink sch_fq_codel hid_sensor_custom_intel_hinge hid_sensor_als hid_sensor_trigger industrialio_triggered_buffer kfifo_buf hid_sensor_iio_common industrialio hid_sensor_custom hid_sensor_hub intel_ishtp_hid snd_ctl_led snd_soc_sof_sdw hid_multitouch snd_soc_intel_hda_dsp_common snd_sof_probes snd_soc_intel_sof_maxim_common snd_soc_rt711 snd_soc_rt715 snd_soc_rt1308_sdw regmap_sdw snd_soc_dmic snd_sof_pci_intel_tgl snd_sof_intel_hda_common snd_soc_hdac_hda soundwire_intel soundwire_cadence snd_sof_intel_hda_mlink snd_sof_intel_hda snd_sof_pci snd_sof_xtensa_dsp snd_sof snd_sof_utils snd_hda_ext_core snd_soc_acpi_intel_match snd_soc_acpi
[ +0.000074] soundwire_generic_allocation soundwire_bus snd_soc_core snd_hda_codec_hdmi snd_compress ac97_bus snd_pcm_dmaengine iwlmvm mac80211 dell_rbtn ptp pps_core libarc4 i2c_designware_platform i2c_designware_core cmdlinepart x86_pkg_temp_thermal intel_powerclamp ee1004 spi_nor iTCO_wdt mei_hdcp mei_wdt intel_pmc_bxt mei_pxp watchdog mtd r8153_ecm nls_iso8859_1 snd_hda_intel cdc_ether nls_cp437 coretemp pmt_telemetry snd_intel_dspcfg usbnet vfat intel_rapl_msr dell_laptop snd_intel_sdw_acpi crc32_pclmul pmt_class ledtrig_audio fat polyval_clmulni iwlwifi snd_usb_audio btusb dell_wmi polyval_generic snd_hda_codec btrtl gf128mul uvcvideo ghash_clmulni_intel hci_uart btbcm processor_thermal_device_pci_legacy snd_usbmidi_lib nvidia_drm(PO) snd_hda_core btintel videobuf2_vmalloc processor_thermal_device uvc btqca dell_smbios snd_rawmidi intel_lpss_pci ucsi_acpi btmtk processor_thermal_rfim rapl videobuf2_memops videobuf2_v4l2 dell_wmi_sysman typec_ucsi dcdbas snd_hwdep intel_cstate nvidia_uvm(PO) snd_seq_device
[ +0.000085] cfg80211 bluetooth psmouse intel_uncore videodev dell_wmi_ddv firmware_attributes_class dell_wmi_descriptor nvidia_modeset(PO) intel_ish_ipc intel_lpss wmi_bmof processor_thermal_mbox snd_pcm idma64 typec r8152 i2c_i801 intel_ishtp tpm_crb videobuf2_common processor_thermal_rapl mei_me spi_intel_pci tiny_power_button spi_intel intel_rapl_common i2c_smbus ecdh_generic mc mei snd_timer 8250_pci int3403_thermal mii virt_dma rfkill intel_vsec snd ecc int3400_thermal i2c_hid_acpi intel_hid mousedev evdev tpm_tis soundcore intel_soc_dts_iosf tpm_tis_core roles battery button i2c_hid crc16 joydev int340x_thermal_zone intel_pmc_core acpi_thermal_rel pinctrl_tigerlake mac_hid acpi_pad sparse_keymap serio_raw acpi_tad ac nvidia(PO) ctr loop cpufreq_powersave xt_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 br_netfilter veth tun tap macvlan bridge stp llc kvm_intel kvm fuse irqbypass deflate efi_pstore configfs efivarfs dmi_sysfs ip_tables x_tables autofs4 dm_crypt cbc encrypted_keys trusted asn1_encoder tee
[ +0.000100] tpm rng_core hid_generic usbhid hid xhci_pci rtsx_pci_sdmmc xhci_pci_renesas xhci_hcd mmc_core input_leds led_class nvme thunderbolt usbcore atkbd libps2 nvme_core vivaldi_fmap sha512_ssse3 sha512_generic aesni_intel rtsx_pci libaes t10_pi crypto_simd cryptd crc64_rocksoft crc64 crc_t10dif crct10dif_generic mfd_core usb_common crct10dif_pclmul crct10dif_common i8042 rtc_cmos serio btrfs blake2b_generic xor libcrc32c crc32c_generic crc32c_intel raid6_pq i915 i2c_algo_bit drm_buddy cec intel_gtt video wmi drm_display_helper drm_kms_helper syscopyarea sysfillrect sysimgblt ttm agpgart drm i2c_core backlight dm_snapshot dm_bufio dm_mod dax [last unloaded: hid_nvidia_shield(O)]
[ +0.000075] CR2: 000000046474e550
[ +0.000004] ---[ end trace 0000000000000000 ]---
drivers/hid/hid-nvidia-shield.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-nvidia-shield.c b/drivers/hid/hid-nvidia-shield.c
index a928ad2be62d..4e183650c447 100644
--- a/drivers/hid/hid-nvidia-shield.c
+++ b/drivers/hid/hid-nvidia-shield.c
@@ -513,21 +513,22 @@ static struct shield_device *thunderstrike_create(struct hid_device *hdev)
hid_set_drvdata(hdev, shield_dev);
+ ts->haptics_dev = shield_haptics_create(shield_dev, thunderstrike_play_effect);
+ if (IS_ERR(ts->haptics_dev))
+ return ERR_CAST(ts->haptics_dev);
+
ret = thunderstrike_led_create(ts);
if (ret) {
hid_err(hdev, "Failed to create Thunderstrike LED instance\n");
- return ERR_PTR(ret);
- }
-
- ts->haptics_dev = shield_haptics_create(shield_dev, thunderstrike_play_effect);
- if (IS_ERR(ts->haptics_dev))
goto err;
+ }
hid_info(hdev, "Registered Thunderstrike controller\n");
return shield_dev;
err:
- led_classdev_unregister(&ts->led_dev);
+ if (ts->haptics_dev)
+ input_unregister_device(ts->haptics_dev);
return ERR_CAST(ts->haptics_dev);
}
--
2.40.1