[PATCH 0/3] iio: mxs-lradc fixes

[PATCH 0/3] iio: mxs-lradc fixes

[Alexandre Belloni]

Here are a few fixes to get in before 3.14.

Two of those are because I wasn't careful engouh when reordering my patch before
thos of Hector, it made me drop a part. and I forgot to reintroduce it when
rebasing.

The first patch fixes a buffer overflow found by smatch.

The second patch doesn't really have any impact as we control the input but is
there for correctness.

The last one removes useless scale_available files for channel 8 and 9 as they
are read has a temperature channel. I feel it is safe to do as no releases were
made with those so we are not really breaking the ABI.

Alexandre Belloni (3):
  iio: mxs-lradc: fix buffer overflow
  iio: mxs-lradc: fix invalid channel number detection
  iio: mxs-lradc: remove useless scale_available files

 drivers/staging/iio/adc/mxs-lradc.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

-- 
1.8.3.2

[PATCH 1/3] iio: mxs-lradc: fix buffer overflow

[Alexandre Belloni]

Fixes:
drivers/staging/iio/adc/mxs-lradc.c:1556 mxs_lradc_probe() error: buffer
overflow 'iio->channels' 15 <= 15

The reported available scales for in_voltage15 were also wrong.

The realbits lookup is not necessary as all the channels of the LRADC have the
same resolution, use LRADC_RESOLUTION instead.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/staging/iio/adc/mxs-lradc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index df71669bb60e..aa86849daeba 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -1613,7 +1613,7 @@ static int mxs_lradc_probe(struct platform_device *pdev)
 			 * of the array.
 			 */
 			scale_uv = ((u64)lradc->vref_mv[i] * 100000000) >>
-				   (iio->channels[i].scan_type.realbits - s);
+				   (LRADC_RESOLUTION - s);
 			lradc->scale_avail[i][s].nano =
 					do_div(scale_uv, 100000000) * 10;
 			lradc->scale_avail[i][s].integer = scale_uv;
-- 
1.8.3.2

[PATCH 2/3] iio: mxs-lradc: fix invalid channel number detection

[Alexandre Belloni]

16 would be accepted as a channel number but it is invalid. It doesn't really
have any effect as mxs_lradc_read_raw is called from a "controlled" environment
so it it only gets values going from 0 to 15.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/staging/iio/adc/mxs-lradc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index aa86849daeba..2289dc1bd928 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -898,7 +898,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 	struct mxs_lradc *lradc = iio_priv(iio_dev);
 
 	/* Check for invalid channel */
-	if (chan->channel > LRADC_MAX_TOTAL_CHANS)
+	if (chan->channel >= LRADC_MAX_TOTAL_CHANS)
 		return -EINVAL;
 
 	switch (m) {
-- 
1.8.3.2

[PATCH 3/3] iio: mxs-lradc: remove useless scale_available files

[Alexandre Belloni]

in_voltage8_scale_available and in_voltage9_scale_available are exposed to
userspace but useless as in_voltage8_raw and in_voltage9_raw are not available.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/staging/iio/adc/mxs-lradc.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index 2289dc1bd928..cfd95d825702 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -1035,8 +1035,6 @@ SHOW_SCALE_AVAILABLE_ATTR(4);
 SHOW_SCALE_AVAILABLE_ATTR(5);
 SHOW_SCALE_AVAILABLE_ATTR(6);
 SHOW_SCALE_AVAILABLE_ATTR(7);
-SHOW_SCALE_AVAILABLE_ATTR(8);
-SHOW_SCALE_AVAILABLE_ATTR(9);
 SHOW_SCALE_AVAILABLE_ATTR(10);
 SHOW_SCALE_AVAILABLE_ATTR(11);
 SHOW_SCALE_AVAILABLE_ATTR(12);
@@ -1053,8 +1051,6 @@ static struct attribute *mxs_lradc_attributes[] = {
 	&iio_dev_attr_in_voltage5_scale_available.dev_attr.attr,
 	&iio_dev_attr_in_voltage6_scale_available.dev_attr.attr,
 	&iio_dev_attr_in_voltage7_scale_available.dev_attr.attr,
-	&iio_dev_attr_in_voltage8_scale_available.dev_attr.attr,
-	&iio_dev_attr_in_voltage9_scale_available.dev_attr.attr,
 	&iio_dev_attr_in_voltage10_scale_available.dev_attr.attr,
 	&iio_dev_attr_in_voltage11_scale_available.dev_attr.attr,
 	&iio_dev_attr_in_voltage12_scale_available.dev_attr.attr,
-- 
1.8.3.2

Re: [PATCH 2/3] iio: mxs-lradc: fix invalid channel number detection

[Marek Vasut]

On Monday, January 13, 2014 at 05:02:02 PM, Alexandre Belloni wrote:
> 16 would be accepted as a channel number but it is invalid. It doesn't
> really have any effect as mxs_lradc_read_raw is called from a "controlled"
> environment so it it only gets values going from 0 to 15.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

Why don't you remove the check entirely then ?

Best regards,
Marek Vasut

Re: [PATCH 2/3] iio: mxs-lradc: fix invalid channel number detection

[Alexandre Belloni]

On 13/01/2014 21:25, Marek Vasut wrote:
> On Monday, January 13, 2014 at 05:02:02 PM, Alexandre Belloni wrote:
>> 16 would be accepted as a channel number but it is invalid. It doesn't
>> really have any effect as mxs_lradc_read_raw is called from a "controlled"
>> environment so it it only gets values going from 0 to 15.
>>
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> 
> Why don't you remove the check entirely then ?
> 

I'm not quite sure the inkernel API is sanitizing the input correctly
but maybe I didn't check enough. Maybe Jonathan can comment ?

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 2/3] iio: mxs-lradc: fix invalid channel number detection

[Jonathan Cameron]

On 13/01/14 23:43, Alexandre Belloni wrote:
> On 13/01/2014 21:25, Marek Vasut wrote:
>> On Monday, January 13, 2014 at 05:02:02 PM, Alexandre Belloni wrote:
>>> 16 would be accepted as a channel number but it is invalid. It doesn't
>>> really have any effect as mxs_lradc_read_raw is called from a "controlled"
>>> environment so it it only gets values going from 0 to 15.
>>>
>>> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>>
>> Why don't you remove the check entirely then ?
>>
>
> I'm not quite sure the inkernel API is sanitizing the input correctly
> but maybe I didn't check enough. Maybe Jonathan can comment ?
>
Unless we have a bug (more than possible as this stuff isn't heavily used
yet), it should be impossible to get the required reference to a channel
that doesn't exist.  Thus I don't 'think' the check is needed.  Feel free
to write a test case to prove me wrong ;)

