| Message ID | 20250410170408.8585-1-purvayeshi550@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] iio: adc: ad_sigma_delta: Fix use of uninitialized status_pos | expand |
On Thu, 10 Apr 2025 22:34:08 +0530 Purva Yeshi <purvayeshi550@gmail.com> wrote: > Fix Smatch-detected issue: > drivers/iio/adc/ad_sigma_delta.c:604 ad_sd_trigger_handler() error: > uninitialized symbol 'status_pos'. > > The variable `status_pos` was only initialized in specific switch cases > (1, 2, 3, 4), which could leave it uninitialized if `reg_size` had an > unexpected value. > > Fix by adding a default case to the switch block to catch unexpected > values of `reg_size`. Use `dev_err_ratelimited()` for error logging and > `goto irq_handled` instead of returning early. > > Signed-off-by: Purva Yeshi <purvayeshi550@gmail.com> Seems like reasonable hardening of the code. Applied. Thanks, > --- > V1 - https://lore.kernel.org/all/20250409200151.201327-1-purvayeshi550@gmail.com/ > V2 - Moved the reg_size validation inside the switch statement using a default case, > replaced dev_err() with dev_err_ratelimited(), and replaced return IRQ_HANDLED > with goto irq_handled; > > drivers/iio/adc/ad_sigma_delta.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c > index 6c37f8e21120..4c5f8d29a559 100644 > --- a/drivers/iio/adc/ad_sigma_delta.c > +++ b/drivers/iio/adc/ad_sigma_delta.c > @@ -587,6 +587,10 @@ static irqreturn_t ad_sd_trigger_handler(int irq, void *p) > * byte set to zero. */ > ad_sd_read_reg_raw(sigma_delta, data_reg, transfer_size, &data[1]); > break; > + > + default: > + dev_err_ratelimited(&indio_dev->dev, "Unsupported reg_size: %u\n", reg_size); > + goto irq_handled; > } > > /*
On 12/04/25 22:30, Jonathan Cameron wrote: > On Thu, 10 Apr 2025 22:34:08 +0530 > Purva Yeshi <purvayeshi550@gmail.com> wrote: > >> Fix Smatch-detected issue: >> drivers/iio/adc/ad_sigma_delta.c:604 ad_sd_trigger_handler() error: >> uninitialized symbol 'status_pos'. >> >> The variable `status_pos` was only initialized in specific switch cases >> (1, 2, 3, 4), which could leave it uninitialized if `reg_size` had an >> unexpected value. >> >> Fix by adding a default case to the switch block to catch unexpected >> values of `reg_size`. Use `dev_err_ratelimited()` for error logging and >> `goto irq_handled` instead of returning early. >> >> Signed-off-by: Purva Yeshi <purvayeshi550@gmail.com> > Seems like reasonable hardening of the code. > > Applied. > > Thanks, Hi Jonathan, Thank you for reviewing and applying the patch. > >> --- >> V1 - https://lore.kernel.org/all/20250409200151.201327-1-purvayeshi550@gmail.com/ >> V2 - Moved the reg_size validation inside the switch statement using a default case, >> replaced dev_err() with dev_err_ratelimited(), and replaced return IRQ_HANDLED >> with goto irq_handled; >> >> drivers/iio/adc/ad_sigma_delta.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c >> index 6c37f8e21120..4c5f8d29a559 100644 >> --- a/drivers/iio/adc/ad_sigma_delta.c >> +++ b/drivers/iio/adc/ad_sigma_delta.c >> @@ -587,6 +587,10 @@ static irqreturn_t ad_sd_trigger_handler(int irq, void *p) >> * byte set to zero. */ >> ad_sd_read_reg_raw(sigma_delta, data_reg, transfer_size, &data[1]); >> break; >> + >> + default: >> + dev_err_ratelimited(&indio_dev->dev, "Unsupported reg_size: %u\n", reg_size); >> + goto irq_handled; >> } >> >> /* > Best regards, Purva
diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c index 6c37f8e21120..4c5f8d29a559 100644 --- a/drivers/iio/adc/ad_sigma_delta.c +++ b/drivers/iio/adc/ad_sigma_delta.c @@ -587,6 +587,10 @@ static irqreturn_t ad_sd_trigger_handler(int irq, void *p) * byte set to zero. */ ad_sd_read_reg_raw(sigma_delta, data_reg, transfer_size, &data[1]); break; + + default: + dev_err_ratelimited(&indio_dev->dev, "Unsupported reg_size: %u\n", reg_size); + goto irq_handled; } /*
Fix Smatch-detected issue: drivers/iio/adc/ad_sigma_delta.c:604 ad_sd_trigger_handler() error: uninitialized symbol 'status_pos'. The variable `status_pos` was only initialized in specific switch cases (1, 2, 3, 4), which could leave it uninitialized if `reg_size` had an unexpected value. Fix by adding a default case to the switch block to catch unexpected values of `reg_size`. Use `dev_err_ratelimited()` for error logging and `goto irq_handled` instead of returning early. Signed-off-by: Purva Yeshi <purvayeshi550@gmail.com> --- V1 - https://lore.kernel.org/all/20250409200151.201327-1-purvayeshi550@gmail.com/ V2 - Moved the reg_size validation inside the switch statement using a default case, replaced dev_err() with dev_err_ratelimited(), and replaced return IRQ_HANDLED with goto irq_handled; drivers/iio/adc/ad_sigma_delta.c | 4 ++++ 1 file changed, 4 insertions(+)