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

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 13 Sep 2026 18:21:20 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sun, 13 Sep 2026 08:50:35 -0700
> srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> 
> > 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@vger.kernel.org
> > > Suggested-by: Jonathan Cameron <jic23@kernel.org>
> > > Assisted-by: LLM
> > > Signed-off-by: Yibo Tan <lhfff@tju.edu.cn>    
> > 
> > 
> > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>  
> 
> Applied to the fixes branch of iio.git.
> 
> It might or might not go upstream before the next merge windows. In practice
> I'm going to assume these devices are typically not hotplugable except in
> dev kits so we are unlikely to see this on production devices unless
> someone manually removes?  That does affect whether to fix it, but only how
> fast we rush it upstream!

I fluffed this again :(.  One day I'll remember which bits of this driver
stack sit in drivers/hid.

Should go via that tree not IIO. So I'll drop it.

Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>


> 
> Jonathan
> 
> > 
> >   
> > > ---
> > > 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@tju.edu.cn/
> > > 
> > >  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;    
> >   
> 
>