Jonathan

Re: [PATCH 1/3] iio: mxs-lradc: fix buffer overflow

[Jonathan Cameron]

On 13/01/14 16:02, Alexandre Belloni wrote:
> Fixes:
> drivers/staging/iio/adc/mxs-lradc.c:1556 mxs_lradc_probe() error: buffer
> overflow 'iio->channels' 15 <= 15
>
> The reported available scales for in_voltage15 were also wrong.
>
> The realbits lookup is not necessary as all the channels of the LRADC have the
> same resolution, use LRADC_RESOLUTION instead.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Applied to the fixes-for-3.14new branch of iio.git.  This will go upstream
after 3.14-rc1 is tagged.

It's a little clunky having a simple array some of which isn't used for these
but I guess it does give fairly simple code.

Thanks,
> ---
>   drivers/staging/iio/adc/mxs-lradc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index df71669bb60e..aa86849daeba 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -1613,7 +1613,7 @@ static int mxs_lradc_probe(struct platform_device *pdev)
>   			 * of the array.
>   			 */
>   			scale_uv = ((u64)lradc->vref_mv[i] * 100000000) >>
> -				   (iio->channels[i].scan_type.realbits - s);
> +				   (LRADC_RESOLUTION - s);
>   			lradc->scale_avail[i][s].nano =
>   					do_div(scale_uv, 100000000) * 10;
>   			lradc->scale_avail[i][s].integer = scale_uv;
>

Re: [PATCH 3/3] iio: mxs-lradc: remove useless scale_available files

[Jonathan Cameron]

On 13/01/14 16:02, Alexandre Belloni wrote:
> in_voltage8_scale_available and in_voltage9_scale_available are exposed to
> userspace but useless as in_voltage8_raw and in_voltage9_raw are not available.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Applied to the fixes-for-3.14new branch of iio.git

Thanks,
> ---
>   drivers/staging/iio/adc/mxs-lradc.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index 2289dc1bd928..cfd95d825702 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -1035,8 +1035,6 @@ SHOW_SCALE_AVAILABLE_ATTR(4);
>   SHOW_SCALE_AVAILABLE_ATTR(5);
>   SHOW_SCALE_AVAILABLE_ATTR(6);
>   SHOW_SCALE_AVAILABLE_ATTR(7);
> -SHOW_SCALE_AVAILABLE_ATTR(8);
> -SHOW_SCALE_AVAILABLE_ATTR(9);
>   SHOW_SCALE_AVAILABLE_ATTR(10);
>   SHOW_SCALE_AVAILABLE_ATTR(11);
>   SHOW_SCALE_AVAILABLE_ATTR(12);
> @@ -1053,8 +1051,6 @@ static struct attribute *mxs_lradc_attributes[] = {
>   	&iio_dev_attr_in_voltage5_scale_available.dev_attr.attr,
>   	&iio_dev_attr_in_voltage6_scale_available.dev_attr.attr,
>   	&iio_dev_attr_in_voltage7_scale_available.dev_attr.attr,
> -	&iio_dev_attr_in_voltage8_scale_available.dev_attr.attr,
> -	&iio_dev_attr_in_voltage9_scale_available.dev_attr.attr,
>   	&iio_dev_attr_in_voltage10_scale_available.dev_attr.attr,
>   	&iio_dev_attr_in_voltage11_scale_available.dev_attr.attr,
>   	&iio_dev_attr_in_voltage12_scale_available.dev_attr.attr,
>