* [PATCH] drivers: mtd: mtdswap: fix possible null dereference
@ 2011-05-18 12:00 maxin
2011-05-20 5:46 ` Artem Bityutskiy
0 siblings, 1 reply; 11+ messages in thread
From: maxin @ 2011-05-18 12:00 UTC (permalink / raw)
To: linux-mtd; +Cc: Jarkko Lavinen, David Woodhouse
This patch fixes the possible null dereference of the variable "oinfo"
Thanks to Coverity for spotting this.
Signed-off-by: Maxin B. John <john.maxin@nokia.com>
---
diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
index fed215c..863987c 100644
--- a/drivers/mtd/mtdswap.c
+++ b/drivers/mtd/mtdswap.c
@@ -1450,7 +1450,12 @@ static void mtdswap_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
}
oinfo = mtd->ecclayout;
- if (!mtd->oobsize || !oinfo || oinfo->oobavail < MTDSWAP_OOBSIZE) {
+ if (!oinfo) {
+ printk(KERN_ERR "Not enough free bytes available for ECC");
+ return;
+ }
+
+ if (!mtd->oobsize || oinfo->oobavail < MTDSWAP_OOBSIZE) {
printk(KERN_ERR "%s: Not enough free bytes in OOB, "
"%d available, %zu needed.\n",
MTDSWAP_PREFIX, oinfo->oobavail, MTDSWAP_OOBSIZE);
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] drivers: mtd: mtdswap: fix possible null dereference
2011-05-18 12:00 [PATCH] drivers: mtd: mtdswap: fix possible null dereference maxin
@ 2011-05-20 5:46 ` Artem Bityutskiy
2011-05-20 11:54 ` maxin
0 siblings, 1 reply; 11+ messages in thread
From: Artem Bityutskiy @ 2011-05-20 5:46 UTC (permalink / raw)
To: maxin; +Cc: Jarkko Lavinen, linux-mtd, David Woodhouse
On Wed, 2011-05-18 at 13:00 +0100, maxin wrote:
> This patch fixes the possible null dereference of the variable "oinfo"
> Thanks to Coverity for spotting this.
>
> Signed-off-by: Maxin B. John <john.maxin@nokia.com>
> ---
> diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
> index fed215c..863987c 100644
> --- a/drivers/mtd/mtdswap.c
> +++ b/drivers/mtd/mtdswap.c
> @@ -1450,7 +1450,12 @@ static void mtdswap_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
> }
>
> oinfo = mtd->ecclayout;
> - if (!mtd->oobsize || !oinfo || oinfo->oobavail < MTDSWAP_OOBSIZE) {
> + if (!oinfo) {
> + printk(KERN_ERR "Not enough free bytes available for ECC");
> + return;
> + }
Does this print makes sense? Which ECC? I think it should me something
like "mtd% does not have OOB", mtd->index.
Moreover, you should make the print consistent with the other prints
which start with MTDSWAP_PREFIX.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] drivers: mtd: mtdswap: fix possible null dereference
2011-05-20 5:46 ` Artem Bityutskiy
@ 2011-05-20 11:54 ` maxin
2011-05-20 10:20 ` Artem Bityutskiy
0 siblings, 1 reply; 11+ messages in thread
From: maxin @ 2011-05-20 11:54 UTC (permalink / raw)
To: dedekind1; +Cc: Jarkko Lavinen, linux-mtd, David Woodhouse
Hi Artem,
On 11-05-20 06:46 AM, ext Artem Bityutskiy wrote:
> On Wed, 2011-05-18 at 13:00 +0100, maxin wrote:
>> This patch fixes the possible null dereference of the variable "oinfo"
>
> Does this print makes sense? Which ECC? I think it should me something
> like "mtd% does not have OOB", mtd->index.
>
Thank you very much for reviewing the patch.I have updated the print message
as per your suggestion as it makes more sense for this case.
> Moreover, you should make the print consistent with the other prints
> which start with MTDSWAP_PREFIX.
>
I have updated the patch as per your comments.
Signed-off-by: Maxin B. John <john.maxin@nokia.com>
---
diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
index fed215c..62ddb3a 100644
--- a/drivers/mtd/mtdswap.c
+++ b/drivers/mtd/mtdswap.c
@@ -1450,7 +1450,13 @@ static void mtdswap_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
}
oinfo = mtd->ecclayout;
- if (!mtd->oobsize || !oinfo || oinfo->oobavail < MTDSWAP_OOBSIZE) {
+ if (!oinfo) {
+ printk(KERN_ERR "%s: mtd%d does not have OOB"
+ "\n", MTDSWAP_PREFIX, mtd->index);
+ return;
+ }
+
+ if (!mtd->oobsize || oinfo->oobavail < MTDSWAP_OOBSIZE) {
printk(KERN_ERR "%s: Not enough free bytes in OOB, "
"%d available, %zu needed.\n",
MTDSWAP_PREFIX, oinfo->oobavail, MTDSWAP_OOBSIZE);
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] drivers: mtd: mtdswap: fix possible null dereference
2011-05-20 11:54 ` maxin
@ 2011-05-20 10:20 ` Artem Bityutskiy
2011-05-20 13:22 ` maxin
0 siblings, 1 reply; 11+ messages in thread
From: Artem Bityutskiy @ 2011-05-20 10:20 UTC (permalink / raw)
To: maxin; +Cc: Jarkko Lavinen, linux-mtd, David Woodhouse
On Fri, 2011-05-20 at 12:54 +0100, maxin wrote:
> > Moreover, you should make the print consistent with the other prints
> > which start with MTDSWAP_PREFIX.
> >
>
> I have updated the patch as per your comments.
>
> Signed-off-by: Maxin B. John <john.maxin@nokia.com>
> ---
> diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
> index fed215c..62ddb3a 100644
> --- a/drivers/mtd/mtdswap.c
> +++ b/drivers/mtd/mtdswap.c
> @@ -1450,7 +1450,13 @@ static void mtdswap_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
> }
>
> oinfo = mtd->ecclayout;
> - if (!mtd->oobsize || !oinfo || oinfo->oobavail < MTDSWAP_OOBSIZE) {
> + if (!oinfo) {
> + printk(KERN_ERR "%s: mtd%d does not have OOB"
Sorry, but if I save this e-mail and feed it to "git am", the commit
message will contain the discussion (with quotes, etc), not the nice
original commit message. Would you please, send a patch which I can just
save and "git am"?
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] drivers: mtd: mtdswap: fix possible null dereference
2011-05-20 10:20 ` Artem Bityutskiy
@ 2011-05-20 13:22 ` maxin
2011-05-20 12:08 ` Artem Bityutskiy
2011-05-20 12:10 ` Artem Bityutskiy
0 siblings, 2 replies; 11+ messages in thread
From: maxin @ 2011-05-20 13:22 UTC (permalink / raw)
To: dedekind1; +Cc: Jarkko Lavinen, linux-mtd, David Woodhouse
This patch fixes the possible null dereference of the variable "oinfo"
Thanks to Coverity for spotting this.
Signed-off-by: Maxin B. John <john.maxin@nokia.com>
---
diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
index fed215c..62ddb3a 100644
--- a/drivers/mtd/mtdswap.c
+++ b/drivers/mtd/mtdswap.c
@@ -1450,7 +1450,13 @@ static void mtdswap_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
}
oinfo = mtd->ecclayout;
- if (!mtd->oobsize || !oinfo || oinfo->oobavail < MTDSWAP_OOBSIZE) {
+ if (!oinfo) {
+ printk(KERN_ERR "%s: mtd%d does not have OOB"
+ "\n", MTDSWAP_PREFIX, mtd->index);
+ return;
+ }
+
+ if (!mtd->oobsize || oinfo->oobavail < MTDSWAP_OOBSIZE) {
printk(KERN_ERR "%s: Not enough free bytes in OOB, "
"%d available, %zu needed.\n",
MTDSWAP_PREFIX, oinfo->oobavail, MTDSWAP_OOBSIZE);
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] drivers: mtd: mtdswap: fix possible null dereference
2011-05-20 13:22 ` maxin
@ 2011-05-20 12:08 ` Artem Bityutskiy
2011-05-20 14:34 ` Maxin B. John
2011-05-20 12:10 ` Artem Bityutskiy
1 sibling, 1 reply; 11+ messages in thread
From: Artem Bityutskiy @ 2011-05-20 12:08 UTC (permalink / raw)
To: maxin; +Cc: Jarkko Lavinen, linux-mtd, David Woodhouse
On Fri, 2011-05-20 at 14:22 +0100, maxin wrote:
> This patch fixes the possible null dereference of the variable "oinfo"
> Thanks to Coverity for spotting this.
>
> Signed-off-by: Maxin B. John <john.maxin@nokia.com>
> ---
> diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
> index fed215c..62ddb3a 100644
> --- a/drivers/mtd/mtdswap.c
> +++ b/drivers/mtd/mtdswap.c
> @@ -1450,7 +1450,13 @@ static void mtdswap_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
> }
>
> oinfo = mtd->ecclayout;
> - if (!mtd->oobsize || !oinfo || oinfo->oobavail < MTDSWAP_OOBSIZE) {
> + if (!oinfo) {
> + printk(KERN_ERR "%s: mtd%d does not have OOB"
> + "\n", MTDSWAP_PREFIX, mtd->index);
Sorry for not noticing this immediately, but we never split the strings
if it is not really necessary. What is the fundamental reason to put
"\n" separately?
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] drivers: mtd: mtdswap: fix possible null dereference
2011-05-20 12:08 ` Artem Bityutskiy
@ 2011-05-20 14:34 ` Maxin B. John
0 siblings, 0 replies; 11+ messages in thread
From: Maxin B. John @ 2011-05-20 14:34 UTC (permalink / raw)
To: dedekind1; +Cc: Jarkko Lavinen, linux-mtd, David Woodhouse
Hi Artem,
On 11-05-20 01:08 PM, ext Artem Bityutskiy wrote:
> On Fri, 2011-05-20 at 14:22 +0100, maxin wrote:
> Sorry for not noticing this immediately, but we never split the strings
> if it is not really necessary. What is the fundamental reason to put
> "\n" separately?
>
Thanks for a thorough review. I will modify it accordingly.
>Also, when I "git am" your patch, the Author's name will be "maxin", is
>this what you want?
Hmm.. I haven't noticed this before. Thanks... I have updated the MUA
settings.
Best Regards,
Maxin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drivers: mtd: mtdswap: fix possible null dereference
2011-05-20 13:22 ` maxin
2011-05-20 12:08 ` Artem Bityutskiy
@ 2011-05-20 12:10 ` Artem Bityutskiy
2011-05-20 14:44 ` Maxin B. John
1 sibling, 1 reply; 11+ messages in thread
From: Artem Bityutskiy @ 2011-05-20 12:10 UTC (permalink / raw)
To: maxin; +Cc: Jarkko Lavinen, linux-mtd, David Woodhouse
On Fri, 2011-05-20 at 14:22 +0100, maxin wrote:
> This patch fixes the possible null dereference of the variable "oinfo"
> Thanks to Coverity for spotting this.
>
> Signed-off-by: Maxin B. John <john.maxin@nokia.com>
Also, when I "git am" your patch, the Author's name will be "maxin", is
this what you want?
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drivers: mtd: mtdswap: fix possible null dereference
2011-05-20 12:10 ` Artem Bityutskiy
@ 2011-05-20 14:44 ` Maxin B. John
2011-05-20 12:47 ` Artem Bityutskiy
2011-05-20 13:51 ` Artem Bityutskiy
0 siblings, 2 replies; 11+ messages in thread
From: Maxin B. John @ 2011-05-20 14:44 UTC (permalink / raw)
To: dedekind1; +Cc: Jarkko Lavinen, linux-mtd, David Woodhouse
This patch fixes the possible null dereference of the variable "oinfo"
Thanks to Coverity for spotting this.
Signed-off-by: Maxin B. John <john.maxin@nokia.com>
---
diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
index fed215c..fd78853 100644
--- a/drivers/mtd/mtdswap.c
+++ b/drivers/mtd/mtdswap.c
@@ -1450,7 +1450,13 @@ static void mtdswap_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
}
oinfo = mtd->ecclayout;
- if (!mtd->oobsize || !oinfo || oinfo->oobavail < MTDSWAP_OOBSIZE) {
+ if (!oinfo) {
+ printk(KERN_ERR "%s: mtd%d does not have OOB\n",
+ MTDSWAP_PREFIX, mtd->index);
+ return;
+ }
+
+ if (!mtd->oobsize || oinfo->oobavail < MTDSWAP_OOBSIZE) {
printk(KERN_ERR "%s: Not enough free bytes in OOB, "
"%d available, %zu needed.\n",
MTDSWAP_PREFIX, oinfo->oobavail, MTDSWAP_OOBSIZE);
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] drivers: mtd: mtdswap: fix possible null dereference
2011-05-20 14:44 ` Maxin B. John
@ 2011-05-20 12:47 ` Artem Bityutskiy
2011-05-20 13:51 ` Artem Bityutskiy
1 sibling, 0 replies; 11+ messages in thread
From: Artem Bityutskiy @ 2011-05-20 12:47 UTC (permalink / raw)
To: Maxin B. John; +Cc: Jarkko Lavinen, linux-mtd, David Woodhouse
On Fri, 2011-05-20 at 15:44 +0100, Maxin B. John wrote:
> This patch fixes the possible null dereference of the variable "oinfo"
> Thanks to Coverity for spotting this.
>
> Signed-off-by: Maxin B. John <john.maxin@nokia.com>
Pushed to l2-mtd-2.6.git, thanks.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drivers: mtd: mtdswap: fix possible null dereference
2011-05-20 14:44 ` Maxin B. John
2011-05-20 12:47 ` Artem Bityutskiy
@ 2011-05-20 13:51 ` Artem Bityutskiy
1 sibling, 0 replies; 11+ messages in thread
From: Artem Bityutskiy @ 2011-05-20 13:51 UTC (permalink / raw)
To: Maxin B. John; +Cc: Jarkko Lavinen, linux-mtd, David Woodhouse
On Fri, 2011-05-20 at 15:44 +0100, Maxin B. John wrote:
> This patch fixes the possible null dereference of the variable "oinfo"
> Thanks to Coverity for spotting this.
>
> Signed-off-by: Maxin B. John <john.maxin@nokia.com>
Pushed to l2-mtd-2.6.git, thanks!
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-05-20 13:55 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-18 12:00 [PATCH] drivers: mtd: mtdswap: fix possible null dereference maxin
2011-05-20 5:46 ` Artem Bityutskiy
2011-05-20 11:54 ` maxin
2011-05-20 10:20 ` Artem Bityutskiy
2011-05-20 13:22 ` maxin
2011-05-20 12:08 ` Artem Bityutskiy
2011-05-20 14:34 ` Maxin B. John
2011-05-20 12:10 ` Artem Bityutskiy
2011-05-20 14:44 ` Maxin B. John
2011-05-20 12:47 ` Artem Bityutskiy
2011-05-20 13:51 ` Artem Bityutskiy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.