Patchwork [(fixed)] Fix infinite loop in INFTL_foldchain() in drivers/mtd/inftlcore.c

login
register
mail settings
Submitter Daniel Rosenthal
Date Oct. 7, 2008, 5:29 p.m.
Message ID <20081007132904.6vvmlioy8co004ko@system.cs.fsu.edu>
Download mbox | patch
Permalink /patch/3259/
State Accepted
Headers show

Comments

Daniel Rosenthal - Oct. 7, 2008, 5:29 p.m.
I'm having problems with my email client, but I've attached a patch  
(both inline and as a regular attachment). This patch fixes a loop  
that is potentially infinite in INFTL_foldchain in  
drivers/mtd/inftlcore.c.

When iterating over a chain in reverse (oldest block first), this  
patch correctly marks the PUtable[] entry of the second to last erase  
block of a chain as BLOCK_NIL, regardless of whether or not it can  
format the last block successfully. Before, the second to last block  
was only marked as pointing to BLOCK_NIL if INFTL_formatblock()  
succeeded on the last block of the chain, which could potentially  
result in an infinite loop if the block was worn out and refused to  
format.

If there are any problems with this patch please let me know. I can  
apply them fine after pulling them from email, but my email client  
(web interface) isn't fullproof, so if anybody else has problems,  
please let me know. (I can't send it through pine or otherwise due to  
network configuration.)

Thanks,
Daniel

----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.
>From 8f1275749768d316788194cc198093b00f85d0d6 Mon Sep 17 00:00:00 2001
From: Daniel Rosenthal <danielrosenthal@acm.org>
Date: Sun, 5 Oct 2008 17:43:10 -0400
Subject: [PATCH] Fix infinite loop in INFTL_foldchain in drivers/mtd/inftlcore.c.


Signed-off-by: Daniel Rosenthal <danielrosenthal@acm.org>
---
 drivers/mtd/inftlcore.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
Greg Ungerer - Oct. 8, 2008, 6:25 a.m.
Hi Daniel,

Daniel Rosenthal wrote:
> I'm having problems with my email client, but I've attached a patch 
> (both inline and as a regular attachment). This patch fixes a loop that 
> is potentially infinite in INFTL_foldchain in drivers/mtd/inftlcore.c.
> 
> When iterating over a chain in reverse (oldest block first), this patch 
> correctly marks the PUtable[] entry of the second to last erase block of 
> a chain as BLOCK_NIL, regardless of whether or not it can format the 
> last block successfully. Before, the second to last block was only 
> marked as pointing to BLOCK_NIL if INFTL_formatblock() succeeded on the 
> last block of the chain, which could potentially result in an infinite 
> loop if the block was worn out and refused to format.
> 
> If there are any problems with this patch please let me know. I can 
> apply them fine after pulling them from email, but my email client (web 
> interface) isn't fullproof, so if anybody else has problems, please let 
> me know. (I can't send it through pine or otherwise due to network 
> configuration.)

I haven't looked at this code for quite a while, but your
reasoning and patch looks right to me. So you have my acked-by.

Acked-by: Greg Ungerer <gerg@snapgear.com>

David, I assume you are going to pick this up?

Regards
Greg



------------------------------------------------------------------------
Greg Ungerer  --  Chief Software Dude       EMAIL:     gerg@snapgear.com
Secure Computing Corporation                PHONE:       +61 7 3435 2888
825 Stanley St,                             FAX:         +61 7 3891 3630
Woolloongabba, QLD, 4102, Australia         WEB: http://www.SnapGear.com
David Woodhouse - Oct. 8, 2008, 10:39 a.m.
On Wed, 2008-10-08 at 16:25 +1000, Greg Ungerer wrote:
> Hi Daniel,
> 
> Daniel Rosenthal wrote:
> > I'm having problems with my email client, but I've attached a patch 
> > (both inline and as a regular attachment). This patch fixes a loop that 
> > is potentially infinite in INFTL_foldchain in drivers/mtd/inftlcore.c.
> > 
> > When iterating over a chain in reverse (oldest block first), this patch 
> > correctly marks the PUtable[] entry of the second to last erase block of 
> > a chain as BLOCK_NIL, regardless of whether or not it can format the 
> > last block successfully. Before, the second to last block was only 
> > marked as pointing to BLOCK_NIL if INFTL_formatblock() succeeded on the 
> > last block of the chain, which could potentially result in an infinite 
> > loop if the block was worn out and refused to format.
> > 
> > If there are any problems with this patch please let me know. I can 
> > apply them fine after pulling them from email, but my email client (web 
> > interface) isn't fullproof, so if anybody else has problems, please let 
> > me know. (I can't send it through pine or otherwise due to network 
> > configuration.)
> 
> I haven't looked at this code for quite a while, but your
> reasoning and patch looks right to me. So you have my acked-by.
> 
> Acked-by: Greg Ungerer <gerg@snapgear.com>
> 
> David, I assume you are going to pick this up?

Committed. Thanks.

Patch

diff --git a/drivers/mtd/inftlcore.c b/drivers/mtd/inftlcore.c
index c4f9d33..50ce138 100644
--- a/drivers/mtd/inftlcore.c
+++ b/drivers/mtd/inftlcore.c
@@ -388,6 +388,10 @@  static u16 INFTL_foldchain(struct INFTLrecord *inftl, unsigned thisVUC, unsigned
 		if (thisEUN == targetEUN)
 			break;
 
+		/* Unlink the last block from the chain. */
+		inftl->PUtable[prevEUN] = BLOCK_NIL;
+
+		/* Now try to erase it. */
 		if (INFTL_formatblock(inftl, thisEUN) < 0) {
 			/*
 			 * Could not erase : mark block as reserved.
@@ -396,7 +400,6 @@  static u16 INFTL_foldchain(struct INFTLrecord *inftl, unsigned thisVUC, unsigned
 		} else {
 			/* Correctly erased : mark it as free */
 			inftl->PUtable[thisEUN] = BLOCK_FREE;
-			inftl->PUtable[prevEUN] = BLOCK_NIL;
 			inftl->numfreeEUNs++;
 		}
 	}