linux-kernel - Re: [PATCH v2 2/2] iio: pressure: mpl115: do not rely on structure field ordering

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] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 5 Feb 2017 09:38:15 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     Peter Rosin <peda@...ntia.se>, linux-kernel@...r.kernel.org
Cc:     Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Sanchayan Maity <maitysanchayan@...il.com>,
        Gregor Boirie <gregor.boirie@...rot.com>,
        Alison Schofield <amsfield22@...il.com>,
        Ken Lin <ken.lin@...antech.com>, linux-iio@...r.kernel.org
Subject: Re: [PATCH v2 2/2] iio: pressure: mpl115: do not rely on structure
 field ordering

On 01/02/17 20:40, Peter Rosin wrote:
> Fixes a regression triggered by a change in the layout of
> struct iio_chan_spec, but the real bug is in the driver which assumed
> a specific structure layout in the first place. Hint: the three bits were
> not OR:ed together as implied by the indentation prior to this patch,
> there was a comma between the first two, which accidentally moved the
> ..._SCALE and ..._OFFSET bits to the next structure field. That field
> was .info_mask_shared_by_type before the _available attributes was added
> by commit 51239600074b ("iio:core: add a callback to allow drivers to
> provide _available attributes") and .info_mask_separate_available
> afterwards, and the regression happened.
> 
> info_mask_shared_by_type is actually a better choice than the originally
> intended info_mask_separate for the ..._SCALE and ..._OFFSET bits since
> a constant is returned from mpl115_read_raw for the scale/offset. Using
> info_mask_shared_by_type also preserves the behavior from before the
> regression and is therefore less likely to cause other interesting side
> effects.
> 
> The above mentioned regression causes unintended sysfs attibutes to
> show up that are not backed by code, in turn causing a NULL pointer
> defererence to happen on access.
> 
> Fixes: 3017d90e8931 ("iio: Add Freescale MPL115A2 pressure / temperature sensor driver")
> Fixes: 51239600074b ("iio:core: add a callback to allow drivers to provide _available attributes")
> Signed-off-by: Peter Rosin <peda@...ntia.se>
Applied to the fixes-togreg branch of iio.git.

Thanks,

Jonathan
> ---
>  drivers/iio/pressure/mpl115.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/pressure/mpl115.c b/drivers/iio/pressure/mpl115.c
> index 73f2f0c46e62..8f2bce213248 100644
> --- a/drivers/iio/pressure/mpl115.c
> +++ b/drivers/iio/pressure/mpl115.c
> @@ -137,6 +137,7 @@ static const struct iio_chan_spec mpl115_channels[] = {
>  	{
>  		.type = IIO_TEMP,
>  		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_type =
>  			BIT(IIO_CHAN_INFO_OFFSET) | BIT(IIO_CHAN_INFO_SCALE),
>  	},
>  };
>