From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from saturn.retrosnub.co.uk ([178.18.118.26]:42757 "EHLO saturn.retrosnub.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751500AbaKEOAZ (ORCPT ); Wed, 5 Nov 2014 09:00:25 -0500 Message-ID: <545A2D76.7050800@kernel.org> Date: Wed, 05 Nov 2014 14:00:22 +0000 From: Jonathan Cameron MIME-Version: 1.0 To: Lars-Peter Clausen CC: Hartmut Knaack , Peter Meerwald , linux-iio@vger.kernel.org Subject: Re: [PATCH 1/3] staging:iio:ade7758: Fix NULL pointer deref when enabling buffer References: <1415120596-21704-1-git-send-email-lars@metafoo.de> In-Reply-To: <1415120596-21704-1-git-send-email-lars@metafoo.de> Content-Type: text/plain; charset=windows-1252 Sender: linux-iio-owner@vger.kernel.org List-Id: linux-iio@vger.kernel.org On 04/11/14 17:03, Lars-Peter Clausen wrote: > In older versions of the IIO framework it was possible to pass a completely > different set of channels to iio_buffer_register() as the one that is > assigned to the IIO device. Commit 959d2952d124 ("staging:iio: make > iio_sw_buffer_preenable much more general.") introduced a restriction that > requires that the set of channels that is passed to iio_buffer_register() is > a subset of the channels assigned to the IIO device as the IIO core will use > the list of channels that is assigned to the device to lookup a channel by > scan index in iio_compute_scan_bytes(). If it can not find the channel the > function will crash. This patch fixes the issue by making sure that the same > set of channels is assigned to the IIO device and passed to > iio_buffer_register(). > > Note that we need to remove the IIO_CHAN_INFO_RAW and IIO_CHAN_INFO_SCALE > info attributes from the channels since we don't actually want those to be > registered. > > Fixes the following crash: > Unable to handle kernel NULL pointer dereference at virtual address 00000016 > pgd = d2094000 > [00000016] *pgd=16e39831, *pte=00000000, *ppte=00000000 > Internal error: Oops: 17 [#1] PREEMPT SMP ARM > Modules linked in: > CPU: 1 PID: 1695 Comm: bash Not tainted 3.17.0-06329-g29461ee #9686 > task: d7768040 ti: d5bd4000 task.ti: d5bd4000 > PC is at iio_compute_scan_bytes+0x38/0xc0 > LR is at iio_compute_scan_bytes+0x34/0xc0 > pc : [] lr : [] psr: 60070013 > sp : d5bd5ec0 ip : 00000000 fp : 00000000 > r10: d769f934 r9 : 00000000 r8 : 00000001 > r7 : 00000000 r6 : c8fc6240 r5 : d769f800 r4 : 00000000 > r3 : d769f800 r2 : 00000000 r1 : ffffffff r0 : 00000000 > Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user > Control: 18c5387d Table: 1209404a DAC: 00000015 > Process bash (pid: 1695, stack limit = 0xd5bd4240) > Stack: (0xd5bd5ec0 to 0xd5bd6000) > 5ec0: d769f800 d7435640 c8fc6240 d769f984 00000000 c03175a4 d7435690 d7435640 > 5ee0: d769f990 00000002 00000000 d769f800 d5bd4000 00000000 000b43a8 c03177f4 > 5f00: d769f810 0162b8c8 00000002 c8fc7e00 d77f1d08 d77f1da8 c8fc7e00 c01faf1c > 5f20: 00000002 c010694c c010690c d5bd5f88 00000002 c8fc6840 c8fc684c c0105e08 > 5f40: 00000000 00000000 d20d1580 00000002 000af408 d5bd5f88 c000de84 c00b76d4 > 5f60: d20d1580 000af408 00000002 d20d1580 d20d1580 00000002 000af408 c000de84 > 5f80: 00000000 c00b7a44 00000000 00000000 00000002 b6ebea78 00000002 000af408 > 5fa0: 00000004 c000dd00 b6ebea78 00000002 00000001 000af408 00000002 00000000 > 5fc0: b6ebea78 00000002 000af408 00000004 bee96a4c 000a6094 00000000 000b43a8 > 5fe0: 00000000 bee969cc b6e2eb77 b6e6525c 40070010 00000001 00000000 00000000 > [] (iio_compute_scan_bytes) from [] (__iio_update_buffers+0x248/0x438) > [] (__iio_update_buffers) from [] (iio_buffer_store_enable+0x60/0x7c) > [] (iio_buffer_store_enable) from [] (dev_attr_store+0x18/0x24) > [] (dev_attr_store) from [] (sysfs_kf_write+0x40/0x4c) > [] (sysfs_kf_write) from [] (kernfs_fop_write+0x110/0x154) > [] (kernfs_fop_write) from [] (vfs_write+0xbc/0x170) > [] (vfs_write) from [] (SyS_write+0x40/0x78) > [] (SyS_write) from [] (ret_fast_syscall+0x0/0x30) > > Fixes: 959d2952d124 ("staging:iio: make iio_sw_buffer_preenable much more general.") > Signed-off-by: Lars-Peter Clausen Applied to the fixes-togreg branch of iio.git marked for stable. Thanks, Jonathan > --- > drivers/staging/iio/meter/ade7758.h | 1 - > drivers/staging/iio/meter/ade7758_core.c | 33 ++------------------------------ > drivers/staging/iio/meter/ade7758_ring.c | 3 +-- > 3 files changed, 3 insertions(+), 34 deletions(-) > > diff --git a/drivers/staging/iio/meter/ade7758.h b/drivers/staging/iio/meter/ade7758.h > index 0731820..e8c98cf 100644 > --- a/drivers/staging/iio/meter/ade7758.h > +++ b/drivers/staging/iio/meter/ade7758.h > @@ -119,7 +119,6 @@ struct ade7758_state { > u8 *tx; > u8 *rx; > struct mutex buf_lock; > - const struct iio_chan_spec *ade7758_ring_channels; > struct spi_transfer ring_xfer[4]; > struct spi_message ring_msg; > /* > diff --git a/drivers/staging/iio/meter/ade7758_core.c b/drivers/staging/iio/meter/ade7758_core.c > index cba183e..214b03e 100644 > --- a/drivers/staging/iio/meter/ade7758_core.c > +++ b/drivers/staging/iio/meter/ade7758_core.c > @@ -631,8 +631,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 0, > .extend_name = "raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_A, AD7758_VOLTAGE), > .scan_index = 0, > .scan_type = { > @@ -645,8 +643,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 0, > .extend_name = "raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_A, AD7758_CURRENT), > .scan_index = 1, > .scan_type = { > @@ -659,8 +655,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 0, > .extend_name = "apparent_raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_A, AD7758_APP_PWR), > .scan_index = 2, > .scan_type = { > @@ -673,8 +667,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 0, > .extend_name = "active_raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_A, AD7758_ACT_PWR), > .scan_index = 3, > .scan_type = { > @@ -687,8 +679,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 0, > .extend_name = "reactive_raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_A, AD7758_REACT_PWR), > .scan_index = 4, > .scan_type = { > @@ -701,8 +691,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 1, > .extend_name = "raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_B, AD7758_VOLTAGE), > .scan_index = 5, > .scan_type = { > @@ -715,8 +703,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 1, > .extend_name = "raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_B, AD7758_CURRENT), > .scan_index = 6, > .scan_type = { > @@ -729,8 +715,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 1, > .extend_name = "apparent_raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_B, AD7758_APP_PWR), > .scan_index = 7, > .scan_type = { > @@ -743,8 +727,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 1, > .extend_name = "active_raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_B, AD7758_ACT_PWR), > .scan_index = 8, > .scan_type = { > @@ -757,8 +739,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 1, > .extend_name = "reactive_raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_B, AD7758_REACT_PWR), > .scan_index = 9, > .scan_type = { > @@ -771,8 +751,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 2, > .extend_name = "raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_C, AD7758_VOLTAGE), > .scan_index = 10, > .scan_type = { > @@ -785,8 +763,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 2, > .extend_name = "raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_C, AD7758_CURRENT), > .scan_index = 11, > .scan_type = { > @@ -799,8 +775,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 2, > .extend_name = "apparent_raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_C, AD7758_APP_PWR), > .scan_index = 12, > .scan_type = { > @@ -813,8 +787,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 2, > .extend_name = "active_raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_C, AD7758_ACT_PWR), > .scan_index = 13, > .scan_type = { > @@ -827,8 +799,6 @@ static const struct iio_chan_spec ade7758_channels[] = { > .indexed = 1, > .channel = 2, > .extend_name = "reactive_raw", > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > .address = AD7758_WT(AD7758_PHASE_C, AD7758_REACT_PWR), > .scan_index = 14, > .scan_type = { > @@ -869,13 +839,14 @@ static int ade7758_probe(struct spi_device *spi) > goto error_free_rx; > } > st->us = spi; > - st->ade7758_ring_channels = &ade7758_channels[0]; > mutex_init(&st->buf_lock); > > indio_dev->name = spi->dev.driver->name; > indio_dev->dev.parent = &spi->dev; > indio_dev->info = &ade7758_info; > indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->channels = ade7758_channels; > + indio_dev->num_channels = ARRAY_SIZE(ade7758_channels); > > ret = ade7758_configure_ring(indio_dev); > if (ret) > diff --git a/drivers/staging/iio/meter/ade7758_ring.c b/drivers/staging/iio/meter/ade7758_ring.c > index c0accf8..628e902 100644 > --- a/drivers/staging/iio/meter/ade7758_ring.c > +++ b/drivers/staging/iio/meter/ade7758_ring.c > @@ -85,7 +85,6 @@ static irqreturn_t ade7758_trigger_handler(int irq, void *p) > **/ > static int ade7758_ring_preenable(struct iio_dev *indio_dev) > { > - struct ade7758_state *st = iio_priv(indio_dev); > unsigned channel; > > if (!bitmap_empty(indio_dev->active_scan_mask, indio_dev->masklength)) > @@ -95,7 +94,7 @@ static int ade7758_ring_preenable(struct iio_dev *indio_dev) > indio_dev->masklength); > > ade7758_write_waveform_type(&indio_dev->dev, > - st->ade7758_ring_channels[channel].address); > + indio_dev->channels[channel].address); > > return 0; > } >