[3/3] cfi_cmdset_0002: Do not allow read/write to suspend erase block.

Message ID 20180301133941.19660-4-joakim.tjernlund@infinera.com
State Accepted
Delegated to: Boris Brezillon
Headers show
Series
  • mtd: fix AMD/Intel flash bugs
Related show

Commit Message

Joakim Tjernlund March 1, 2018, 1:39 p.m.
Currently it is possible to read and/or write to suspend EB's.
Writing /dev/mtdX or /dev/mtdblockX from several processes may
break the flash state machine.

Taken from cfi_cmdset_0001 driver.

Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
Cc: <stable@vger.kernel.org>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Richard Weinberger March 22, 2018, 2:21 p.m. | #1
Joakim,

On Thu, Mar 1, 2018 at 2:39 PM, Joakim Tjernlund
<joakim.tjernlund@infinera.com> wrote:
> Currently it is possible to read and/or write to suspend EB's.
> Writing /dev/mtdX or /dev/mtdblockX from several processes may
> break the flash state machine.
>
> Taken from cfi_cmdset_0001 driver.

Does cfi_cmdset_0020 also need fixing?

> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> Cc: <stable@vger.kernel.org>
> ---
>  drivers/mtd/chips/cfi_cmdset_0002.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index 56aa6b75213d..d524a64ed754 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -816,9 +816,10 @@ static int get_chip(struct map_info *map, struct flchip *chip, unsigned long adr
>                     (mode == FL_WRITING && (cfip->EraseSuspend & 0x2))))
>                         goto sleep;
>
> -               /* We could check to see if we're trying to access the sector
> -                * that is currently being erased. However, no user will try
> -                * anything like that so we just wait for the timeout. */

:-)

> +               /* Do not allow suspend iff read/write to EB address */
> +               if ((adr & chip->in_progress_block_mask) ==
> +                   chip->in_progress_block_addr)
> +                       goto sleep;
>
>                 /* Erase suspend */
>                 /* It's harmless to issue the Erase-Suspend and Erase-Resume
> @@ -2267,6 +2268,7 @@ static int __xipram do_erase_chip(struct map_info *map, struct flchip *chip)
>         chip->state = FL_ERASING;
>         chip->erase_suspended = 0;
>         chip->in_progress_block_addr = adr;
> +       chip->in_progress_block_mask = ~(map->size - 1);
>
>         INVALIDATE_CACHE_UDELAY(map, chip,
>                                 adr, map->size,
> @@ -2356,6 +2358,7 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip,
>         chip->state = FL_ERASING;
>         chip->erase_suspended = 0;
>         chip->in_progress_block_addr = adr;
> +       chip->in_progress_block_mask = ~(len - 1);
>
>         INVALIDATE_CACHE_UDELAY(map, chip,
>                                 adr, len,

Reviewed-by: Richard Weinberger <richard@nod.at>
Joakim Tjernlund March 22, 2018, 2:27 p.m. | #2
On Thu, 2018-03-22 at 15:21 +0100, Richard Weinberger wrote:
> 
> Joakim,
> 
> On Thu, Mar 1, 2018 at 2:39 PM, Joakim Tjernlund
> <joakim.tjernlund@infinera.com> wrote:
> > Currently it is possible to read and/or write to suspend EB's.
> > Writing /dev/mtdX or /dev/mtdblockX from several processes may
> > break the flash state machine.
> > 
> > Taken from cfi_cmdset_0001 driver.
> 
> Does cfi_cmdset_0020 also need fixing?

Dunno, but my guess is yes.

 Jocke

Patch

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 56aa6b75213d..d524a64ed754 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -816,9 +816,10 @@  static int get_chip(struct map_info *map, struct flchip *chip, unsigned long adr
 		    (mode == FL_WRITING && (cfip->EraseSuspend & 0x2))))
 			goto sleep;
 
-		/* We could check to see if we're trying to access the sector
-		 * that is currently being erased. However, no user will try
-		 * anything like that so we just wait for the timeout. */
+		/* Do not allow suspend iff read/write to EB address */
+		if ((adr & chip->in_progress_block_mask) ==
+		    chip->in_progress_block_addr)
+			goto sleep;
 
 		/* Erase suspend */
 		/* It's harmless to issue the Erase-Suspend and Erase-Resume
@@ -2267,6 +2268,7 @@  static int __xipram do_erase_chip(struct map_info *map, struct flchip *chip)
 	chip->state = FL_ERASING;
 	chip->erase_suspended = 0;
 	chip->in_progress_block_addr = adr;
+	chip->in_progress_block_mask = ~(map->size - 1);
 
 	INVALIDATE_CACHE_UDELAY(map, chip,
 				adr, map->size,
@@ -2356,6 +2358,7 @@  static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip,
 	chip->state = FL_ERASING;
 	chip->erase_suspended = 0;
 	chip->in_progress_block_addr = adr;
+	chip->in_progress_block_mask = ~(len - 1);
 
 	INVALIDATE_CACHE_UDELAY(map, chip,
 				adr, len,