Patchwork drivers: mtd: mtdswap: fix possible null dereference

login
register
mail settings
Submitter john.maxin@nokia.com
Date May 18, 2011, noon
Message ID <4DD3B4C2.30301@nokia.com>
Download mbox | patch
Permalink /patch/96143/
State New
Headers show

Comments

john.maxin@nokia.com - May 18, 2011, noon
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>
---
Artem Bityutskiy - May 20, 2011, 5:46 a.m.
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.

Patch

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);