* Patch to solve NULL pointer dereference in physmap_of.c
@ 2012-11-09 7:45 Prins Anton (ST-CO/ENG1.1)
2012-11-21 7:42 ` Artem Bityutskiy
0 siblings, 1 reply; 10+ messages in thread
From: Prins Anton (ST-CO/ENG1.1) @ 2012-11-09 7:45 UTC (permalink / raw)
To: linux-mtd@lists.infradead.org
commit 0905a6f4aec377123e94d2260f2f7a0d867e19be
Author: Anton Prins <anton.prins@nl.bosch.com>
Date: Fri Nov 9 10:12:58 2012 +0100
Correct error checking to prevent a NULL pointer dereference
The problem only occurs if the DTS is not correct, the requested mapping is not reserved on the parent bus.
In this special case the count is 1, but the list_size after mapping is 0. list_size 0 should generate an error!
diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
index 2e6fb68..83d121e 100644
--- a/drivers/mtd/maps/physmap_of.c
+++ b/drivers/mtd/maps/physmap_of.c
@@ -267,13 +267,14 @@ static int __devinit of_flash_probe(struct platform_device *dev)
info->list[i].mtd->dev.parent = &dev->dev;
}
- err = 0;
if (info->list_size == 1) {
+ err = 0;
info->cmtd = info->list[0].mtd;
} else if (info->list_size > 1) {
/*
* We detected multiple devices. Concatenate them together.
*/
+ err = 0;
info->cmtd = mtd_concat_create(mtd_list, info->list_size,
dev_name(&dev->dev));
if (info->cmtd == NULL)
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: Patch to solve NULL pointer dereference in physmap_of.c
2012-11-09 7:45 Patch to solve NULL pointer dereference in physmap_of.c Prins Anton (ST-CO/ENG1.1)
@ 2012-11-21 7:42 ` Artem Bityutskiy
2012-11-21 8:43 ` Prins Anton (ST-CO/ENG1.1)
2012-11-22 15:20 ` Prins Anton (ST-CO/ENG1.1)
0 siblings, 2 replies; 10+ messages in thread
From: Artem Bityutskiy @ 2012-11-21 7:42 UTC (permalink / raw)
To: Prins Anton (ST-CO/ENG1.1); +Cc: linux-mtd@lists.infradead.org
[-- Attachment #1: Type: text/plain, Size: 1712 bytes --]
On Fri, 2012-11-09 at 08:45 +0100, Prins Anton (ST-CO/ENG1.1) wrote:
> commit 0905a6f4aec377123e94d2260f2f7a0d867e19be
> Author: Anton Prins <anton.prins@nl.bosch.com>
> Date: Fri Nov 9 10:12:58 2012 +0100
>
> Correct error checking to prevent a NULL pointer dereference
>
> The problem only occurs if the DTS is not correct, the requested mapping is not reserved on the parent bus.
> In this special case the count is 1, but the list_size after mapping is 0. list_size 0 should generate an error!
Sorry, I do not really understand which problem this patch solves, could
you please improve the commit message and re-send?
>
> diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
> index 2e6fb68..83d121e 100644
> --- a/drivers/mtd/maps/physmap_of.c
> +++ b/drivers/mtd/maps/physmap_of.c
> @@ -267,13 +267,14 @@ static int __devinit of_flash_probe(struct platform_device *dev)
> info->list[i].mtd->dev.parent = &dev->dev;
> }
>
It seems the error condition should be checked and acted upon here. What
you looks more like making the code less readable.
> - err = 0;
> if (info->list_size == 1) {
> + err = 0;
> info->cmtd = info->list[0].mtd;
> } else if (info->list_size > 1) {
> /*
> * We detected multiple devices. Concatenate them together.
> */
> + err = 0;
> info->cmtd = mtd_concat_create(mtd_list, info->list_size,
> dev_name(&dev->dev));
> if (info->cmtd == NULL)
--
Best Regards,
Artem Bityutskiy
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Patch to solve NULL pointer dereference in physmap_of.c
2012-11-21 7:42 ` Artem Bityutskiy
@ 2012-11-21 8:43 ` Prins Anton (ST-CO/ENG1.1)
2012-11-22 15:20 ` Prins Anton (ST-CO/ENG1.1)
1 sibling, 0 replies; 10+ messages in thread
From: Prins Anton (ST-CO/ENG1.1) @ 2012-11-21 8:43 UTC (permalink / raw)
To: dedekind1@gmail.com; +Cc: linux-mtd@lists.infradead.org
[-- Attachment #1: Type: text/plain, Size: 2696 bytes --]
Hi Artem,
This patch solves a NULL pointer dereference, this may occur if the tuple is not mappable (jumps to continue in the for-loop).
Out of the loop possible results are:
- info->list_size == 0 if no of the tuples is mappable
- info->list_size == 1
- info->list_size > 1
In the case no tuple is mappable (info->list_size == 0) info->cmtd is not set but used in mtd_device_parse_register...OOPS.
Indeed you also check for 'info->list_size == 0' for me it doesn't differ in readability.
My choice was to reset the error-code only If I'm sure the list_size is >= 1.
Another option, maybe the best... is to check for info->cmtd == NULL @ line 296.
Met vriendelijke groeten | Best Regards,
Anton Prins
-----Original Message-----
From: Artem Bityutskiy [mailto:dedekind1@gmail.com]
Sent: woensdag 21 november 2012 8:42
To: Prins Anton (ST-CO/ENG1.1)
Cc: linux-mtd@lists.infradead.org
Subject: Re: Patch to solve NULL pointer dereference in physmap_of.c
On Fri, 2012-11-09 at 08:45 +0100, Prins Anton (ST-CO/ENG1.1) wrote:\r> commit 0905a6f4aec377123e94d2260f2f7a0d867e19be
> Author: Anton Prins <anton.prins@nl.bosch.com>
> Date: Fri Nov 9 10:12:58 2012 +0100
>
> Correct error checking to prevent a NULL pointer dereference
>
> The problem only occurs if the DTS is not correct, the requested mapping is not reserved on the parent bus.
> In this special case the count is 1, but the list_size after mapping is 0. list_size 0 should generate an error!
Sorry, I do not really understand which problem this patch solves, could
you please improve the commit message and re-send?
>
> diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
> index 2e6fb68..83d121e 100644
> --- a/drivers/mtd/maps/physmap_of.c
> +++ b/drivers/mtd/maps/physmap_of.c
> @@ -267,13 +267,14 @@ static int __devinit of_flash_probe(struct platform_device *dev)
> info->list[i].mtd->dev.parent = &dev->dev;
> }
>
It seems the error condition should be checked and acted upon here. What
you looks more like making the code less readable.
> - err = 0;
> if (info->list_size == 1) {
> + err = 0;
> info->cmtd = info->list[0].mtd;
> } else if (info->list_size > 1) {
> /*
> * We detected multiple devices. Concatenate them together.
> */
> + err = 0;
> info->cmtd = mtd_concat_create(mtd_list, info->list_size,
> dev_name(&dev->dev));\r> if (info->cmtd == NULL)
--
Best Regards,
Artem Bityutskiy
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 6798 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Patch to solve NULL pointer dereference in physmap_of.c
2012-11-21 7:42 ` Artem Bityutskiy
2012-11-21 8:43 ` Prins Anton (ST-CO/ENG1.1)
@ 2012-11-22 15:20 ` Prins Anton (ST-CO/ENG1.1)
2012-11-30 11:51 ` Artem Bityutskiy
1 sibling, 1 reply; 10+ messages in thread
From: Prins Anton (ST-CO/ENG1.1) @ 2012-11-22 15:20 UTC (permalink / raw)
To: dedekind1@gmail.com; +Cc: linux-mtd@lists.infradead.org
[-- Attachment #1: Type: text/plain, Size: 3528 bytes --]
[PATCH] mtd: maps/physmap_of.c: change error checking to prevent a NULL pointer dereference if no DTS tuple is mappable
This patch solves a NULL pointer dereference, this may occur if the tuple is not mappable (jumps to continue in the for-loop).
Out of the loop possible results are:
- info->list_size == 0 if no of the tuples is mappable
- info->list_size == 1
- info->list_size > 1
If no one of the supplied tuples is mappable (info->list_size == 0) and info->cmtd will not be set.
But it is used in mtd_device_parse_register, OOPS... if should generate an error in this case!
[From: Anton Prins <anton.prins@nl.bosch.com>]
diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
index 2e6fb68..f6de444 100644
--- a/drivers/mtd/maps/physmap_of.c
+++ b/drivers/mtd/maps/physmap_of.c
@@ -268,6 +268,7 @@ static int __devinit of_flash_probe(struct platform_device *dev)
}
err = 0;
+ info->cmtd = NULL;
if (info->list_size == 1) {
info->cmtd = info->list[0].mtd;
} else if (info->list_size > 1) {
@@ -276,9 +277,10 @@ static int __devinit of_flash_probe(struct platform_device *dev)
*/
info->cmtd = mtd_concat_create(mtd_list, info->list_size,
dev_name(&dev->dev));\r- if (info->cmtd == NULL)
- err = -ENXIO;
}
+ if (info->cmtd == NULL)
+ err = -ENXIO;
+
if (err)
goto err_out;
-----Original Message-----
From: Artem Bityutskiy [mailto:dedekind1@gmail.com]
Sent: woensdag 21 november 2012 8:42
To: Prins Anton (ST-CO/ENG1.1)
Cc: linux-mtd@lists.infradead.org
Subject: Re: Patch to solve NULL pointer dereference in physmap_of.c
On Fri, 2012-11-09 at 08:45 +0100, Prins Anton (ST-CO/ENG1.1) wrote:\r> commit 0905a6f4aec377123e94d2260f2f7a0d867e19be
> Author: Anton Prins <anton.prins@nl.bosch.com>
> Date: Fri Nov 9 10:12:58 2012 +0100
>
> Correct error checking to prevent a NULL pointer dereference
>
> The problem only occurs if the DTS is not correct, the requested mapping is not reserved on the parent bus.
> In this special case the count is 1, but the list_size after mapping is 0. list_size 0 should generate an error!
Sorry, I do not really understand which problem this patch solves, could
you please improve the commit message and re-send?
>
> diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
> index 2e6fb68..83d121e 100644
> --- a/drivers/mtd/maps/physmap_of.c
> +++ b/drivers/mtd/maps/physmap_of.c
> @@ -267,13 +267,14 @@ static int __devinit of_flash_probe(struct platform_device *dev)
> info->list[i].mtd->dev.parent = &dev->dev;
> }
>
It seems the error condition should be checked and acted upon here. What
you looks more like making the code less readable.
> - err = 0;
> if (info->list_size == 1) {
> + err = 0;
> info->cmtd = info->list[0].mtd;
> } else if (info->list_size > 1) {
> /*
> * We detected multiple devices. Concatenate them together.
> */
> + err = 0;
> info->cmtd = mtd_concat_create(mtd_list, info->list_size,
> dev_name(&dev->dev));\r> if (info->cmtd == NULL)
--
Best Regards,
Artem Bityutskiy
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 6798 bytes --]
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: Patch to solve NULL pointer dereference in physmap_of.c
2012-11-22 15:20 ` Prins Anton (ST-CO/ENG1.1)
@ 2012-11-30 11:51 ` Artem Bityutskiy
2012-12-03 7:31 ` Prins Anton (ST-CO/ENG1.1)
2012-12-05 14:19 ` Prins Anton (ST-CO/ENG1.1)
0 siblings, 2 replies; 10+ messages in thread
From: Artem Bityutskiy @ 2012-11-30 11:51 UTC (permalink / raw)
To: Prins Anton (ST-CO/ENG1.1); +Cc: linux-mtd@lists.infradead.org
[-- Attachment #1: Type: text/plain, Size: 3969 bytes --]
Hi, sorry for long delay, but would you please re-send this in a nice
form:
1. Wrap commit message lines to 80 characters.
2. Make the patch applicable with 'git am' - this one does not apply.
Thanks!
On Thu, 2012-11-22 at 16:20 +0100, Prins Anton (ST-CO/ENG1.1) wrote:
> [PATCH] mtd: maps/physmap_of.c: change error checking to prevent a NULL pointer dereference if no DTS tuple is mappable
>
> This patch solves a NULL pointer dereference, this may occur if the tuple is not mappable (jumps to continue in the for-loop).
> Out of the loop possible results are:
> - info->list_size == 0 if no of the tuples is mappable
> - info->list_size == 1
> - info->list_size > 1
> If no one of the supplied tuples is mappable (info->list_size == 0) and info->cmtd will not be set.
> But it is used in mtd_device_parse_register, OOPS... if should generate an error in this case!
>
> [From: Anton Prins <anton.prins@nl.bosch.com>]
>
> diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
> index 2e6fb68..f6de444 100644
> --- a/drivers/mtd/maps/physmap_of.c
> +++ b/drivers/mtd/maps/physmap_of.c
> @@ -268,6 +268,7 @@ static int __devinit of_flash_probe(struct platform_device *dev)
> }
>
> err = 0;
> + info->cmtd = NULL;
> if (info->list_size == 1) {
> info->cmtd = info->list[0].mtd;
> } else if (info->list_size > 1) {
> @@ -276,9 +277,10 @@ static int __devinit of_flash_probe(struct platform_device *dev)
> */
> info->cmtd = mtd_concat_create(mtd_list, info->list_size,
> dev_name(&dev->dev));\r- if (info->cmtd == NULL)
> - err = -ENXIO;
> }
> + if (info->cmtd == NULL)
> + err = -ENXIO;
> +
> if (err)
> goto err_out;
>
> -----Original Message-----
> From: Artem Bityutskiy [mailto:dedekind1@gmail.com]
> Sent: woensdag 21 november 2012 8:42
> To: Prins Anton (ST-CO/ENG1.1)
> Cc: linux-mtd@lists.infradead.org
> Subject: Re: Patch to solve NULL pointer dereference in physmap_of.c
> On Fri, 2012-11-09 at 08:45 +0100, Prins Anton (ST-CO/ENG1.1) wrote:\r> commit 0905a6f4aec377123e94d2260f2f7a0d867e19be
> > Author: Anton Prins <anton.prins@nl.bosch.com>
> > Date: Fri Nov 9 10:12:58 2012 +0100
> >
> > Correct error checking to prevent a NULL pointer dereference
> >
> > The problem only occurs if the DTS is not correct, the requested mapping is not reserved on the parent bus.
> > In this special case the count is 1, but the list_size after mapping is 0. list_size 0 should generate an error!
>
> Sorry, I do not really understand which problem this patch solves, could
> you please improve the commit message and re-send?
>
> >
> > diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
> > index 2e6fb68..83d121e 100644
> > --- a/drivers/mtd/maps/physmap_of.c
> > +++ b/drivers/mtd/maps/physmap_of.c
> > @@ -267,13 +267,14 @@ static int __devinit of_flash_probe(struct platform_device *dev)
> > info->list[i].mtd->dev.parent = &dev->dev;
> > }
> >
>
> It seems the error condition should be checked and acted upon here. What
> you looks more like making the code less readable.
>
> > - err = 0;
> > if (info->list_size == 1) {
> > + err = 0;
> > info->cmtd = info->list[0].mtd;
> > } else if (info->list_size > 1) {
> > /*
> > * We detected multiple devices. Concatenate them together.
> > */
> > + err = 0;
> > info->cmtd = mtd_concat_create(mtd_list, info->list_size,
> > dev_name(&dev->dev));\r> if (info->cmtd == NULL)
>
--
Best Regards,
Artem Bityutskiy
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Patch to solve NULL pointer dereference in physmap_of.c
2012-11-30 11:51 ` Artem Bityutskiy
@ 2012-12-03 7:31 ` Prins Anton (ST-CO/ENG1.1)
2012-12-10 13:31 ` Artem Bityutskiy
2012-12-05 14:19 ` Prins Anton (ST-CO/ENG1.1)
1 sibling, 1 reply; 10+ messages in thread
From: Prins Anton (ST-CO/ENG1.1) @ 2012-12-03 7:31 UTC (permalink / raw)
To: dedekind1@gmail.com; +Cc: linux-mtd@lists.infradead.org
[-- Attachment #1.1: Type: text/plain, Size: 4450 bytes --]
Sorry I'm in a learning curve... now used 'git format-patch'.
This made it working for 'git am'; so hopefully this time its fine!
Artem, thanks for your support!
Met vriendelijke groeten | Best Regards,
Anton Prins
-----Original Message-----
From: Artem Bityutskiy [mailto:dedekind1@gmail.com]
Sent: vrijdag 30 november 2012 12:51
To: Prins Anton (ST-CO/ENG1.1)
Cc: linux-mtd@lists.infradead.org
Subject: Re: Patch to solve NULL pointer dereference in physmap_of.c
Hi, sorry for long delay, but would you please re-send this in a nice\rform:
1. Wrap commit message lines to 80 characters.
2. Make the patch applicable with 'git am' - this one does not apply.
Thanks!
On Thu, 2012-11-22 at 16:20 +0100, Prins Anton (ST-CO/ENG1.1) wrote:\r> [PATCH] mtd: maps/physmap_of.c: change error checking to prevent a NULL pointer dereference if no DTS tuple is mappable
>
> This patch solves a NULL pointer dereference, this may occur if the tuple is not mappable (jumps to continue in the for-loop).
> Out of the loop possible results are:
> - info->list_size == 0 if no of the tuples is mappable
> - info->list_size == 1
> - info->list_size > 1
> If no one of the supplied tuples is mappable (info->list_size == 0) and info->cmtd will not be set.
> But it is used in mtd_device_parse_register, OOPS... if should generate an error in this case!
>
> [From: Anton Prins <anton.prins@nl.bosch.com>]
>
> diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
> index 2e6fb68..f6de444 100644
> --- a/drivers/mtd/maps/physmap_of.c
> +++ b/drivers/mtd/maps/physmap_of.c
> @@ -268,6 +268,7 @@ static int __devinit of_flash_probe(struct platform_device *dev)
> }
>
> err = 0;
> + info->cmtd = NULL;
> if (info->list_size == 1) {
> info->cmtd = info->list[0].mtd;
> } else if (info->list_size > 1) {
> @@ -276,9 +277,10 @@ static int __devinit of_flash_probe(struct platform_device *dev)
> */
> info->cmtd = mtd_concat_create(mtd_list, info->list_size,
> dev_name(&dev->dev));\r- if (info->cmtd == NULL)
> - err = -ENXIO;
> }
> + if (info->cmtd == NULL)
> + err = -ENXIO;
> +
> if (err)
> goto err_out;
>
> -----Original Message-----
> From: Artem Bityutskiy [mailto:dedekind1@gmail.com]
> Sent: woensdag 21 november 2012 8:42
> To: Prins Anton (ST-CO/ENG1.1)
> Cc: linux-mtd@lists.infradead.org
> Subject: Re: Patch to solve NULL pointer dereference in physmap_of.c\r> On Fri, 2012-11-09 at 08:45 +0100, Prins Anton (ST-CO/ENG1.1) wrote:\r> commit 0905a6f4aec377123e94d2260f2f7a0d867e19be
> > Author: Anton Prins <anton.prins@nl.bosch.com>
> > Date: Fri Nov 9 10:12:58 2012 +0100
> >
> > Correct error checking to prevent a NULL pointer dereference\r> >
> > The problem only occurs if the DTS is not correct, the requested mapping is not reserved on the parent bus.
> > In this special case the count is 1, but the list_size after mapping is 0. list_size 0 should generate an error!
>
> Sorry, I do not really understand which problem this patch solves, could
> you please improve the commit message and re-send?
>
> >
> > diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
> > index 2e6fb68..83d121e 100644
> > --- a/drivers/mtd/maps/physmap_of.c
> > +++ b/drivers/mtd/maps/physmap_of.c
> > @@ -267,13 +267,14 @@ static int __devinit of_flash_probe(struct platform_device *dev)
> > info->list[i].mtd->dev.parent = &dev->dev;
> > }
> >
>
> It seems the error condition should be checked and acted upon here. What
> you looks more like making the code less readable.
>
> > - err = 0;
> > if (info->list_size == 1) {
> > + err = 0;
> > info->cmtd = info->list[0].mtd;
> > } else if (info->list_size > 1) {
> > /*
> > * We detected multiple devices. Concatenate them together.
> > */
> > + err = 0;
> > info->cmtd = mtd_concat_create(mtd_list, info->list_size,
> > dev_name(&dev->dev));
> if (info->cmtd == NULL)
>
--
Best Regards,
Artem Bityutskiy
[-- Attachment #1.2: 0001-mtd-maps-physmap_of.c-error-checking-to-prevent-a-NU.patch --]
[-- Type: application/octetstream, Size: 1544 bytes --]
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 6798 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Patch to solve NULL pointer dereference in physmap_of.c
2012-11-30 11:51 ` Artem Bityutskiy
2012-12-03 7:31 ` Prins Anton (ST-CO/ENG1.1)
@ 2012-12-05 14:19 ` Prins Anton (ST-CO/ENG1.1)
1 sibling, 0 replies; 10+ messages in thread
From: Prins Anton (ST-CO/ENG1.1) @ 2012-12-05 14:19 UTC (permalink / raw)
To: dedekind1@gmail.com; +Cc: linux-mtd@lists.infradead.org
[-- Attachment #1: Type: text/plain, Size: 6303 bytes --]
Hi Artem,
Sorry I'm in a learning curve... now used 'git format-patch'.
This made it working for 'git am'; so hopefully this time its fine!
Artem, thanks for your support!
Met vriendelijke groeten | Best Regards,
Anton Prins
>From 1f7a524dfd9b0c4b315651ebdab87938430048a9 Mon Sep 17 00:00:00 2001
From: Anton Prins <anton.prins@nl.bosch.com>
Date: Tue, 27 Nov 2012 16:38:16 +0100
Subject: [PATCH] mtd: maps/physmap_of.c: error checking to prevent a NULL pointer dereference
This patch solves a NULL pointer dereference, this may occur if the tuple
is not mappable (jumps to continue in the for-loop). Out of the loop\rpossible results are:
- info->list_size == 0 if no of the tuples is mappable
- info->list_size == 1
- info->list_size > 1
If no one of the supplied tuples is mappable (info->list_size == 0) and
info->cmtd will not be set. But it is used in mtd_device_parse_register, OOPS!
actually it should generate an error in this case!
---
drivers/mtd/maps/physmap_of.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
index 2e6fb68..f6de444 100644
--- a/drivers/mtd/maps/physmap_of.c
+++ b/drivers/mtd/maps/physmap_of.c
@@ -268,6 +268,7 @@ static int __devinit of_flash_probe(struct platform_device *dev)
}
err = 0;
+ info->cmtd = NULL;
if (info->list_size == 1) {
info->cmtd = info->list[0].mtd;
} else if (info->list_size > 1) {
@@ -276,9 +277,10 @@ static int __devinit of_flash_probe(struct platform_device *dev)
*/
info->cmtd = mtd_concat_create(mtd_list, info->list_size,
dev_name(&dev->dev));
- if (info->cmtd == NULL)
- err = -ENXIO;
}
+ if (info->cmtd == NULL)
+ err = -ENXIO;
+
if (err)
goto err_out;
--
1.7.0.4
Met vriendelijke groeten | Best Regards,
Anton Prins
Bosch Security Systems BV,
Conference Systems (ST-CO/ENG1.1)
Torenallee 49
5617 BA Eindhoven
The Netherlands
www.boschsecurity.com
T. +31 (0)40 2577077
anton.prins@nl.bosch.com
-----Original Message-----
From: Artem Bityutskiy [mailto:dedekind1@gmail.com]
Sent: vrijdag 30 november 2012 12:51
To: Prins Anton (ST-CO/ENG1.1)
Cc: linux-mtd@lists.infradead.org
Subject: Re: Patch to solve NULL pointer dereference in physmap_of.c
Hi, sorry for long delay, but would you please re-send this in a nice\rform:
1. Wrap commit message lines to 80 characters.
2. Make the patch applicable with 'git am' - this one does not apply.
Thanks!
On Thu, 2012-11-22 at 16:20 +0100, Prins Anton (ST-CO/ENG1.1) wrote:\r> [PATCH] mtd: maps/physmap_of.c: change error checking to prevent a NULL pointer dereference if no DTS tuple is mappable
>
> This patch solves a NULL pointer dereference, this may occur if the tuple is not mappable (jumps to continue in the for-loop).
> Out of the loop possible results are:
> - info->list_size == 0 if no of the tuples is mappable
> - info->list_size == 1
> - info->list_size > 1
> If no one of the supplied tuples is mappable (info->list_size == 0) and info->cmtd will not be set.
> But it is used in mtd_device_parse_register, OOPS... if should generate an error in this case!
>
> [From: Anton Prins <anton.prins@nl.bosch.com>]
>
> diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
> index 2e6fb68..f6de444 100644
> --- a/drivers/mtd/maps/physmap_of.c
> +++ b/drivers/mtd/maps/physmap_of.c
> @@ -268,6 +268,7 @@ static int __devinit of_flash_probe(struct platform_device *dev)
> }
>
> err = 0;
> + info->cmtd = NULL;
> if (info->list_size == 1) {
> info->cmtd = info->list[0].mtd;
> } else if (info->list_size > 1) {
> @@ -276,9 +277,10 @@ static int __devinit of_flash_probe(struct platform_device *dev)
> */
> info->cmtd = mtd_concat_create(mtd_list, info->list_size,
> dev_name(&dev->dev));\r- if (info->cmtd == NULL)
> - err = -ENXIO;
> }
> + if (info->cmtd == NULL)
> + err = -ENXIO;
> +
> if (err)
> goto err_out;
>
> -----Original Message-----
> From: Artem Bityutskiy [mailto:dedekind1@gmail.com]
> Sent: woensdag 21 november 2012 8:42
> To: Prins Anton (ST-CO/ENG1.1)
> Cc: linux-mtd@lists.infradead.org
> Subject: Re: Patch to solve NULL pointer dereference in physmap_of.c\r> On Fri, 2012-11-09 at 08:45 +0100, Prins Anton (ST-CO/ENG1.1) wrote:\r> commit 0905a6f4aec377123e94d2260f2f7a0d867e19be
> > Author: Anton Prins <anton.prins@nl.bosch.com>
> > Date: Fri Nov 9 10:12:58 2012 +0100
> >
> > Correct error checking to prevent a NULL pointer dereference\r> >
> > The problem only occurs if the DTS is not correct, the requested mapping is not reserved on the parent bus.
> > In this special case the count is 1, but the list_size after mapping is 0. list_size 0 should generate an error!
>
> Sorry, I do not really understand which problem this patch solves, could
> you please improve the commit message and re-send?
>
> >
> > diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
> > index 2e6fb68..83d121e 100644
> > --- a/drivers/mtd/maps/physmap_of.c
> > +++ b/drivers/mtd/maps/physmap_of.c
> > @@ -267,13 +267,14 @@ static int __devinit of_flash_probe(struct platform_device *dev)
> > info->list[i].mtd->dev.parent = &dev->dev;
> > }
> >
>
> It seems the error condition should be checked and acted upon here. What
> you looks more like making the code less readable.
>
> > - err = 0;
> > if (info->list_size == 1) {
> > + err = 0;
> > info->cmtd = info->list[0].mtd;
> > } else if (info->list_size > 1) {
> > /*
> > * We detected multiple devices. Concatenate them together.
> > */
> > + err = 0;
> > info->cmtd = mtd_concat_create(mtd_list, info->list_size,
> > dev_name(&dev->dev));
> if (info->cmtd == NULL)
>
--
Best Regards,
Artem Bityutskiy
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 6798 bytes --]
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: Patch to solve NULL pointer dereference in physmap_of.c
2012-12-03 7:31 ` Prins Anton (ST-CO/ENG1.1)
@ 2012-12-10 13:31 ` Artem Bityutskiy
2012-12-11 8:34 ` Prins Anton (ST-CO/ENG1.1)
0 siblings, 1 reply; 10+ messages in thread
From: Artem Bityutskiy @ 2012-12-10 13:31 UTC (permalink / raw)
To: Prins Anton (ST-CO/ENG1.1); +Cc: linux-mtd@lists.infradead.org
[-- Attachment #1: Type: text/plain, Size: 1633 bytes --]
On Mon, 2012-12-03 at 08:31 +0100, Prins Anton (ST-CO/ENG1.1) wrote:
> Sorry I'm in a learning curve... now used 'git format-patch'.
> This made it working for 'git am'; so hopefully this time its fine!
>
> Artem, thanks for your support!
A lot better, but is still not ideal. You did not run checkpatch.pl,
otherwise you would notice this warning:
checkpatch.pl results for patch "[PATCH] mtd: maps/physmap_of.c: error checking to prevent a NULL pointer dereference"
ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s)
total: 1 errors, 0 warnings, 19 lines checked
I've added the S-o-b line for you and pushed the patch to l2-mtd.git,
thanks!
--
Best Regards,
Artem Bityutskiy
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Patch to solve NULL pointer dereference in physmap_of.c
2012-12-10 13:31 ` Artem Bityutskiy
@ 2012-12-11 8:34 ` Prins Anton (ST-CO/ENG1.1)
2012-12-11 8:49 ` Artem Bityutskiy
0 siblings, 1 reply; 10+ messages in thread
From: Prins Anton (ST-CO/ENG1.1) @ 2012-12-11 8:34 UTC (permalink / raw)
To: dedekind1@gmail.com; +Cc: linux-mtd@lists.infradead.org
[-- Attachment #1: Type: text/plain, Size: 2002 bytes --]
Great, I was not aware I was allowed to sign-off. Thx
Met vriendelijke groeten | Best Regards,
Anton Prins
-----Original Message-----
From: Artem Bityutskiy [mailto:dedekind1@gmail.com]
Sent: maandag 10 december 2012 14:31
To: Prins Anton (ST-CO/ENG1.1)
Cc: linux-mtd@lists.infradead.org
Subject: Re: Patch to solve NULL pointer dereference in physmap_of.c
On Mon, 2012-12-03 at 08:31 +0100, Prins Anton (ST-CO/ENG1.1) wrote:\r> Sorry I'm in a learning curve... now used 'git format-patch'.
> This made it working for 'git am'; so hopefully this time its fine!\r>
> Artem, thanks for your support!
A lot better, but is still not ideal. You did not run checkpatch.pl,\rotherwise you would notice this warning:
checkpatch.pl results for patch "[PATCH] mtd: maps/physmap_of.c: error checking to prevent a NULL pointer dereference"
ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s)
total: 1 errors, 0 warnings, 19 lines checked
I've added the S-o-b line for you and pushed the patch to l2-mtd.git,\rthanks!
--
Best Regards,
Artem Bityutskiy
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 6798 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch to solve NULL pointer dereference in physmap_of.c
2012-12-11 8:34 ` Prins Anton (ST-CO/ENG1.1)
@ 2012-12-11 8:49 ` Artem Bityutskiy
0 siblings, 0 replies; 10+ messages in thread
From: Artem Bityutskiy @ 2012-12-11 8:49 UTC (permalink / raw)
To: Prins Anton (ST-CO/ENG1.1); +Cc: linux-mtd@lists.infradead.org
[-- Attachment #1: Type: text/plain, Size: 446 bytes --]
On Tue, 2012-12-11 at 09:34 +0100, Prins Anton (ST-CO/ENG1.1) wrote:
> Great, I was not aware I was allowed to sign-off. Thx
You should do this in kernel. The idea is to make the chain of people
the patch went through visible. By putting your signed-off you confirm
that this is your patch. By putting my signed-off I confirm that I took
this patch from you. Sometimes there are long s-o-f chains.
--
Best Regards,
Artem Bityutskiy
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-12-11 8:48 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-09 7:45 Patch to solve NULL pointer dereference in physmap_of.c Prins Anton (ST-CO/ENG1.1)
2012-11-21 7:42 ` Artem Bityutskiy
2012-11-21 8:43 ` Prins Anton (ST-CO/ENG1.1)
2012-11-22 15:20 ` Prins Anton (ST-CO/ENG1.1)
2012-11-30 11:51 ` Artem Bityutskiy
2012-12-03 7:31 ` Prins Anton (ST-CO/ENG1.1)
2012-12-10 13:31 ` Artem Bityutskiy
2012-12-11 8:34 ` Prins Anton (ST-CO/ENG1.1)
2012-12-11 8:49 ` Artem Bityutskiy
2012-12-05 14:19 ` Prins Anton (ST-CO/ENG1.1)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox