Re: [PATCH v3] HID: sensor-hub: Fail unfinished multi-value reads on removal

[srinivas pandruvada]

On Sun, 2026-09-13 at 15:29 +0800, Yibo Tan wrote:
> sensor_hub_remove() completes pending reads after stopping the HID
> device,
> but does not record why they completed.  A successful completion wait
> therefore returns zero even if no complete input report was received.
> Multi-value IIO callers then format their untouched automatic buffer
> as a
> successful result.
> 
> With a valid four-element signed 32-bit quaternion report descriptor,
> an
> unprivileged reader received all 16 bytes of the untouched buffer. 
> Across
> 11 independent KASLR-enabled boots, four reads exposed exact pointers
> to
> dev_rot_channels or dev_sysfs_ops.  Subtracting the matching link-
> time
> symbol address recovered the kernel KASLR slide in all four cases.
> 
> The reader ran as UID/GID 65534 with no effective capabilities
> through the
> mode-0644 IIO attribute.  The test used a privileged UHID broker to
> create
> and remove the provider; it does not demonstrate unprivileged
> provider
> removal.
> 
> Mark a pending request as shut down before completing it from the
> removal
> path, and return -ENODEV from a multi-value read that observes the
> marker
> after a successful wait.  Let removal win even if a response raced
> with
> teardown, since the device is no longer available.
> 
> The Root B-only repair returned -ENODEV with no payload or kernel
> diagnostic in 3/3 matching signed-32-bit runs.  The source
> reproducer,
> complete vulnerable and fixed serial logs, result tables, and
> checksums are
> available in [1].
> 
> Link:
> https://github.com/kimaiden1984-boop/linux-kernel-poc-collections/tree/main/cases/hid-sensor-quaternion-root-b-kaslr
>  [1]
> Fixes: f784fcea4506 ("HID: sensor-hub: Add
> sensor_hub_input_attr_read_values() for multi-byte reads")
> Cc: stable@xxxxxxxxxxxxxxx
> Suggested-by: Jonathan Cameron <jic23@xxxxxxxxxx>
> Assisted-by: LLM
> Signed-off-by: Yibo Tan <lhfff@xxxxxxxxxx>


Acked-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>


> ---
> Changes in v3:
> - Replace the raw_size error sentinel with a dedicated teardown flag,
> as
>   suggested by Jonathan Cameron.
> - Let teardown win if it races with a completed response.
> 
> v2:
> https://lore.kernel.org/r/20260912050257.837340-1-lhfff@xxxxxxxxxx/
> 
>  drivers/hid/hid-sensor-hub.c   | 6 +++++-
>  include/linux/hid-sensor-hub.h | 2 ++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-
> hub.c
> index 6470a290ebfc..a9bd72218c07 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -334,6 +334,8 @@ int sensor_hub_input_attr_read_values(struct
> hid_sensor_hub_device *hsdev,
>  			ret = -ETIMEDOUT;
>  		else if (cycles < 0)
>  			ret = cycles;
> +		else if (hsdev->pending.shutdown)
> +			ret = -ENODEV;
>  
>  		hsdev->pending.status = false;
>  	}
> @@ -805,8 +807,10 @@ static int sensor_hub_finalize_pending_fn(struct
> device *dev, void *data)
>  {
>  	struct hid_sensor_hub_device *hsdev = dev->platform_data;
>  
> -	if (hsdev->pending.status)
> +	if (hsdev->pending.status) {
> +		hsdev->pending.shutdown = true;
>  		complete(&hsdev->pending.ready);
> +	}
>  
>  	return 0;
>  }
> diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-
> sensor-hub.h
> index ab5cc8db3fbb..5aecf4474183 100644
> --- a/include/linux/hid-sensor-hub.h
> +++ b/include/linux/hid-sensor-hub.h
> @@ -38,6 +38,7 @@ struct hid_sensor_hub_attribute_info {
>  /**
>   * struct sensor_hub_pending - Synchronous read pending information
>   * @status:		Pending status true/false.
> + * @shutdown:		The device is being removed.
>   * @ready:		Completion synchronization data.
>   * @usage_id:		Usage id for physical device, e.g. gyro
> usage id.
>   * @attr_usage_id:	Usage Id of a field, e.g. X-axis for a gyro.
> @@ -48,6 +49,7 @@ struct hid_sensor_hub_attribute_info {
>   */
>  struct sensor_hub_pending {
>  	bool status;
> +	bool shutdown;
>  	struct completion ready;
>  	u32 usage_id;
>  	u32 attr_usage_id;