* [PATCH 1/2] iio: Pass scan mask as unsigned long
@ 2013-09-18 21:10 Peter Meerwald
2013-09-18 21:10 ` [PATCH 2/2] iio: Fix crash when scan_bytes is computed with active_scan_mask == NULL Peter Meerwald
2013-09-21 11:22 ` [PATCH 1/2] iio: Pass scan mask as unsigned long Jonathan Cameron
0 siblings, 2 replies; 5+ messages in thread
From: Peter Meerwald @ 2013-09-18 21:10 UTC (permalink / raw)
To: linux-iio; +Cc: Peter Meerwald
Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
drivers/iio/industrialio-buffer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 58180ec..2361fbc 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -429,8 +429,8 @@ static const unsigned long *iio_scan_mask_match(const unsigned long *av_masks,
return NULL;
}
-static int iio_compute_scan_bytes(struct iio_dev *indio_dev, const long *mask,
- bool timestamp)
+static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
+ const unsigned long *mask, bool timestamp)
{
const struct iio_chan_spec *ch;
unsigned bytes = 0;
--
1.8.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] iio: Fix crash when scan_bytes is computed with active_scan_mask == NULL
2013-09-18 21:10 [PATCH 1/2] iio: Pass scan mask as unsigned long Peter Meerwald
@ 2013-09-18 21:10 ` Peter Meerwald
2013-09-19 6:48 ` Lars-Peter Clausen
2013-09-21 11:22 ` [PATCH 1/2] iio: Pass scan mask as unsigned long Jonathan Cameron
1 sibling, 1 reply; 5+ messages in thread
From: Peter Meerwald @ 2013-09-18 21:10 UTC (permalink / raw)
To: linux-iio; +Cc: Peter Meerwald
if device has available_scan_masks set and the buffer is enabled without
any scan_elements enabled, in a NULL pointer is dereferenced in iio_compute_scan_bytes()
[ 18.993713] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[ 19.002593] pgd = debd4000
[ 19.005432] [00000000] *pgd=9ebc0831, *pte=00000000, *ppte=00000000
[ 19.012329] Internal error: Oops: 17 [#1] PREEMPT ARM
[ 19.017639] Modules linked in:
[ 19.020843] CPU: 0 Not tainted (3.9.11-00036-g75c888a-dirty #207)
[ 19.027587] PC is at _find_first_bit_le+0xc/0x2c
[ 19.032440] LR is at iio_compute_scan_bytes+0x2c/0xf4
[ 19.037719] pc : [<c021dc60>] lr : [<c03198d0>] psr: 200d0013
[ 19.037719] sp : debd9ed0 ip : 00000000 fp : 000802bc
[ 19.049713] r10: 00000000 r9 : 00000000 r8 : deb67250
[ 19.055206] r7 : 00000000 r6 : 00000000 r5 : 00000000 r4 : deb67000
[ 19.062011] r3 : de96ec00 r2 : 00000000 r1 : 00000004 r0 : 00000000
[ 19.068847] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 19.076324] Control: 10c5387d Table: 9ebd4019 DAC: 00000015
problem is the rollback code in iio_update_buffers(), old_mask may be NULL (e.g. on first
call)
I'm not too confident about the fix; works for me...
Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
---
drivers/iio/industrialio-buffer.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 2361fbc..d5754b8 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -522,8 +522,15 @@ int iio_update_buffers(struct iio_dev *indio_dev,
* Note can only occur when adding a buffer.
*/
list_del_init(&insert_buffer->buffer_list);
- indio_dev->active_scan_mask = old_mask;
- success = -EINVAL;
+ if (old_mask) {
+ indio_dev->active_scan_mask = old_mask;
+ success = -EINVAL;
+ }
+ else {
+ kfree(compound_mask);
+ ret = -EINVAL;
+ goto error_ret;
+ }
}
} else {
indio_dev->active_scan_mask = compound_mask;
--
1.8.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] iio: Fix crash when scan_bytes is computed with active_scan_mask == NULL
2013-09-18 21:10 ` [PATCH 2/2] iio: Fix crash when scan_bytes is computed with active_scan_mask == NULL Peter Meerwald
@ 2013-09-19 6:48 ` Lars-Peter Clausen
2013-09-21 11:32 ` Jonathan Cameron
0 siblings, 1 reply; 5+ messages in thread
From: Lars-Peter Clausen @ 2013-09-19 6:48 UTC (permalink / raw)
To: Peter Meerwald; +Cc: linux-iio
On 09/18/2013 11:10 PM, Peter Meerwald wrote:
> if device has available_scan_masks set and the buffer is enabled without
> any scan_elements enabled, in a NULL pointer is dereferenced in iio_compute_scan_bytes()
>
> [ 18.993713] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [ 19.002593] pgd = debd4000
> [ 19.005432] [00000000] *pgd=9ebc0831, *pte=00000000, *ppte=00000000
> [ 19.012329] Internal error: Oops: 17 [#1] PREEMPT ARM
> [ 19.017639] Modules linked in:
> [ 19.020843] CPU: 0 Not tainted (3.9.11-00036-g75c888a-dirty #207)
> [ 19.027587] PC is at _find_first_bit_le+0xc/0x2c
> [ 19.032440] LR is at iio_compute_scan_bytes+0x2c/0xf4
> [ 19.037719] pc : [<c021dc60>] lr : [<c03198d0>] psr: 200d0013
> [ 19.037719] sp : debd9ed0 ip : 00000000 fp : 000802bc
> [ 19.049713] r10: 00000000 r9 : 00000000 r8 : deb67250
> [ 19.055206] r7 : 00000000 r6 : 00000000 r5 : 00000000 r4 : deb67000
> [ 19.062011] r3 : de96ec00 r2 : 00000000 r1 : 00000004 r0 : 00000000
> [ 19.068847] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 19.076324] Control: 10c5387d Table: 9ebd4019 DAC: 00000015
>
> problem is the rollback code in iio_update_buffers(), old_mask may be NULL (e.g. on first
> call)
>
> I'm not too confident about the fix; works for me...
Looks good. We should probably try to restructure the function at some point as
it is quite hard to follow as it is right now.
Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> ---
> drivers/iio/industrialio-buffer.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 2361fbc..d5754b8 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -522,8 +522,15 @@ int iio_update_buffers(struct iio_dev *indio_dev,
> * Note can only occur when adding a buffer.
> */
> list_del_init(&insert_buffer->buffer_list);
> - indio_dev->active_scan_mask = old_mask;
> - success = -EINVAL;
> + if (old_mask) {
> + indio_dev->active_scan_mask = old_mask;
> + success = -EINVAL;
> + }
> + else {
> + kfree(compound_mask);
> + ret = -EINVAL;
> + goto error_ret;
> + }
> }
> } else {
> indio_dev->active_scan_mask = compound_mask;
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] iio: Fix crash when scan_bytes is computed with active_scan_mask == NULL
2013-09-19 6:48 ` Lars-Peter Clausen
@ 2013-09-21 11:32 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2013-09-21 11:32 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: Peter Meerwald, linux-iio
On 09/19/13 07:48, Lars-Peter Clausen wrote:
> On 09/18/2013 11:10 PM, Peter Meerwald wrote:
>> if device has available_scan_masks set and the buffer is enabled without
>> any scan_elements enabled, in a NULL pointer is dereferenced in iio_compute_scan_bytes()
>>
>> [ 18.993713] Unable to handle kernel NULL pointer dereference at virtual address 00000000
>> [ 19.002593] pgd = debd4000
>> [ 19.005432] [00000000] *pgd=9ebc0831, *pte=00000000, *ppte=00000000
>> [ 19.012329] Internal error: Oops: 17 [#1] PREEMPT ARM
>> [ 19.017639] Modules linked in:
>> [ 19.020843] CPU: 0 Not tainted (3.9.11-00036-g75c888a-dirty #207)
>> [ 19.027587] PC is at _find_first_bit_le+0xc/0x2c
>> [ 19.032440] LR is at iio_compute_scan_bytes+0x2c/0xf4
>> [ 19.037719] pc : [<c021dc60>] lr : [<c03198d0>] psr: 200d0013
>> [ 19.037719] sp : debd9ed0 ip : 00000000 fp : 000802bc
>> [ 19.049713] r10: 00000000 r9 : 00000000 r8 : deb67250
>> [ 19.055206] r7 : 00000000 r6 : 00000000 r5 : 00000000 r4 : deb67000
>> [ 19.062011] r3 : de96ec00 r2 : 00000000 r1 : 00000004 r0 : 00000000
>> [ 19.068847] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
>> [ 19.076324] Control: 10c5387d Table: 9ebd4019 DAC: 00000015
>>
>> problem is the rollback code in iio_update_buffers(), old_mask may be NULL (e.g. on first
>> call)
>>
>> I'm not too confident about the fix; works for me...
>
> Looks good. We should probably try to restructure the function at some point as it is quite hard to follow as it is
> right now.
>
> Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>
>
I've back ported this fix to the current fixes-togreg branch of iio.git and
applied. It will cause some merge grief so I'll try and remember to warn
Greg about that.
I'll probably apply at least some of Lars' fixes there as well so there might be
quite a bit of merge grief unfortunately.
What fun :)
Thanks,
Jonathan
>>
>> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
>> ---
>> drivers/iio/industrialio-buffer.c | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
>> index 2361fbc..d5754b8 100644
>> --- a/drivers/iio/industrialio-buffer.c
>> +++ b/drivers/iio/industrialio-buffer.c
>> @@ -522,8 +522,15 @@ int iio_update_buffers(struct iio_dev *indio_dev,
>> * Note can only occur when adding a buffer.
>> */
>> list_del_init(&insert_buffer->buffer_list);
>> - indio_dev->active_scan_mask = old_mask;
>> - success = -EINVAL;
>> + if (old_mask) {
>> + indio_dev->active_scan_mask = old_mask;
>> + success = -EINVAL;
>> + }
>> + else {
>> + kfree(compound_mask);
>> + ret = -EINVAL;
>> + goto error_ret;
>> + }
>> }
>> } else {
>> indio_dev->active_scan_mask = compound_mask;
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] iio: Pass scan mask as unsigned long
2013-09-18 21:10 [PATCH 1/2] iio: Pass scan mask as unsigned long Peter Meerwald
2013-09-18 21:10 ` [PATCH 2/2] iio: Fix crash when scan_bytes is computed with active_scan_mask == NULL Peter Meerwald
@ 2013-09-21 11:22 ` Jonathan Cameron
1 sibling, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2013-09-21 11:22 UTC (permalink / raw)
To: Peter Meerwald; +Cc: linux-iio
On 09/18/13 22:10, Peter Meerwald wrote:
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Applied to the togreg branch of iio.git
Thanks,
> ---
> drivers/iio/industrialio-buffer.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> index 58180ec..2361fbc 100644
> --- a/drivers/iio/industrialio-buffer.c
> +++ b/drivers/iio/industrialio-buffer.c
> @@ -429,8 +429,8 @@ static const unsigned long *iio_scan_mask_match(const unsigned long *av_masks,
> return NULL;
> }
>
> -static int iio_compute_scan_bytes(struct iio_dev *indio_dev, const long *mask,
> - bool timestamp)
> +static int iio_compute_scan_bytes(struct iio_dev *indio_dev,
> + const unsigned long *mask, bool timestamp)
> {
> const struct iio_chan_spec *ch;
> unsigned bytes = 0;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-09-21 10:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-18 21:10 [PATCH 1/2] iio: Pass scan mask as unsigned long Peter Meerwald
2013-09-18 21:10 ` [PATCH 2/2] iio: Fix crash when scan_bytes is computed with active_scan_mask == NULL Peter Meerwald
2013-09-19 6:48 ` Lars-Peter Clausen
2013-09-21 11:32 ` Jonathan Cameron
2013-09-21 11:22 ` [PATCH 1/2] iio: Pass scan mask as unsigned long 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).