Re: [PATCH RFC v5 6/6] iio: osf: register IIO devices from capabilities

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing synchronization between asynchronous IIO buffer push and buffer enable/disable operations, leading to list corruption.
- [High] Uninitialized padding bytes in the IIO bounce buffer leak kernel heap memory to user-space.
--

commit e42cf74aa62314d9b92e389435a75cfcd58b0820
Author: Jinseob Kim <kimjinseob88@gmail.com>

iio: osf: register IIO devices from capabilities

Register IIO devices for supported Open Sensor Fusion capability entries
and push received samples into IIO buffers when enabled.

> diff --git a/drivers/iio/opensensorfusion/osf_iio.c b/drivers/iio/opensensorfusion/osf_iio.c
> new file mode 100644
> index 0000000000000..862a797f4fb84
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_iio.c

[ ... ]

> +int osf_iio_push_sample(struct iio_dev *indio_dev, const s32 *values,
> +			unsigned int channel_count)
> +{
> +	struct osf_iio_state *state = iio_priv(indio_dev);
> +	s64 timestamp;
> +
> +	if (channel_count != state->spec->channel_count)
> +		return -EPROTO;
> +
> +	/* This is only a fast path; IIO rechecks buffer state while pushing. */
> +	if (!iio_buffer_enabled(indio_dev))

[Severity: High]
Is there a race condition between this asynchronous IIO buffer push and
buffer enable/disable operations?

The IIO core's iio_push_to_buffers() function iterates over the buffer_list
without locks, expecting the driver to guarantee no pushes occur concurrently
with buffer state changes. 

Since this driver doesn't implement IIO setup_ops (like predisable) to stop
incoming frames, or use a software lock, could a concurrent write to the
sysfs enable attribute invoke iio_disable_buffers() and modify the
buffer_list while the UART RX path is traversing it, causing list corruption?

> +		return 0;
> +
> +	timestamp = iio_get_time_ns(indio_dev);
> +
> +	return iio_push_to_buffers_with_ts_unaligned(indio_dev, values,
> +						     channel_count * sizeof(*values),
> +						     timestamp);

[Severity: High]
Does this leak uninitialized kernel heap memory to user-space through the
bounce buffer padding?

The data payload size passed here is exactly the size of the channels
without padding. The IIO core computes scan_bytes to include padding for the
64-bit timestamp to ensure alignment, and allocates the internal bounce
buffer using devm_krealloc() without the __GFP_ZERO flag.

Because the IIO core only copies the exact data payload size and inserts the
timestamp at the aligned offset, it leaves the intermediate padding bytes
completely uninitialized. When this bounce buffer is pushed into the KFIFO,
will it leak uninitialized memory with every sample?

Would it be safer to assemble the sample in a zero-initialized padded struct
on the stack and use iio_push_to_buffers_with_timestamp() instead?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616072242.3942-1-kimjinseob88@gmail.com?part=6