* [PATCH] iio: adc: imx25-gcq: fix uninitialized variable usage
@ 2019-09-30 19:53 Yizhuo
2019-10-01 8:47 ` Jonathan Cameron
0 siblings, 1 reply; 2+ messages in thread
From: Yizhuo @ 2019-09-30 19:53 UTC (permalink / raw)
Cc: csong, zhiyunq, Yizhuo, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Enrico Weigelt, Kate Stewart, Stephen Boyd,
Thomas Gleixner, linux-iio, linux-arm-kernel, linux-kernel
In function mx25_gcq_irq(), local variable "stats" could
be uninitialized if function regmap_read() returns -EINVAL.
However, this value is used in if statement, which is
potentially unsafe. The same case applied to the variable
"data" in function mx25_gcq_get_raw_value() in the same file.
Signed-off-by: Yizhuo <yzhai003@ucr.edu>
---
drivers/iio/adc/fsl-imx25-gcq.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/fsl-imx25-gcq.c b/drivers/iio/adc/fsl-imx25-gcq.c
index fa71489195c6..3b1e12b7c1ac 100644
--- a/drivers/iio/adc/fsl-imx25-gcq.c
+++ b/drivers/iio/adc/fsl-imx25-gcq.c
@@ -73,8 +73,12 @@ static irqreturn_t mx25_gcq_irq(int irq, void *data)
{
struct mx25_gcq_priv *priv = data;
u32 stats;
+ int ret;
- regmap_read(priv->regs, MX25_ADCQ_SR, &stats);
+ ret = regmap_read(priv->regs, MX25_ADCQ_SR, &stats);
+ if (ret) {
+ return ret;
+ }
if (stats & MX25_ADCQ_SR_EOQ) {
regmap_update_bits(priv->regs, MX25_ADCQ_MR,
@@ -100,6 +104,7 @@ static int mx25_gcq_get_raw_value(struct device *dev,
{
long timeout;
u32 data;
+ int ret;
/* Setup the configuration we want to use */
regmap_write(priv->regs, MX25_ADCQ_ITEM_7_0,
@@ -121,7 +126,11 @@ static int mx25_gcq_get_raw_value(struct device *dev,
return -ETIMEDOUT;
}
- regmap_read(priv->regs, MX25_ADCQ_FIFO, &data);
+ ret = regmap_read(priv->regs, MX25_ADCQ_FIFO, &data);
+ if (ret) {
+ dev_err(dev, "Failed to read MX25_ADCQ_FIFO.\n");
+ return ret;
+ }
*val = MX25_ADCQ_FIFO_DATA(data);
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] iio: adc: imx25-gcq: fix uninitialized variable usage
2019-09-30 19:53 [PATCH] iio: adc: imx25-gcq: fix uninitialized variable usage Yizhuo
@ 2019-10-01 8:47 ` Jonathan Cameron
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Cameron @ 2019-10-01 8:47 UTC (permalink / raw)
To: Yizhuo
Cc: csong, zhiyunq, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
Enrico Weigelt, Kate Stewart, Stephen Boyd, Thomas Gleixner,
linux-iio, linux-arm-kernel, linux-kernel
On Mon, 30 Sep 2019 12:53:54 -0700
Yizhuo <yzhai003@ucr.edu> wrote:
> In function mx25_gcq_irq(), local variable "stats" could
> be uninitialized if function regmap_read() returns -EINVAL.
> However, this value is used in if statement, which is
> potentially unsafe. The same case applied to the variable
> "data" in function mx25_gcq_get_raw_value() in the same file.
>
> Signed-off-by: Yizhuo <yzhai003@ucr.edu>
Following similar logic to the other patch I just reviewed
for the stm32-timer-trigger, lets chase if this can happen.
In this case a clock is not provided during the regmap iomem register
and as such, the call can't actually fail.
So this one is more of a tidy up and hardening against future
problems if the code changes, than an actual fix.
Worth having, but perhaps remove the word fix from the description
unless you can find a path I've missed in which this might actually
happen as the code currently is.
One minor comment inline,
Thanks,
Jonathan
> ---
> drivers/iio/adc/fsl-imx25-gcq.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/fsl-imx25-gcq.c b/drivers/iio/adc/fsl-imx25-gcq.c
> index fa71489195c6..3b1e12b7c1ac 100644
> --- a/drivers/iio/adc/fsl-imx25-gcq.c
> +++ b/drivers/iio/adc/fsl-imx25-gcq.c
> @@ -73,8 +73,12 @@ static irqreturn_t mx25_gcq_irq(int irq, void *data)
> {
> struct mx25_gcq_priv *priv = data;
> u32 stats;
> + int ret;
>
> - regmap_read(priv->regs, MX25_ADCQ_SR, &stats);
> + ret = regmap_read(priv->regs, MX25_ADCQ_SR, &stats);
> + if (ret) {
No brackets around a single line block like this.
> + return ret;
> + }
>
> if (stats & MX25_ADCQ_SR_EOQ) {
> regmap_update_bits(priv->regs, MX25_ADCQ_MR,
> @@ -100,6 +104,7 @@ static int mx25_gcq_get_raw_value(struct device *dev,
> {
> long timeout;
> u32 data;
> + int ret;
>
> /* Setup the configuration we want to use */
> regmap_write(priv->regs, MX25_ADCQ_ITEM_7_0,
> @@ -121,7 +126,11 @@ static int mx25_gcq_get_raw_value(struct device *dev,
> return -ETIMEDOUT;
> }
>
> - regmap_read(priv->regs, MX25_ADCQ_FIFO, &data);
> + ret = regmap_read(priv->regs, MX25_ADCQ_FIFO, &data);
> + if (ret) {
> + dev_err(dev, "Failed to read MX25_ADCQ_FIFO.\n");
> + return ret;
> + }
>
> *val = MX25_ADCQ_FIFO_DATA(data);
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-10-01 8:47 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-30 19:53 [PATCH] iio: adc: imx25-gcq: fix uninitialized variable usage Yizhuo
2019-10-01 8:47 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).