Re: [PATCH RFC v6 5/5] iio: osf: add UART IIO driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Propagating application-level errors corrupts the UART transport stream state machine. In `osf_stream_process()`, if `osf_core_receive_frame()` returns any error, the code assumes the frame length is untrusted, drops only 1 byte, and forces a stream resynchronization. However, `osf_core_receive_frame()` returns errors for valid, successfully decoded frames under application-level failures, such as `-EOPNOTSUPP` for unsupported frame types, or errors from IIO buffer pushes (e.g., in `osf_iio_push_sample()`). Treating these application-layer errors as framing errors causes the driver to drop the first byte of a perfectly valid frame, destroying the UART parser's synchronization and causing it to discard the rest of the frame and potentially subsequent frames.
--

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

iio: osf: add UART IIO driver
    
Add the Open Sensor Fusion serdev transport, driver core, and IIO
registration path as one complete driver patch.

> diff --git a/drivers/iio/opensensorfusion/osf_core.c b/drivers/iio/opensensorfusion/osf_core.c
> new file mode 100644
> index 0000000000000..207de03dbcdef
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_core.c

[ ... ]

> +int osf_core_receive_frame(struct osf_device *osf, const u8 *buf, size_t len)
> +{
> +	struct osf_frame frame;
> +	size_t frame_len;
> +	int ret;
> +
> +	ret = osf_protocol_decode_frame(buf, len, &frame, &frame_len);
> +	if (ret)
> +		return ret;
> +
> +	if (frame_len != len)
> +		return -EMSGSIZE;
> +
> +	switch (frame.message_type) {
> +	case OSF_MSG_SENSOR_SAMPLE:
> +		return osf_core_handle_sensor_sample(osf, &frame);
> +	case OSF_MSG_DEVICE_STATUS:
> +		return osf_core_handle_device_status(osf, &frame);
> +	case OSF_MSG_CAPABILITY_REPORT:
> +		return osf_core_handle_capability_report(osf, &frame);
> +	default:
> +		if (frame.message_type >= OSF_RESERVED_MSG_FIRST &&
> +		    frame.message_type <= OSF_RESERVED_MSG_LAST)
> +			return 0;
> +		if (frame.message_type >= OSF_VENDOR_PRIVATE_FIRST)
> +			return 0;
> +		return -EOPNOTSUPP;

[Severity: High]
Will returning an application-level error like -EOPNOTSUPP here corrupt the
UART transport stream state machine?

If osf_core_receive_frame() returns an error for a validly framed but
unsupported message type, osf_stream_process() appears to treat it as a
framing error:

osf_stream_process() {
    ...
    ret = osf_core_receive_frame(stream->osf, stream->buf, frame_len);
    if (ret) {
        ...
        stream->stats.dropped_bytes++;
        osf_stream_drop_invalid_head(stream);
        continue;
    }
}

By dropping 1 byte instead of consuming the full frame_len, the transport
state loses synchronization. This will likely cause the rest of the valid
frame and potentially subsequent frames to be dropped while it attempts to
resynchronize.

Should application-layer failures (like unsupported message types or failing
IIO buffer pushes) be absorbed and return 0 here to ensure the full frame
is consumed?

> +	}
> +}

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