Fwd: [BUG] HID: sensor: custom: use-after-free of sensor_inst via open misc device after remove (hard lockup)

Fwd: [BUG] HID: sensor: custom: use-after-free of sensor_inst via open misc device after remove (hard lockup)

[Семён Марьясин]

[-- Attachment #1: Type: text/plain, Size: 5585 bytes --]

Hello,

hid-sensor-custom frees its per-sensor state (sensor_inst,
devm-allocated) on remove while userspace
can still hold the misc device /dev/HID-SENSOR-2000e1.N.auto open. A
reader blocked in read() stays
queued on sensor_inst->wait in freed memory; when it wakes up,
finish_wait() spins on a garbage
wait-queue lock with interrupts disabled and the machine hard-locks.
Any read()/poll()/release() on such
an fd after remove is a use-after-free as well.

The Sashiko AI review already flagged this as a pre-existing issue
on Haoxiang Li's "Fix fields lifetime issues" series
("The /dev/HID-SENSOR-* character device operations do not synchronize
with device
removal,
leading to Use-After-Free on devm-managed structures"):
https://lore.kernel.org/linux-input/20260707071545.3087073-1-haoxiang_li2024@163.com/
The applied patches don't touch it, and the current mainline is
unchanged. Below is a real-world hit.

Kernel: 7.2.8 (NixOS; drivers/hid/hid-sensor-custom.c is identical to
current mainline,
        including ad8fb82b0442 and 3789d0802ddb)
Hardware: Dell Latitude 5420 (Tiger Lake, Intel ISH), BIOS 1.49.0;
Dell's vendor ISH sensors appear as
          HID-SENSOR-2000e1.N.auto (e.g. an ST VL53L1 time-of-flight sensor).
Taint: O, from unrelated out-of-tree WWAN modules (qcserial, usb_wwan).

How it was hit
--------------

1. echo 1 > /sys/bus/platform/devices/HID-SENSOR-2000e1.2.auto/enable_sensor
2. xxd /dev/HID-SENSOR-2000e1.2.auto        # streaming reader, left running
3. modprobe -r intel_ishtp_hid intel_ish_ipc && modprobe intel_ish_ipc

The hub went away and was re-enumerated fine; ~30 s later CPU0
hard-locked in the still-running xxd
and the hard-lockup detector panicked the machine (log recovered from
EFI pstore).

Presumably a minimal reproducer is just a blocked reader plus
unbinding the platform device
(echo HID-SENSOR-2000e1.N.auto >
/sys/bus/platform/drivers/hid_sensor_custom/unbind),
but only the module-reload path above was actually tested so far.

Log (trimmed)
-------------

[146608.877730] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting
for response from ISHTP device
[146611.885959] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting
for response from ISHTP device
[146614.893909] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting
for response from ISHTP device
[146617.901872] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting
for response from ISHTP device
[146619.972550] ish-hid {33AECD58-B679-4E54-9BD9-A04D34F0C226}:
[hid-ish]: enum_devices_done OK, num_hid_devices=3
[146620.031441] hid-sensor-hub 001F:8087:0AC2.0007: hidraw3: SENSOR
HUB HID v2.00 Device [hid-ishtp 8087:0AC2] on
[146643.503266] iwlwifi 0000:00:14.3: Queue 3 is stuck 46 70
[146648.924391] watchdog: CPU0: Watchdog detected hard LOCKUP on cpu 0
[146648.924796] CPU: 0 UID: 0 PID: 1316004 Comm: xxd Tainted: G
   O        7.2.8 #1-NixOS PREEMPT(lazy)
[146648.924804] Hardware name: Dell Inc. Latitude 5420/03P5X2, BIOS
1.49.0 09/01/2025
[146648.924808] RIP: 0010:native_queued_spin_lock_slowpath+0x64/0x2c0
[146648.924851] Call Trace:
[146648.924855]  <TASK>
[146648.924857]  _raw_spin_lock_irqsave+0x3d/0x50
[146648.924872]  finish_wait+0x3b/0x90
[146648.924886]  hid_sensor_custom_read+0x102/0x150 [hid_sensor_custom]
[146648.924901]  vfs_read+0xbf/0x390
[146648.924934]  ksys_read+0xbf/0xf0
[146648.924939]  do_syscall_64+0x134/0x6b0
[146648.925010]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
[146648.925083]  </TASK>
[146648.925086] Kernel panic - not syncing: Hard LOCKUP

Also attaching a full dmesg log (with only wi-fi noise filtered out).

Analysis (line numbers as in mainline 73bef5)
---------------------------------------------

- hid_sensor_custom_probe() allocates sensor_inst with devm_kzalloc()
(line 980), so it is freed right
  after hid_sensor_custom_remove() returns, regardless of open files.
- The misc device's fops reach sensor_inst through
container_of(file->private_data, ..., custom_dev);
  misc_deregister() does not revoke already-open files, so
read/poll/release keep dereferencing it.
- hid_sensor_custom_dev_if_remove() (lines 751-758) does wake_up(),
misc_deregister(), kfifo_free().
  The woken reader in hid_sensor_custom_read() (line 664) re-evaluates
  wait_event_interruptible(sensor_inst->wait,
!kfifo_is_empty(&sensor_inst->data_fifo)), finds the fifo
  empty and goes back to sleep on sensor_inst->wait. Nothing tells it
the device is gone.
- Once sensor_inst is freed and reused, the next wakeup of that reader
runs finish_wait() on the freed
  wait_queue_head: spin_lock_irqsave() on garbage, hard lockup.
hid_sensor_custom_release() similarly
  does clear_bit() on freed sensor_inst->misc_opened when the fd is
finally closed.

Possible fix direction
----------------------

Tie sensor_inst's lifetime to its users rather than to devres, the way
hidraw handles disconnect:
refcount it (kref: one reference for the device, one per open file;
kfifo_free() and kfree() on the last
put), add a "removed" flag set in remove() before
wake_up_interruptible_all(), include it in the
wait_event condition, and make read()/poll() return -ENODEV / EPOLLHUP
| EPOLLERR once removed.

The same review pointed out two more problems in this teardown path
that a fix would naturally cover:
remove() frees data_fifo before sensor_hub_remove_callback(), so a
sample arriving in between is
kfifo_in()'d into freed memory; and an enabled sensor is never
sensor_hub_device_close()d on remove.

I can test patches on this machine.

Thanks,
Semyon Maryasin

P.S.: sorry for a possible duplicate, I think I messed with the headers.

[-- Attachment #2: dmesg-clean.txt --]
[-- Type: text/plain, Size: 14021 bytes --]

<6>[122586.929775] PM: suspend entry (s2idle)
<6>[122586.977820] Filesystems sync: 0.048 seconds
<6>[122586.988535] Freezing user space processes
<6>[122586.990269] Freezing user space processes completed (elapsed 0.001 seconds)
<6>[122586.990274] OOM killer disabled.
<6>[122586.990275] Freezing remaining freezable tasks
<6>[122586.991832] Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
<6>[122586.991835] printk: Suspending console(s) (use no_console_suspend to debug)
<6>[122587.012388] e1000e: EEE TX LPI TIMER: 00000011
<6>[122587.394462] ACPI: EC: interrupt blocked
<6>[122590.828113] ACPI: EC: interrupt unblocked
<6>[122591.185256] i915 0000:00:02.0: vgaarb: VGA decodes changed: olddecodes=io,decodes=io:owns=io+mem
<6>[122591.205074] nvme nvme0: 8/0/0 default/read/poll queues
<6>[122591.207053] nvme nvme0: Ignoring bogus Namespace Identifiers
<6>[122592.245524] OOM killer enabled.
<6>[122592.245530] Restarting tasks: Starting
<6>[122592.248615] mei_hdcp 0000:00:16.0-b638ab7e-94e2-4ea2-a552-d1c54b627f04: bound 0000:00:02.0 (ops i915_hdcp_ops [i915])
<6>[122592.251481] Restarting tasks: Done
<6>[122592.251488] efivarfs: resyncing variable state
<6>[122592.259633] efivarfs: finished resyncing variable state
<5>[122592.259666] random: crng reseeded on system resumption
<6>[122592.294055] PM: suspend exit
<6>[122592.392682] e1000e 0000:00:1f.6 eth0: NIC Link is Down
<6>[122622.964242] PM: suspend entry (s2idle)
<6>[122623.008566] Filesystems sync: 0.044 seconds
<6>[122623.023288] Freezing user space processes
<6>[122623.025388] Freezing user space processes completed (elapsed 0.002 seconds)
<6>[122623.025393] OOM killer disabled.
<6>[122623.025394] Freezing remaining freezable tasks
<6>[122623.027011] Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
<6>[122623.027014] printk: Suspending console(s) (use no_console_suspend to debug)
<6>[122623.045649] e1000e: EEE TX LPI TIMER: 00000011
<6>[122623.426308] ACPI: EC: interrupt blocked
<6>[142890.179053] ACPI: EC: interrupt unblocked
<6>[142890.577373] i915 0000:00:02.0: vgaarb: VGA decodes changed: olddecodes=io,decodes=io:owns=io+mem
<6>[142890.602715] nvme nvme0: 8/0/0 default/read/poll queues
<6>[142890.604761] nvme nvme0: Ignoring bogus Namespace Identifiers
<6>[142891.665519] OOM killer enabled.
<6>[142891.665532] Restarting tasks: Starting
<6>[142891.669205] mei_hdcp 0000:00:16.0-b638ab7e-94e2-4ea2-a552-d1c54b627f04: bound 0000:00:02.0 (ops i915_hdcp_ops [i915])
<6>[142891.681315] Restarting tasks: Done
<6>[142891.681353] efivarfs: resyncing variable state
<6>[142891.727579] efivarfs: finished resyncing variable state
<5>[142891.727685] random: crng reseeded on system resumption
<6>[142891.870922] PM: suspend exit
<6>[142892.153878] e1000e 0000:00:1f.6 eth0: NIC Link is Down
<3>[146608.877730] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting for response from ISHTP device
<3>[146611.885959] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting for response from ISHTP device
<3>[146614.893909] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting for response from ISHTP device
<3>[146617.901872] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting for response from ISHTP device
<6>[146619.972550] ish-hid {33AECD58-B679-4E54-9BD9-A04D34F0C226}: [hid-ish]: enum_devices_done OK, num_hid_devices=3
<6>[146619.981751] hid-sensor-hub 001F:8087:0AC2.0005: hidraw0: SENSOR HUB HID v2.00 Device [hid-ishtp 8087:0AC2] on 
<6>[146620.011396] hid-sensor-hub 001F:8087:0AC2.0006: hidraw1: SENSOR HUB HID v2.00 Device [hid-ishtp 8087:0AC2] on 
<6>[146620.031441] hid-sensor-hub 001F:8087:0AC2.0007: hidraw3: SENSOR HUB HID v2.00 Device [hid-ishtp 8087:0AC2] on 
<0>[146648.924391] watchdog: CPU0: Watchdog detected hard LOCKUP on cpu 0
<4>[146648.924399] Modules linked in: intel_ishtp_hid intel_ish_ipc intel_ishtp ext4 mbcache jbd2 sd_mod uas usb_storage scsi_mod scsi_common nf_conntrack_netlink unix_diag ccm tcp_diag inet_diag qrtr rfcomm snd_seq_dummy snd_hrtimer snd_seq snd_seq_device xfrm_user xfrm_algo xt_set ip_set xt_addrtype overlay xt_CHECKSUM xt_MASQUERADE af_packet aes algif_hash algif_skcipher af_alg uinput wireguard libcurve25519 bnep libchacha20poly1305 libchacha libpoly1305 snd_hda_codec_intelhdmi ip6_udp_tunnel snd_hda_codec_hdmi udp_tunnel msr snd_ctl_led dell_pc snd_hda_codec_alc269 snd_hda_codec_realtek_lib snd_hda_scodec_component snd_hda_codec_generic hid_sensor_custom_intel_hinge snd_hda_intel snd_sof_pci_intel_tgl snd_sof_pci_intel_cnl snd_sof_intel_hda_generic soundwire_intel hid_sensor_gyro_3d hid_sensor_trigger snd_sof_intel_hda_sdw_bpt kfifo_buf hid_sensor_iio_common snd_sof_intel_hda_common industrialio snd_soc_hdac_hda hid_sensor_custom snd_sof_intel_hda_mlink snd_sof_intel_hda soundwire_cadence snd_sof_pci snd_sof_xtensa_dsp
<4>[146648.924491]  snd_sof joydev snd_sof_utils snd_soc_acpi_intel_match snd_soc_acpi_intel_sdca_quirks soundwire_generic_allocation hid_sensor_hub snd_soc_sdw_utils snd_soc_acpi soundwire_bus mousedev nls_iso8859_1 snd_soc_sdca nls_cp437 ofpart vfat crc8 cmdlinepart ee1004 spi_nor hid_multitouch snd_soc_avs iTCO_wdt mtd hid_generic intel_pmc_bxt snd_soc_hda_codec snd_hda_ext_core snd_hda_codec intel_rapl_msr snd_hda_core uvcvideo snd_intel_dspcfg intel_uncore_frequency intel_uncore_frequency_common snd_intel_sdw_acpi videobuf2_vmalloc snd_hwdep uvc x86_pkg_temp_thermal intel_powerclamp cdc_mbim snd_soc_core videobuf2_memops cdc_wdm videobuf2_v4l2 cdc_ncm cdc_ether videobuf2_common snd_compress iwlmvm ac97_bus snd_pcm_dmaengine dell_rbtn videodev coretemp snd_pcm usbnet dell_wmi mei_hdcp mei_pxp mei_wdt mc mii aesni_intel dell_laptop gf128mul mac80211 e1000e fat snd_timer i2c_i801 rapl dell_smbios ucsi_acpi snd ptp typec_ucsi btusb dell_wmi_sysman i2c_smbus intel_cstate dcdbas spi_intel_pci libarc4 xe intel_uncore psmouse
<4>[146648.924696]  int340x_thermal_zone 8250_pci edac_core virt_dma intel_soc_dts_iosf rfkill evdev mac_hid ecc crc16 serio_raw xt_conntrack xt_tcpudp ip6t_rpfilter ipt_rpfilter xt_pkttype nft_compat nf_tables sch_fq_codel kvm_intel kvm xt_nat x_tables nf_nat br_netfilter nf_conntrack bridge qcserial(O) nf_defrag_ipv6 nf_defrag_ipv4 usb_wwan(O) tun veth stp fuse loop llc irqbypass cpufreq_powersave ctr dmi_sysfs nvme atkbd rtsx_pci_sdmmc libps2 nvme_core mmc_core vivaldi_fmap xhci_pci i8042 nvme_keyring rtsx_pci nvme_auth thunderbolt serio xhci_hcd btrfs i915 libblake2b drm_buddy i2c_algo_bit video wmi ttm intel_gtt drm_display_helper dm_mod raid6_pq cec xor configfs efivarfs autofs4 [last unloaded: intel_ishtp]
<4>[146648.924796] CPU: 0 UID: 0 PID: 1316004 Comm: xxd Tainted: G           O        7.2.8 #1-NixOS PREEMPT(lazy) 
<4>[146648.924803] Tainted: [O]=OOT_MODULE
<4>[146648.924804] Hardware name: Dell Inc. Latitude 5420/03P5X2, BIOS 1.49.0 09/01/2025
<4>[146648.924808] RIP: 0010:native_queued_spin_lock_slowpath+0x64/0x2c0
<4>[146648.924821] Code: 77 71 f0 0f ba 29 08 0f 92 c2 8b 01 0f b6 d2 c1 e2 08 30 e4 09 d0 3d ff 00 00 00 77 4d 85 c0 74 10 0f b6 01 84 c0 74 09 f3 90 <0f> b6 01 84 c0 75 f7 b8 01 00 00 00 66 89 01 e9 98 1b 00 00 8b 37
<4>[146648.924824] RSP: 0018:ffffd1d80fbbfa80 EFLAGS: 00000002
<4>[146648.924828] RAX: 0000000000000001 RBX: 0000000000000202 RCX: ffff8eb6c9f17f28
<4>[146648.924831] RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff8eb6c9f17f28
<4>[146648.924833] RBP: ffffd1d80fbbfae0 R08: 0000000000000001 R09: 0000000000000001
<4>[146648.924836] R10: ffff8ec019080000 R11: 0000000000000000 R12: ffff8eb6c9f17f28
<4>[146648.924838] R13: ffff8ec1b5d03180 R14: 0000000000000001 R15: 0000000000000000
<4>[146648.924840] FS:  00007e18ed70a780(0000) GS:ffff8ec66aeb9000(0000) knlGS:0000000000000000
<4>[146648.924844] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4>[146648.924846] CR2: 0000742747faf000 CR3: 0000000bf305f004 CR4: 0000000000f72ef0
<4>[146648.924849] PKRU: 55555554
<4>[146648.924851] Call Trace:
<4>[146648.924855]  <TASK>
<4>[146648.924857]  _raw_spin_lock_irqsave+0x3d/0x50
<4>[146648.924872]  finish_wait+0x3b/0x90
<4>[146648.924886]  hid_sensor_custom_read+0x102/0x150 [hid_sensor_custom]
<4>[146648.924894]  ? __pfx_autoremove_wake_function+0x10/0x10
<4>[146648.924901]  vfs_read+0xbf/0x390
<4>[146648.924910]  ? tty_insert_flip_string_and_push_buffer+0x95/0xc0
<4>[146648.924921]  ? remove_wait_queue+0x24/0x60
<4>[146648.924926]  ? n_tty_write+0x3b9/0x4e0
<4>[146648.924934]  ksys_read+0xbf/0xf0
<4>[146648.924939]  do_syscall_64+0x134/0x6b0
<4>[146648.924946]  ? file_tty_write.isra.0+0x20e/0x320
<4>[146648.924952]  ? vfs_write+0x25d/0x470
<4>[146648.924958]  ? ksys_write+0x73/0xf0
<4>[146648.924963]  ? do_syscall_64+0x173/0x6b0
<4>[146648.924966]  ? n_tty_write+0x3b9/0x4e0
<4>[146648.924972]  ? __wake_up+0x44/0x60
<4>[146648.924978]  ? file_tty_write.isra.0+0x20e/0x320
<4>[146648.924983]  ? vfs_write+0x25d/0x470
<4>[146648.924989]  ? ksys_write+0x73/0xf0
<4>[146648.924993]  ? do_syscall_64+0x173/0x6b0
<4>[146648.924997]  ? do_syscall_64+0x173/0x6b0
<4>[146648.925000]  ? do_syscall_64+0xeb/0x6b0
<4>[146648.925002]  ? clear_bhb_loop+0x60/0xb0
<4>[146648.925010]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
<4>[146648.925016] RIP: 0033:0x7e18ed49915e
<4>[146648.925059] Code: 03 00 00 59 5e 48 83 f8 fc 74 39 c9 31 d2 31 c9 31 f6 31 ff 45 31 c0 45 31 c9 45 31 d2 45 31 db c3 0f 1f 00 48 8b 45 10 0f 05 <c9> 31 d2 31 c9 31 f6 31 ff 45 31 c0 45 31 c9 45 31 d2 45 31 db c3
<4>[146648.925062] RSP: 002b:00007ffd67ba5f60 EFLAGS: 00000202 ORIG_RAX: 0000000000000000
<4>[146648.925065] RAX: ffffffffffffffda RBX: 0000614264f63310 RCX: 00007e18ed49915e
<4>[146648.925067] RDX: 0000000000001000 RSI: 0000614264f634f0 RDI: 0000000000000003
<4>[146648.925069] RBP: 00007ffd67ba5f70 R08: 0000000000000000 R09: 0000000000000000
<4>[146648.925071] R10: 0000000000000000 R11: 0000000000000202 R12: 00007e18ed5f9010
<4>[146648.925073] R13: 00007e18ed5f8ec0 R14: 0000000000000000 R15: 0000000000000000
<4>[146648.925083]  </TASK>
<0>[146648.925086] Kernel panic - not syncing: Hard LOCKUP
<4>[146648.925089] CPU: 0 UID: 0 PID: 1316004 Comm: xxd Tainted: G           O        7.2.8 #1-NixOS PREEMPT(lazy) 
<4>[146648.925093] Tainted: [O]=OOT_MODULE
<4>[146648.925094] Hardware name: Dell Inc. Latitude 5420/03P5X2, BIOS 1.49.0 09/01/2025
<4>[146648.925096] Call Trace:
<4>[146648.925099]  <NMI>
<4>[146648.925101]  dump_stack_lvl+0x5d/0x80
<4>[146648.925110]  vpanic+0x20f/0x410
<4>[146648.925118]  panic+0x6b/0x70
<4>[146648.925124]  nmi_panic.cold+0xc/0xc
<4>[146648.925129]  watchdog_hardlockup_check.cold+0x11e/0x1e8
<4>[146648.925136]  __perf_event_overflow+0x120/0x3c0
<4>[146648.925148]  handle_pmi_common+0x1e0/0x450
<4>[146648.925162]  intel_pmu_handle_irq+0xf4/0x4c0
<4>[146648.925167]  perf_event_nmi_handler+0x42/0x70
<4>[146648.925176]  nmi_handle.part.0+0x53/0x110
<4>[146648.925185]  default_do_nmi+0x10e/0x170
<4>[146648.925193]  exc_nmi+0x129/0x1a0
<4>[146648.925198]  end_repeat_nmi+0xf/0x53
<4>[146648.925203] RIP: 0010:native_queued_spin_lock_slowpath+0x64/0x2c0
<4>[146648.925207] Code: 77 71 f0 0f ba 29 08 0f 92 c2 8b 01 0f b6 d2 c1 e2 08 30 e4 09 d0 3d ff 00 00 00 77 4d 85 c0 74 10 0f b6 01 84 c0 74 09 f3 90 <0f> b6 01 84 c0 75 f7 b8 01 00 00 00 66 89 01 e9 98 1b 00 00 8b 37
<4>[146648.925210] RSP: 0018:ffffd1d80fbbfa80 EFLAGS: 00000002
<4>[146648.925213] RAX: 0000000000000001 RBX: 0000000000000202 RCX: ffff8eb6c9f17f28
<4>[146648.925215] RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff8eb6c9f17f28
<4>[146648.925217] RBP: ffffd1d80fbbfae0 R08: 0000000000000001 R09: 0000000000000001
<4>[146648.925219] R10: ffff8ec019080000 R11: 0000000000000000 R12: ffff8eb6c9f17f28
<4>[146648.925221] R13: ffff8ec1b5d03180 R14: 0000000000000001 R15: 0000000000000000
<4>[146648.925227]  ? native_queued_spin_lock_slowpath+0x64/0x2c0
<4>[146648.925231]  ? native_queued_spin_lock_slowpath+0x64/0x2c0
<4>[146648.925235]  </NMI>
<4>[146648.925236]  <TASK>
<4>[146648.925238]  _raw_spin_lock_irqsave+0x3d/0x50
<4>[146648.925243]  finish_wait+0x3b/0x90
<4>[146648.925249]  hid_sensor_custom_read+0x102/0x150 [hid_sensor_custom]
<4>[146648.925254]  ? __pfx_autoremove_wake_function+0x10/0x10
<4>[146648.925261]  vfs_read+0xbf/0x390
<4>[146648.925264]  ? tty_insert_flip_string_and_push_buffer+0x95/0xc0
<4>[146648.925271]  ? remove_wait_queue+0x24/0x60
<4>[146648.925276]  ? n_tty_write+0x3b9/0x4e0
<4>[146648.925281]  ksys_read+0xbf/0xf0
<4>[146648.925285]  do_syscall_64+0x134/0x6b0
<4>[146648.925289]  ? file_tty_write.isra.0+0x20e/0x320
<4>[146648.925294]  ? vfs_write+0x25d/0x470
<4>[146648.925300]  ? ksys_write+0x73/0xf0
<4>[146648.925304]  ? do_syscall_64+0x173/0x6b0
<4>[146648.925308]  ? n_tty_write+0x3b9/0x4e0
<4>[146648.925313]  ? __wake_up+0x44/0x60
<4>[146648.925319]  ? file_tty_write.isra.0+0x20e/0x320
<4>[146648.925323]  ? vfs_write+0x25d/0x470
<4>[146648.925330]  ? ksys_write+0x73/0xf0
<4>[146648.925334]  ? do_syscall_64+0x173/0x6b0
<4>[146648.925337]  ? do_syscall_64+0x173/0x6b0
<4>[146648.925340]  ? do_syscall_64+0xeb/0x6b0
<4>[146648.925343]  ? clear_bhb_loop+0x60/0xb0
<4>[146648.925348]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
<4>[146648.925352] RIP: 0033:0x7e18ed49915e
<4>[146648.925359] Code: 03 00 00 59 5e 48 83 f8 fc 74 39 c9 31 d2 31 c9 31 f6 31 ff 45 31 c0 45 31 c9 45 31 d2 45 31 db c3 0f 1f 00 48 8b 45 10 0f 05 <c9> 31 d2 31 c9 31 f6 31 ff 45 31 c0 45 31 c9 45 31 d2 45 31 db c3
<4>[146648.925361] RSP: 002b:00007ffd67ba5f60 EFLAGS: 00000202 ORIG_RAX: 0000000000000000
<4>[146648.925365] RAX: ffffffffffffffda RBX: 0000614264f63310 RCX: 00007e18ed49915e
<4>[146648.925367] RDX: 0000000000001000 RSI: 0000614264f634f0 RDI: 0000000000000003
<4>[146648.925369] RBP: 00007ffd67ba5f70 R08: 0000000000000000 R09: 0000000000000000
<4>[146648.925371] R10: 0000000000000000 R11: 0000000000000202 R12: 00007e18ed5f9010
<4>[146648.925373] R13: 00007e18ed5f8ec0 R14: 0000000000000000 R15: 0000000000000000
<4>[146648.925379]  </TASK>
<0>[146648.925436] Kernel Offset: 0x20c00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)

Re: Fwd: [BUG] HID: sensor: custom: use-after-free of sensor_inst via open misc device after remove (hard lockup)

[Srinivas Pandruvada]

[-- Attachment #1: Type: text/plain, Size: 6221 bytes --]

Hi,
On Fri, 2026-10-23 at 13:42 +0300, Семён Марьясин wrote:
> Hello,
> 
> hid-sensor-custom frees its per-sensor state (sensor_inst,
> devm-allocated) on remove while userspace
> can still hold the misc device /dev/HID-SENSOR-2000e1.N.auto open. A
> reader blocked in read() stays
> queued on sensor_inst->wait in freed memory; when it wakes up,
> finish_wait() spins on a garbage
> wait-queue lock with interrupts disabled and the machine hard-locks.
> Any read()/poll()/release() on such
> an fd after remove is a use-after-free as well.
> 

Please give a try with the attached diff.


Thanks,
Srinivas


> The Sashiko AI review already flagged this as a pre-existing issue
> on Haoxiang Li's "Fix fields lifetime issues" series
> ("The /dev/HID-SENSOR-* character device operations do not
> synchronize
> with device
> removal,
> leading to Use-After-Free on devm-managed structures"):
> https://lore.kernel.org/linux-input/20260707071545.3087073-1-haoxiang_li2024@163.com/
> The applied patches don't touch it, and the current mainline is
> unchanged. Below is a real-world hit.
> 
> Kernel: 7.2.8 (NixOS; drivers/hid/hid-sensor-custom.c is identical to
> current mainline,
>         including ad8fb82b0442 and 3789d0802ddb)
> Hardware: Dell Latitude 5420 (Tiger Lake, Intel ISH), BIOS 1.49.0;
> Dell's vendor ISH sensors appear as
>           HID-SENSOR-2000e1.N.auto (e.g. an ST VL53L1 time-of-flight
> sensor).
> Taint: O, from unrelated out-of-tree WWAN modules (qcserial,
> usb_wwan).
> 
> How it was hit
> --------------
> 
> 1. echo 1 > /sys/bus/platform/devices/HID-SENSOR-
> 2000e1.2.auto/enable_sensor
> 2. xxd /dev/HID-SENSOR-2000e1.2.auto        # streaming reader, left
> running
> 3. modprobe -r intel_ishtp_hid intel_ish_ipc && modprobe
> intel_ish_ipc
> 
> The hub went away and was re-enumerated fine; ~30 s later CPU0
> hard-locked in the still-running xxd
> and the hard-lockup detector panicked the machine (log recovered from
> EFI pstore).
> 
> Presumably a minimal reproducer is just a blocked reader plus
> unbinding the platform device
> (echo HID-SENSOR-2000e1.N.auto >
> /sys/bus/platform/drivers/hid_sensor_custom/unbind),
> but only the module-reload path above was actually tested so far.
> 
> Log (trimmed)
> -------------
> 
> [146608.877730] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting
> for response from ISHTP device
> [146611.885959] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting
> for response from ISHTP device
> [146614.893909] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting
> for response from ISHTP device
> [146617.901872] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting
> for response from ISHTP device
> [146619.972550] ish-hid {33AECD58-B679-4E54-9BD9-A04D34F0C226}:
> [hid-ish]: enum_devices_done OK, num_hid_devices=3
> [146620.031441] hid-sensor-hub 001F:8087:0AC2.0007: hidraw3: SENSOR
> HUB HID v2.00 Device [hid-ishtp 8087:0AC2] on
> [146643.503266] iwlwifi 0000:00:14.3: Queue 3 is stuck 46 70
> [146648.924391] watchdog: CPU0: Watchdog detected hard LOCKUP on cpu
> 0
> [146648.924796] CPU: 0 UID: 0 PID: 1316004 Comm: xxd Tainted: G
>    O        7.2.8 #1-NixOS PREEMPT(lazy)
> [146648.924804] Hardware name: Dell Inc. Latitude 5420/03P5X2, BIOS
> 1.49.0 09/01/2025
> [146648.924808] RIP: 0010:native_queued_spin_lock_slowpath+0x64/0x2c0
> [146648.924851] Call Trace:
> [146648.924855]  <TASK>
> [146648.924857]  _raw_spin_lock_irqsave+0x3d/0x50
> [146648.924872]  finish_wait+0x3b/0x90
> [146648.924886]  hid_sensor_custom_read+0x102/0x150
> [hid_sensor_custom]
> [146648.924901]  vfs_read+0xbf/0x390
> [146648.924934]  ksys_read+0xbf/0xf0
> [146648.924939]  do_syscall_64+0x134/0x6b0
> [146648.925010]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> [146648.925083]  </TASK>
> [146648.925086] Kernel panic - not syncing: Hard LOCKUP
> 
> Also attaching a full dmesg log (with only wi-fi noise filtered out).
> 
> Analysis (line numbers as in mainline 73bef5)
> ---------------------------------------------
> 
> - hid_sensor_custom_probe() allocates sensor_inst with devm_kzalloc()
> (line 980), so it is freed right
>   after hid_sensor_custom_remove() returns, regardless of open files.
> - The misc device's fops reach sensor_inst through
> container_of(file->private_data, ..., custom_dev);
>   misc_deregister() does not revoke already-open files, so
> read/poll/release keep dereferencing it.
> - hid_sensor_custom_dev_if_remove() (lines 751-758) does wake_up(),
> misc_deregister(), kfifo_free().
>   The woken reader in hid_sensor_custom_read() (line 664) re-
> evaluates
>   wait_event_interruptible(sensor_inst->wait,
> !kfifo_is_empty(&sensor_inst->data_fifo)), finds the fifo
>   empty and goes back to sleep on sensor_inst->wait. Nothing tells it
> the device is gone.
> - Once sensor_inst is freed and reused, the next wakeup of that
> reader
> runs finish_wait() on the freed
>   wait_queue_head: spin_lock_irqsave() on garbage, hard lockup.
> hid_sensor_custom_release() similarly
>   does clear_bit() on freed sensor_inst->misc_opened when the fd is
> finally closed.
> 
> Possible fix direction
> ----------------------
> 
> Tie sensor_inst's lifetime to its users rather than to devres, the
> way
> hidraw handles disconnect:
> refcount it (kref: one reference for the device, one per open file;
> kfifo_free() and kfree() on the last
> put), add a "removed" flag set in remove() before
> wake_up_interruptible_all(), include it in the
> wait_event condition, and make read()/poll() return -ENODEV /
> EPOLLHUP
> > EPOLLERR once removed.
> 
> The same review pointed out two more problems in this teardown path
> that a fix would naturally cover:
> remove() frees data_fifo before sensor_hub_remove_callback(), so a
> sample arriving in between is
> kfifo_in()'d into freed memory; and an enabled sensor is never
> sensor_hub_device_close()d on remove.
> 
> I can test patches on this machine.
> 
> Thanks,
> Semyon Maryasin
> 
> P.S.: sorry for a possible duplicate, I think I messed with the
> headers.

[-- Attachment #2: test_patch.diff --]
[-- Type: text/x-patch, Size: 7630 bytes --]

diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
index afffea894021..c12417cddab4 100644
--- a/drivers/hid/hid-sensor-custom.c
+++ b/drivers/hid/hid-sensor-custom.c
@@ -11,6 +11,7 @@
 #include <linux/init.h>
 #include <linux/miscdevice.h>
 #include <linux/kfifo.h>
+#include <linux/kref.h>
 #include <linux/sched.h>
 #include <linux/wait.h>
 #include <linux/poll.h>
@@ -37,6 +38,8 @@ struct hid_sensor_custom_field {
 
 struct hid_sensor_custom {
 	struct mutex mutex;
+	struct kref ref;
+	bool removed;
 	struct platform_device *pdev;
 	struct hid_sensor_hub_device *hsdev;
 	struct hid_sensor_hub_callbacks callbacks;
@@ -636,6 +639,15 @@ static void hid_sensor_custom_remove_attributes(struct hid_sensor_custom *
 	kfree(sensor_inst->fields);
 }
 
+static void hid_sensor_custom_free(struct kref *ref)
+{
+	struct hid_sensor_custom *sensor_inst =
+		container_of(ref, struct hid_sensor_custom, ref);
+
+	kfifo_free(&sensor_inst->data_fifo);
+	kfree(sensor_inst);
+}
+
 static ssize_t hid_sensor_custom_read(struct file *file, char __user *buf,
 				      size_t count, loff_t *f_ps)
 {
@@ -649,22 +661,35 @@ static ssize_t hid_sensor_custom_read(struct file *file, char __user *buf,
 	if (count < sizeof(struct hid_sensor_sample))
 		return -EINVAL;
 
-	do {
-		if (kfifo_is_empty(&sensor_inst->data_fifo)) {
-			if (file->f_flags & O_NONBLOCK)
-				return -EAGAIN;
+	for (;;) {
+		ret = mutex_lock_interruptible(&sensor_inst->mutex);
+		if (ret)
+			return ret;
 
-			ret = wait_event_interruptible(sensor_inst->wait,
-				!kfifo_is_empty(&sensor_inst->data_fifo));
-			if (ret)
-				return ret;
+		if (sensor_inst->removed) {
+			ret = -ENODEV;
+			goto unlock;
 		}
 		ret = kfifo_to_user(&sensor_inst->data_fifo, buf, count,
 				    &copied);
+		if (ret || copied)
+			goto unlock;
+		mutex_unlock(&sensor_inst->mutex);
+
+		if (file->f_flags & O_NONBLOCK)
+			return -EAGAIN;
+
+		ret = wait_event_interruptible(sensor_inst->wait,
+			READ_ONCE(sensor_inst->removed) ||
+			!kfifo_is_empty(&sensor_inst->data_fifo));
 		if (ret)
 			return ret;
+	}
 
-	} while (copied == 0);
+unlock:
+	mutex_unlock(&sensor_inst->mutex);
+	if (ret)
+		return ret;
 
 	return copied;
 }
@@ -677,6 +702,7 @@ static int hid_sensor_custom_release(struct inode *inode, struct file *file)
 				   struct hid_sensor_custom, custom_dev);
 
 	clear_bit(0, &sensor_inst->misc_opened);
+	kref_put(&sensor_inst->ref, hid_sensor_custom_free);
 
 	return 0;
 }
@@ -684,14 +710,22 @@ static int hid_sensor_custom_release(struct inode *inode, struct file *file)
 static int hid_sensor_custom_open(struct inode *inode, struct file *file)
 {
 	struct hid_sensor_custom *sensor_inst;
+	int ret;
 
 	sensor_inst = container_of(file->private_data,
 				   struct hid_sensor_custom, custom_dev);
-	/* We essentially have single reader and writer */
-	if (test_and_set_bit(0, &sensor_inst->misc_opened))
-		return -EBUSY;
+	mutex_lock(&sensor_inst->mutex);
+	if (sensor_inst->removed) {
+		ret = -ENODEV;
+	} else if (test_and_set_bit(0, &sensor_inst->misc_opened)) {
+		ret = -EBUSY;
+	} else {
+		kref_get(&sensor_inst->ref);
+		ret = stream_open(inode, file);
+	}
+	mutex_unlock(&sensor_inst->mutex);
 
-	return stream_open(inode, file);
+	return ret;
 }
 
 static __poll_t hid_sensor_custom_poll(struct file *file,
@@ -705,6 +739,9 @@ static __poll_t hid_sensor_custom_poll(struct file *file,
 
 	poll_wait(file, &sensor_inst->wait, wait);
 
+	if (READ_ONCE(sensor_inst->removed))
+		return EPOLLHUP | EPOLLERR;
+
 	if (!kfifo_is_empty(&sensor_inst->data_fifo))
 		mask = EPOLLIN | EPOLLRDNORM;
 
@@ -712,6 +749,7 @@ static __poll_t hid_sensor_custom_poll(struct file *file,
 }
 
 static const struct file_operations hid_sensor_custom_fops = {
+	.owner = THIS_MODULE,
 	.open =  hid_sensor_custom_open,
 	.read =  hid_sensor_custom_read,
 	.release = hid_sensor_custom_release,
@@ -734,20 +772,17 @@ static int hid_sensor_custom_dev_if_add(struct hid_sensor_custom *sensor_inst)
 	sensor_inst->custom_dev.name = dev_name(&sensor_inst->pdev->dev);
 	sensor_inst->custom_dev.fops = &hid_sensor_custom_fops;
 	ret = misc_register(&sensor_inst->custom_dev);
-	if (ret) {
-		kfifo_free(&sensor_inst->data_fifo);
-		return ret;
-	}
-	return 0;
+	return ret;
 }
 
 static void hid_sensor_custom_dev_if_remove(struct hid_sensor_custom
 								*sensor_inst)
 {
-	wake_up(&sensor_inst->wait);
+	mutex_lock(&sensor_inst->mutex);
+	WRITE_ONCE(sensor_inst->removed, true);
+	mutex_unlock(&sensor_inst->mutex);
+	wake_up_interruptible_all(&sensor_inst->wait);
 	misc_deregister(&sensor_inst->custom_dev);
-	kfifo_free(&sensor_inst->data_fifo);
-
 }
 
 /*
@@ -970,8 +1005,7 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
 	int ret;
 	const struct hid_sensor_custom_match *match;
 
-	sensor_inst = devm_kzalloc(&pdev->dev, sizeof(*sensor_inst),
-				   GFP_KERNEL);
+	sensor_inst = kzalloc_obj(*sensor_inst);
 	if (!sensor_inst)
 		return -ENOMEM;
 
@@ -981,6 +1015,7 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
 	sensor_inst->hsdev = hsdev;
 	sensor_inst->pdev = pdev;
 	mutex_init(&sensor_inst->mutex);
+	kref_init(&sensor_inst->ref);
 	platform_set_drvdata(pdev, sensor_inst);
 
 	ret = hid_sensor_custom_get_known(hsdev, &match);
@@ -992,7 +1027,7 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
 		if (ret) {
 			dev_err(&pdev->dev,
 				"register_platform_device failed\n");
-			return ret;
+			goto err_free;
 		}
 
 		return 0;
@@ -1002,7 +1037,7 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
 					   &sensor_inst->callbacks);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "callback reg failed\n");
-		return ret;
+		goto err_free;
 	}
 
 	ret = sysfs_create_group(&sensor_inst->pdev->dev.kobj,
@@ -1027,6 +1062,8 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
 			   &enable_sensor_attr_group);
 err_remove_callback:
 	sensor_hub_remove_callback(hsdev, hsdev->usage);
+err_free:
+	kref_put(&sensor_inst->ref, hid_sensor_custom_free);
 
 	return ret;
 }
@@ -1038,14 +1075,18 @@ static void hid_sensor_custom_remove(struct platform_device *pdev)
 
 	if (sensor_inst->custom_pdev) {
 		platform_device_unregister(sensor_inst->custom_pdev);
-		return;
+		goto out;
 	}
 
 	hid_sensor_custom_dev_if_remove(sensor_inst);
 	hid_sensor_custom_remove_attributes(sensor_inst);
 	sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
 			   &enable_sensor_attr_group);
+	if (sensor_inst->enable)
+		sensor_hub_device_close(hsdev);
 	sensor_hub_remove_callback(hsdev, hsdev->usage);
+out:
+	kref_put(&sensor_inst->ref, hid_sensor_custom_free);
 }
 
 static const struct platform_device_id hid_sensor_custom_ids[] = {
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 90666ff629de..2bac8b82ce37 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -173,7 +173,8 @@ int sensor_hub_remove_callback(struct hid_sensor_hub_device *hsdev,
 	struct sensor_hub_data *pdata = hid_get_drvdata(hsdev->hdev);
 	unsigned long flags;
 
-	spin_lock_irqsave(&pdata->dyn_callback_lock, flags);
+	spin_lock_irqsave(&pdata->lock, flags);
+	spin_lock(&pdata->dyn_callback_lock);
 	list_for_each_entry(callback, &pdata->dyn_callback_list, list)
 		if (callback->usage_id == usage_id &&
 						callback->hsdev == hsdev) {
@@ -181,7 +182,8 @@ int sensor_hub_remove_callback(struct hid_sensor_hub_device *hsdev,
 			kfree(callback);
 			break;
 		}
-	spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
+	spin_unlock(&pdata->dyn_callback_lock);
+	spin_unlock_irqrestore(&pdata->lock, flags);
 
 	return 0;
 }

Re: Fwd: [BUG] HID: sensor: custom: use-after-free of sensor_inst via open misc device after remove (hard lockup)

[Семён Марьясин]

[-- Attachment #1: Type: text/plain, Size: 7078 bytes --]

Hello! Tested this patch (had to adapt it to apply it to 7.2.8 kernel;
attaching the adapted version).

Without patch: xxd running while intel_ishtp_hid module is removed ->
xxd hangs, system either crashes or at least logs "hid-sensor-hub
001F:8087:0AC2.0004: timeout waiting for response from ISHTP device"
With patch: xxd correctly closes with message "No such device", no
error in kernel logs, no crashes — checked for several times.

пт, 23 окт. 2026 г. в 17:12, Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com>:
>
> Hi,
> On Fri, 2026-10-23 at 13:42 +0300, Семён Марьясин wrote:
> > Hello,
> >
> > hid-sensor-custom frees its per-sensor state (sensor_inst,
> > devm-allocated) on remove while userspace
> > can still hold the misc device /dev/HID-SENSOR-2000e1.N.auto open. A
> > reader blocked in read() stays
> > queued on sensor_inst->wait in freed memory; when it wakes up,
> > finish_wait() spins on a garbage
> > wait-queue lock with interrupts disabled and the machine hard-locks.
> > Any read()/poll()/release() on such
> > an fd after remove is a use-after-free as well.
> >
>
> Please give a try with the attached diff.
>
>
> Thanks,
> Srinivas
>
>
> > The Sashiko AI review already flagged this as a pre-existing issue
> > on Haoxiang Li's "Fix fields lifetime issues" series
> > ("The /dev/HID-SENSOR-* character device operations do not
> > synchronize
> > with device
> > removal,
> > leading to Use-After-Free on devm-managed structures"):
> > https://lore.kernel.org/linux-input/20260707071545.3087073-1-haoxiang_li2024@163.com/
> > The applied patches don't touch it, and the current mainline is
> > unchanged. Below is a real-world hit.
> >
> > Kernel: 7.2.8 (NixOS; drivers/hid/hid-sensor-custom.c is identical to
> > current mainline,
> >         including ad8fb82b0442 and 3789d0802ddb)
> > Hardware: Dell Latitude 5420 (Tiger Lake, Intel ISH), BIOS 1.49.0;
> > Dell's vendor ISH sensors appear as
> >           HID-SENSOR-2000e1.N.auto (e.g. an ST VL53L1 time-of-flight
> > sensor).
> > Taint: O, from unrelated out-of-tree WWAN modules (qcserial,
> > usb_wwan).
> >
> > How it was hit
> > --------------
> >
> > 1. echo 1 > /sys/bus/platform/devices/HID-SENSOR-
> > 2000e1.2.auto/enable_sensor
> > 2. xxd /dev/HID-SENSOR-2000e1.2.auto        # streaming reader, left
> > running
> > 3. modprobe -r intel_ishtp_hid intel_ish_ipc && modprobe
> > intel_ish_ipc
> >
> > The hub went away and was re-enumerated fine; ~30 s later CPU0
> > hard-locked in the still-running xxd
> > and the hard-lockup detector panicked the machine (log recovered from
> > EFI pstore).
> >
> > Presumably a minimal reproducer is just a blocked reader plus
> > unbinding the platform device
> > (echo HID-SENSOR-2000e1.N.auto >
> > /sys/bus/platform/drivers/hid_sensor_custom/unbind),
> > but only the module-reload path above was actually tested so far.
> >
> > Log (trimmed)
> > -------------
> >
> > [146608.877730] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting
> > for response from ISHTP device
> > [146611.885959] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting
> > for response from ISHTP device
> > [146614.893909] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting
> > for response from ISHTP device
> > [146617.901872] hid-sensor-hub 001F:8087:0AC2.0003: timeout waiting
> > for response from ISHTP device
> > [146619.972550] ish-hid {33AECD58-B679-4E54-9BD9-A04D34F0C226}:
> > [hid-ish]: enum_devices_done OK, num_hid_devices=3
> > [146620.031441] hid-sensor-hub 001F:8087:0AC2.0007: hidraw3: SENSOR
> > HUB HID v2.00 Device [hid-ishtp 8087:0AC2] on
> > [146643.503266] iwlwifi 0000:00:14.3: Queue 3 is stuck 46 70
> > [146648.924391] watchdog: CPU0: Watchdog detected hard LOCKUP on cpu
> > 0
> > [146648.924796] CPU: 0 UID: 0 PID: 1316004 Comm: xxd Tainted: G
> >    O        7.2.8 #1-NixOS PREEMPT(lazy)
> > [146648.924804] Hardware name: Dell Inc. Latitude 5420/03P5X2, BIOS
> > 1.49.0 09/01/2025
> > [146648.924808] RIP: 0010:native_queued_spin_lock_slowpath+0x64/0x2c0
> > [146648.924851] Call Trace:
> > [146648.924855]  <TASK>
> > [146648.924857]  _raw_spin_lock_irqsave+0x3d/0x50
> > [146648.924872]  finish_wait+0x3b/0x90
> > [146648.924886]  hid_sensor_custom_read+0x102/0x150
> > [hid_sensor_custom]
> > [146648.924901]  vfs_read+0xbf/0x390
> > [146648.924934]  ksys_read+0xbf/0xf0
> > [146648.924939]  do_syscall_64+0x134/0x6b0
> > [146648.925010]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > [146648.925083]  </TASK>
> > [146648.925086] Kernel panic - not syncing: Hard LOCKUP
> >
> > Also attaching a full dmesg log (with only wi-fi noise filtered out).
> >
> > Analysis (line numbers as in mainline 73bef5)
> > ---------------------------------------------
> >
> > - hid_sensor_custom_probe() allocates sensor_inst with devm_kzalloc()
> > (line 980), so it is freed right
> >   after hid_sensor_custom_remove() returns, regardless of open files.
> > - The misc device's fops reach sensor_inst through
> > container_of(file->private_data, ..., custom_dev);
> >   misc_deregister() does not revoke already-open files, so
> > read/poll/release keep dereferencing it.
> > - hid_sensor_custom_dev_if_remove() (lines 751-758) does wake_up(),
> > misc_deregister(), kfifo_free().
> >   The woken reader in hid_sensor_custom_read() (line 664) re-
> > evaluates
> >   wait_event_interruptible(sensor_inst->wait,
> > !kfifo_is_empty(&sensor_inst->data_fifo)), finds the fifo
> >   empty and goes back to sleep on sensor_inst->wait. Nothing tells it
> > the device is gone.
> > - Once sensor_inst is freed and reused, the next wakeup of that
> > reader
> > runs finish_wait() on the freed
> >   wait_queue_head: spin_lock_irqsave() on garbage, hard lockup.
> > hid_sensor_custom_release() similarly
> >   does clear_bit() on freed sensor_inst->misc_opened when the fd is
> > finally closed.
> >
> > Possible fix direction
> > ----------------------
> >
> > Tie sensor_inst's lifetime to its users rather than to devres, the
> > way
> > hidraw handles disconnect:
> > refcount it (kref: one reference for the device, one per open file;
> > kfifo_free() and kfree() on the last
> > put), add a "removed" flag set in remove() before
> > wake_up_interruptible_all(), include it in the
> > wait_event condition, and make read()/poll() return -ENODEV /
> > EPOLLHUP
> > > EPOLLERR once removed.
> >
> > The same review pointed out two more problems in this teardown path
> > that a fix would naturally cover:
> > remove() frees data_fifo before sensor_hub_remove_callback(), so a
> > sample arriving in between is
> > kfifo_in()'d into freed memory; and an enabled sensor is never
> > sensor_hub_device_close()d on remove.
> >
> > I can test patches on this machine.
> >
> > Thanks,
> > Semyon Maryasin
> >
> > P.S.: sorry for a possible duplicate, I think I messed with the
> > headers.



-- 
С уважением,
Семён Марьясин

[-- Attachment #2: test_patch.diff --]
[-- Type: text/x-patch, Size: 7671 bytes --]

diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
index c2b425a..fe67e4b 100644
--- a/drivers/hid/hid-sensor-custom.c
+++ b/drivers/hid/hid-sensor-custom.c
@@ -11,6 +11,7 @@
 #include <linux/init.h>
 #include <linux/miscdevice.h>
 #include <linux/kfifo.h>
+#include <linux/kref.h>
 #include <linux/sched.h>
 #include <linux/wait.h>
 #include <linux/poll.h>
@@ -37,6 +38,8 @@ struct hid_sensor_custom_field {
 
 struct hid_sensor_custom {
 	struct mutex mutex;
+	struct kref ref;
+	bool removed;
 	struct platform_device *pdev;
 	struct hid_sensor_hub_device *hsdev;
 	struct hid_sensor_hub_callbacks callbacks;
@@ -643,6 +646,15 @@ static void hid_sensor_custom_remove_attributes(struct hid_sensor_custom *
 	kfree(sensor_inst->fields);
 }
 
+static void hid_sensor_custom_free(struct kref *ref)
+{
+	struct hid_sensor_custom *sensor_inst =
+		container_of(ref, struct hid_sensor_custom, ref);
+
+	kfifo_free(&sensor_inst->data_fifo);
+	kfree(sensor_inst);
+}
+
 static ssize_t hid_sensor_custom_read(struct file *file, char __user *buf,
 				      size_t count, loff_t *f_ps)
 {
@@ -656,22 +668,35 @@ static ssize_t hid_sensor_custom_read(struct file *file, char __user *buf,
 	if (count < sizeof(struct hid_sensor_sample))
 		return -EINVAL;
 
-	do {
-		if (kfifo_is_empty(&sensor_inst->data_fifo)) {
-			if (file->f_flags & O_NONBLOCK)
-				return -EAGAIN;
+	for (;;) {
+		ret = mutex_lock_interruptible(&sensor_inst->mutex);
+		if (ret)
+			return ret;
 
-			ret = wait_event_interruptible(sensor_inst->wait,
-				!kfifo_is_empty(&sensor_inst->data_fifo));
-			if (ret)
-				return ret;
+		if (sensor_inst->removed) {
+			ret = -ENODEV;
+			goto unlock;
 		}
 		ret = kfifo_to_user(&sensor_inst->data_fifo, buf, count,
 				    &copied);
+		if (ret || copied)
+			goto unlock;
+		mutex_unlock(&sensor_inst->mutex);
+
+		if (file->f_flags & O_NONBLOCK)
+			return -EAGAIN;
+
+		ret = wait_event_interruptible(sensor_inst->wait,
+			READ_ONCE(sensor_inst->removed) ||
+			!kfifo_is_empty(&sensor_inst->data_fifo));
 		if (ret)
 			return ret;
+	}
 
-	} while (copied == 0);
+unlock:
+	mutex_unlock(&sensor_inst->mutex);
+	if (ret)
+		return ret;
 
 	return copied;
 }
@@ -684,6 +709,7 @@ static int hid_sensor_custom_release(struct inode *inode, struct file *file)
 				   struct hid_sensor_custom, custom_dev);
 
 	clear_bit(0, &sensor_inst->misc_opened);
+	kref_put(&sensor_inst->ref, hid_sensor_custom_free);
 
 	return 0;
 }
@@ -691,14 +717,22 @@ static int hid_sensor_custom_release(struct inode *inode, struct file *file)
 static int hid_sensor_custom_open(struct inode *inode, struct file *file)
 {
 	struct hid_sensor_custom *sensor_inst;
+	int ret;
 
 	sensor_inst = container_of(file->private_data,
 				   struct hid_sensor_custom, custom_dev);
-	/* We essentially have single reader and writer */
-	if (test_and_set_bit(0, &sensor_inst->misc_opened))
-		return -EBUSY;
+	mutex_lock(&sensor_inst->mutex);
+	if (sensor_inst->removed) {
+		ret = -ENODEV;
+	} else if (test_and_set_bit(0, &sensor_inst->misc_opened)) {
+		ret = -EBUSY;
+	} else {
+		kref_get(&sensor_inst->ref);
+		ret = stream_open(inode, file);
+	}
+	mutex_unlock(&sensor_inst->mutex);
 
-	return stream_open(inode, file);
+	return ret;
 }
 
 static __poll_t hid_sensor_custom_poll(struct file *file,
@@ -712,6 +746,9 @@ static __poll_t hid_sensor_custom_poll(struct file *file,
 
 	poll_wait(file, &sensor_inst->wait, wait);
 
+	if (READ_ONCE(sensor_inst->removed))
+		return EPOLLHUP | EPOLLERR;
+
 	if (!kfifo_is_empty(&sensor_inst->data_fifo))
 		mask = EPOLLIN | EPOLLRDNORM;
 
@@ -719,6 +756,7 @@ static __poll_t hid_sensor_custom_poll(struct file *file,
 }
 
 static const struct file_operations hid_sensor_custom_fops = {
+	.owner = THIS_MODULE,
 	.open =  hid_sensor_custom_open,
 	.read =  hid_sensor_custom_read,
 	.release = hid_sensor_custom_release,
@@ -741,20 +779,17 @@ static int hid_sensor_custom_dev_if_add(struct hid_sensor_custom *sensor_inst)
 	sensor_inst->custom_dev.name = dev_name(&sensor_inst->pdev->dev);
 	sensor_inst->custom_dev.fops = &hid_sensor_custom_fops;
 	ret = misc_register(&sensor_inst->custom_dev);
-	if (ret) {
-		kfifo_free(&sensor_inst->data_fifo);
-		return ret;
-	}
-	return 0;
+	return ret;
 }
 
 static void hid_sensor_custom_dev_if_remove(struct hid_sensor_custom
 								*sensor_inst)
 {
-	wake_up(&sensor_inst->wait);
+	mutex_lock(&sensor_inst->mutex);
+	WRITE_ONCE(sensor_inst->removed, true);
+	mutex_unlock(&sensor_inst->mutex);
+	wake_up_interruptible_all(&sensor_inst->wait);
 	misc_deregister(&sensor_inst->custom_dev);
-	kfifo_free(&sensor_inst->data_fifo);
-
 }
 
 /*
@@ -977,8 +1012,7 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
 	int ret;
 	const struct hid_sensor_custom_match *match;
 
-	sensor_inst = devm_kzalloc(&pdev->dev, sizeof(*sensor_inst),
-				   GFP_KERNEL);
+	sensor_inst = kzalloc_obj(*sensor_inst);
 	if (!sensor_inst)
 		return -ENOMEM;
 
@@ -988,6 +1022,7 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
 	sensor_inst->hsdev = hsdev;
 	sensor_inst->pdev = pdev;
 	mutex_init(&sensor_inst->mutex);
+	kref_init(&sensor_inst->ref);
 	platform_set_drvdata(pdev, sensor_inst);
 
 	ret = hid_sensor_custom_get_known(hsdev, &match);
@@ -999,7 +1034,7 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
 		if (ret) {
 			dev_err(&pdev->dev,
 				"register_platform_device failed\n");
-			return ret;
+			goto err_free;
 		}
 
 		return 0;
@@ -1009,7 +1044,7 @@ static int hid_sensor_custom_probe(struct platform_device *pdev)
 					   &sensor_inst->callbacks);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "callback reg failed\n");
-		return ret;
+		goto err_free;
 	}
 
 	ret = hid_sensor_custom_add_attributes(sensor_inst);
@@ -1034,6 +1069,8 @@ err_remove_attributes:
 	hid_sensor_custom_remove_attributes(sensor_inst);
 err_remove_callback:
 	sensor_hub_remove_callback(hsdev, hsdev->usage);
+err_free:
+	kref_put(&sensor_inst->ref, hid_sensor_custom_free);
 
 	return ret;
 }
@@ -1045,7 +1082,7 @@ static void hid_sensor_custom_remove(struct platform_device *pdev)
 
 	if (sensor_inst->custom_pdev) {
 		platform_device_unregister(sensor_inst->custom_pdev);
-		return;
+		goto out;
 	}
 
 	hid_sensor_custom_dev_if_remove(sensor_inst);
@@ -1053,7 +1090,11 @@ static void hid_sensor_custom_remove(struct platform_device *pdev)
 	sysfs_remove_group(&sensor_inst->pdev->dev.kobj,
 			   &enable_sensor_attr_group);
 	hid_sensor_custom_remove_attributes(sensor_inst);
+	if (sensor_inst->enable)
+		sensor_hub_device_close(hsdev);
 	sensor_hub_remove_callback(hsdev, hsdev->usage);
+out:
+	kref_put(&sensor_inst->ref, hid_sensor_custom_free);
 }
 
 static const struct platform_device_id hid_sensor_custom_ids[] = {
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 6470a29..f270b59 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -173,7 +173,8 @@ int sensor_hub_remove_callback(struct hid_sensor_hub_device *hsdev,
 	struct sensor_hub_data *pdata = hid_get_drvdata(hsdev->hdev);
 	unsigned long flags;
 
-	spin_lock_irqsave(&pdata->dyn_callback_lock, flags);
+	spin_lock_irqsave(&pdata->lock, flags);
+	spin_lock(&pdata->dyn_callback_lock);
 	list_for_each_entry(callback, &pdata->dyn_callback_list, list)
 		if (callback->usage_id == usage_id &&
 						callback->hsdev == hsdev) {
@@ -181,7 +182,8 @@ int sensor_hub_remove_callback(struct hid_sensor_hub_device *hsdev,
 			kfree(callback);
 			break;
 		}
-	spin_unlock_irqrestore(&pdata->dyn_callback_lock, flags);
+	spin_unlock(&pdata->dyn_callback_lock);
+	spin_unlock_irqrestore(&pdata->lock, flags);
 
 	return 0;
 }