linux-kernel - Re: [DEBUG] mtd: spi-nor: dump DWORDs of the Basic Flash Parameter Table

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 12 Sep 2017 15:12:36 +0200
From:   Boris Brezillon <boris.brezillon@...e-electrons.com>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Cyrille Pitchen <cyrille.pitchen@...ev4u.fr>,
        Marek Vasut <marek.vasut@...il.com>,
        MTD Maling List <linux-mtd@...ts.infradead.org>,
        Brian Norris <computersforpeace@...il.com>,
        David Woodhouse <dwmw2@...radead.org>,
        Richard Weinberger <richard@....at>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [DEBUG] mtd: spi-nor: dump DWORDs of the Basic Flash Parameter
 Table

Hi Geert,

On Mon, 11 Sep 2017 10:58:36 +0200
Geert Uytterhoeven <geert@...ux-m68k.org> wrote:

> Hi Cyrille,
> 
> On Thu, Sep 7, 2017 at 9:28 PM, Cyrille Pitchen
> <cyrille.pitchen@...ev4u.fr> wrote:
> >> Can you apply this patch on your tree then report me what was printed, please?
> >> I have an idea of the root cause of your issue then a potential work-around
> >> but I first need to validate my assumption to confirm that the work-around
> >> would actually work.  
> 
> +m25p80 spi0.0: DWORD1 = 0xffffffff
> +m25p80 spi0.0: DWORD2 = 0xffffffff
> +m25p80 spi0.0: DWORD3 = 0xffffffff
> +m25p80 spi0.0: DWORD4 = 0xffffffff
> +m25p80 spi0.0: DWORD5 = 0xffffffff
> +m25p80 spi0.0: DWORD6 = 0xffffffff
> +m25p80 spi0.0: DWORD7 = 0xffffffff
> +m25p80 spi0.0: DWORD8 = 0xffffffff
> +m25p80 spi0.0: DWORD9 = 0xffffffff
> +m25p80 spi0.0: DWORD10 = 0x00000000
> +m25p80 spi0.0: DWORD11 = 0x00000000
> +m25p80 spi0.0: DWORD12 = 0x00000000
> +m25p80 spi0.0: DWORD13 = 0x00000000
> +m25p80 spi0.0: DWORD14 = 0x00000000
> +m25p80 spi0.0: DWORD15 = 0x00000000
> +m25p80 spi0.0: DWORD16 = 0x00000000
> +m25p80 spi0.0: BFPT version 1.0 (length = 9)
> 
> > If you could also dump the value of the 'addr' argument of
> > spi_nor_read_sfdp_dma_unsafe() just before the for () loop below in the
> > very same function. Actually, I suspect the SFDP tables of your SPI NOR  
> 
> +m25p80 spi0.0: addr = 0x448
> 
> > memory sample to have been programmed with invalid values, neither
> > compliant with the JEDEC JESD216 specification nor with the Cypress
> > datasheet for this memory part.  
> 
> Sounds plausible.
> I get the same values when disabling DMA, so it's not due to bad DMA handling.
> All Renesas boards I have local or remote access to have spansion,s25fl512s.

Can you try with the following patch?

Thanks,

Boris

--->8---
>From 000ff63fdb149d87d755483f5edc0aba010da6b4 Mon Sep 17 00:00:00 2001
From: Boris Brezillon <boris.brezillon@...e-electrons.com>
Date: Tue, 12 Sep 2017 15:10:35 +0200
Subject: [PATCH] mtd: spi-nor: Check consistency of the memory size extracted
 from the SFDP

One field of the flash parameter table contains information about the
flash device size.
Most of the time the data extracted from this field is valid, but
sometimes the BFPT section of the SFDP table is corrupted or invalid and
this field is set to 0xffffffff, thus resulting in an integer overflow
when setting params->size.

Since NOR devices are anayway always smaller than 2^64 bytes, we can
easily stop the BFPT parsing if the size reported in this table is
invalid.

Signed-off-by: Boris Brezillon <boris.brezillon@...e-electrons.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index cf1d4a15e10a..665ccae1d090 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2127,6 +2127,15 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
 	params->size = bfpt.dwords[BFPT_DWORD(2)];
 	if (params->size & BIT(31)) {
 		params->size &= ~BIT(31);
+
+		/*
+		 * Prevent overflows on params->size. Anyway, a NOR of 1^64
+		 * bytes is unlikely to exist so this error probably means
+		 * the BFPT we are reading is corrupted/wrong.
+		 */
+		if (params->size > 63)
+			return -EINVAL;
+
 		params->size = 1ULL << params->size;
 	} else {
 		params->size++;