linux-kernel - Re: [PATCH v2 4/4] staging: iio: adt7316: fix the dac write calculation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20190105175712.7e5075e1@archlinux>
Date:   Sat, 5 Jan 2019 17:57:12 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     Jeremy Fertic <jeremyfertic@...il.com>
Cc:     Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Hartmut Knaack <knaack.h@....de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-iio@...r.kernel.org, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org,
        Shreeya Patel <shreeya.patel23498@...il.com>
Subject: Re: [PATCH v2 4/4] staging: iio: adt7316: fix the dac write
 calculation

On Sat, 22 Dec 2018 21:57:43 -0700
Jeremy Fertic <jeremyfertic@...il.com> wrote:

> The lsb calculation is not masking the correct bits from the user input.
> Subtract 1 from (1 << offset) to correctly set up the mask to be applied
> to user input.
> 
> The lsb register stores its value starting at the bit 7 position.
> adt7316_store_DAC() currently assumes the value is at the other end of the
> register. Shift the lsb value before storing it in a new variable lsb_reg,
> and write this variable to the lsb register.
> 
> Fixes: 35f6b6b86ede ("staging: iio: new ADT7316/7/8 and ADT7516/7/9 driver")
> Signed-off-by: Jeremy Fertic <jeremyfertic@...il.com>
Applied to the togreg branch fo iio.git and pushed out as testing for the
autobuilders to play with it.

I cc'd Shreeya on all of these.   It would be helpful if you both cc'd
the other one on any future series. Cross review also good given you are both
working with this part!

thanks,

Jonathan

> ---
>  drivers/staging/iio/addac/adt7316.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/addac/adt7316.c b/drivers/staging/iio/addac/adt7316.c
> index d7f3d68e525b..6f7891b567b9 100644
> --- a/drivers/staging/iio/addac/adt7316.c
> +++ b/drivers/staging/iio/addac/adt7316.c
> @@ -1431,7 +1431,7 @@ static ssize_t adt7316_show_DAC(struct adt7316_chip_info *chip,
>  static ssize_t adt7316_store_DAC(struct adt7316_chip_info *chip,
>  				 int channel, const char *buf, size_t len)
>  {
> -	u8 msb, lsb, offset;
> +	u8 msb, lsb, lsb_reg, offset;
>  	u16 data;
>  	int ret;
>  
> @@ -1449,9 +1449,13 @@ static ssize_t adt7316_store_DAC(struct adt7316_chip_info *chip,
>  		return -EINVAL;
>  
>  	if (chip->dac_bits > 8) {
> -		lsb = data & (1 << offset);
> +		lsb = data & ((1 << offset) - 1);
> +		if (chip->dac_bits == 12)
> +			lsb_reg = lsb << ADT7316_DA_12_BIT_LSB_SHIFT;
> +		else
> +			lsb_reg = lsb << ADT7316_DA_10_BIT_LSB_SHIFT;
>  		ret = chip->bus.write(chip->bus.client,
> -			ADT7316_DA_DATA_BASE + channel * 2, lsb);
> +			ADT7316_DA_DATA_BASE + channel * 2, lsb_reg);
>  		if (ret)
>  			return -EIO;
>  	}