Re: [RFC PATCH 3/3] iio: proximity: srf04: Add Cytron HC-SR04 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unchecked return value from `device_get_match_data()` can cause a NULL pointer dereference.
- [High] Interrupt handler may miss the rising edge of the echo pulse due to interrupt latency.
--

--- Patch [3]: [RFC PATCH 3/3] iio: proximity: srf04: Add Cytron HC-SR04 support ---
commit e27499a683a294e308b2d89b6fa06d4fd87a1cef
Author: Paweł Wiśniewski <pawel.wis.me@gmail.com>

iio: proximity: srf04: Add Cytron HC-SR04 support

This commit adds support for the Cytron HC-SR04 sensor to the srf04 driver.
It introduces the new compatible string "cytron,hc-sr04" and updates the
Kconfig to list the supported device.

> diff --git a/drivers/iio/proximity/srf04.c b/drivers/iio/proximity/srf04.c
> index 7be50bdebfcbc..f5ee7d6572b5b 100644
> --- a/drivers/iio/proximity/srf04.c
> +++ b/drivers/iio/proximity/srf04.c

[ ... ]

> @@ -231,6 +231,7 @@ static const struct iio_chan_spec srf04_chan_spec[] = {
>  };
>  
>  static const struct of_device_id of_srf04_match[] = {
> +	{ .compatible = "cytron,hc-sr04", .data = &srf04_cfg },

[Severity: High]
This is a pre-existing issue, and was not introduced by the patch under
review, but could the unchecked return value from device_get_match_data()
cause a NULL pointer dereference?

In drivers/iio/proximity/srf04.c:srf04_probe(), the return value is assigned
without validation:

    data->cfg = device_get_match_data(dev);

If the driver is instantiated via legacy platform device registration,
device_get_match_data() will return NULL. Later, in srf04_read(), this could
be dereferenced:

    udelay(data->cfg->trigger_pulse_us);

Can this result in a kernel panic when a user reads from the IIO device?


[Severity: High]
This is a pre-existing issue, and was not introduced by the patch under
review, but could the interrupt handler miss the rising edge of the echo
pulse due to interrupt latency?

In drivers/iio/proximity/srf04.c:srf04_handle_irq(), the handler relies on
the live GPIO state:

    if (gpiod_get_value(data->gpiod_echo)) {

If an object is close, the echo pulse is very short. If the system's
interrupt latency exceeds this duration, the GPIO will already have
transitioned back to low by the time the handler executes for the rising
edge. The handler will read low, complete the falling completion, and
ignore the rising edge.

This would cause the read operation to time out and fail in srf04_read():

    ret = wait_for_completion_killable_timeout(&data->rising, HZ/50);

Could this cause the sensor to consistently fail to measure short distances
and return -ETIMEDOUT to userspace?

>  	{ .compatible = "devantech,srf04", .data = &srf04_cfg },
>  	{ .compatible = "maxbotix,mb1000", .data = &mb_lv_cfg },

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784221074.git.pawel.wis.me@gmail.com?part=3