JFFS2 list_dirty corruption

JFFS2 list_dirty corruption

[Thomas Gleixner]

While hacking on JFFS2 for NAND I found a possibility, where scan_medium 
corrupts list_dirty.

jffs2_scan_medium calls
jffs2_scan_eraseblock calls
jffs2_scan_dirent_node calls	
jffs2_add_fd_to_list
	There is a duplicate entry detected, so it calls
jffs2_mark_node_obsolete
	There is it possible that the jeb is added to list_dirty,
	because it's a duplicate entry
	with list_add_tail
	We come back to
jffs2_scan_eraseblock 
	There is another condition, that marks the block dirty
	We come back to
jffs2_scan_medium
	There we add the block to list_dirty too with add_list
	Then we have a circular list entry.
Result:	chaos	

That's not a problem of the NAND modifications. I verified, that this can 
happen in the actual CVS-version too.
-- 
Thomas
__________________________________________________
Thomas Gleixner, autronix automation GmbH
auf dem berg 3, d-88690 uhldingen-muehlhofen
fon: +49 7556 919891 , fax: +49 7556 919886
mail: gleixner@autronix.de, http://www.autronix.de  

Re: JFFS2 list_dirty corruption

[Thomas Gleixner]

On Wednesday, 20. February 2002 20:41, Thomas Gleixner wrote:
> While hacking on JFFS2 for NAND I found a possibility, where scan_medium
> corrupts list_dirty.
This patch resolves the problem:

--- org/fs/jffs2/scan.c
+++ work/fs/jffs2/scan.c
@@ -128,9 +128,15 @@
                                 (!c->nextblock || c->nextblock->free_size < 
jeb->free_size)) {
                                 /* Better candidate for the next writes to 
go to */
                                 if (c->nextblock)
+					/* We must delete, because mark_node_obsolete
+					   could have added this block to dirty_list already */
+                                        list_del(&c->nextblock->list);
                                         list_add(&c->nextblock->list, 
&c->dirty_list);
                                 c->nextblock = jeb;
                         } else {
+				/* We must delete, because mark_node_obsolete
+				   could have added this block to dirty_list already */
+				list_del(&jeb->list); 
                                 list_add(&jeb->list, &c->dirty_list);
                         }
 		} else {

I put it into CVS too
-- 
Thomas
__________________________________________________
Thomas Gleixner, autronix automation GmbH
auf dem berg 3, d-88690 uhldingen-muehlhofen
fon: +49 7556 919891 , fax: +49 7556 919886
mail: gleixner@autronix.de, http://www.autronix.de  

Re: JFFS2 list_dirty corruption

[Adam Wozniak]

Thomas Gleixner wrote:
> 
> On Wednesday, 20. February 2002 20:41, Thomas Gleixner wrote:
> > While hacking on JFFS2 for NAND I found a possibility, where scan_medium
> > corrupts list_dirty.
> This patch resolves the problem:
> 
> --- org/fs/jffs2/scan.c
> +++ work/fs/jffs2/scan.c
> @@ -128,9 +128,15 @@
>                                  (!c->nextblock || c->nextblock->free_size <
> jeb->free_size)) {
>                                  /* Better candidate for the next writes to
> go to */
>                                  if (c->nextblock)
> +                                       /* We must delete, because mark_node_obsolete
> +                                          could have added this block to dirty_list already */
> +                                        list_del(&c->nextblock->list);
>                                          list_add(&c->nextblock->list,
> &c->dirty_list);
>                                  c->nextblock = jeb;
>                          } else {
> +                               /* We must delete, because mark_node_obsolete
> +                                  could have added this block to dirty_list already */
> +                               list_del(&jeb->list);
>                                  list_add(&jeb->list, &c->dirty_list);
>                          }
>                 } else {
> 
> I put it into CVS too

Ummm.  Applying that patch gives me a seg fault on boot.

--Adam
-- 
Adam Wozniak (KG6GZR)   COM DEV Wireless - Digital and Software Systems
awozniak@comdev.cc      805 Aerovista Place, San Luis Obispo, CA 93401
                        http://www.comdev.cc
                        Voice: (805) 544-1089       Fax: (805) 544-2055

Re: JFFS2 list_dirty corruption

[Adam Wozniak]

Ok, maybe this is a problem:


Thomas Gleixner wrote:
> 
> On Wednesday, 20. February 2002 20:41, Thomas Gleixner wrote:
> > While hacking on JFFS2 for NAND I found a possibility, where scan_medium
> > corrupts list_dirty.
> This patch resolves the problem:
> 
> --- org/fs/jffs2/scan.c
> +++ work/fs/jffs2/scan.c
> @@ -128,9 +128,15 @@
>                                  (!c->nextblock || c->nextblock->free_size <
> jeb->free_size)) {
>                                  /* Better candidate for the next writes to
> go to */
>                                  if (c->nextblock)

UMMM open { here?

> +                                       /* We must delete, because mark_node_obsolete
> +                                          could have added this block to dirty_list already */
> +                                        list_del(&c->nextblock->list);

and close } here?

>                                          list_add(&c->nextblock->list,
> &c->dirty_list);
>                                  c->nextblock = jeb;
>                          } else {
> +                               /* We must delete, because mark_node_obsolete
> +                                  could have added this block to dirty_list already */
> +                               list_del(&jeb->list);
>                                  list_add(&jeb->list, &c->dirty_list);
>                          }
>                 } else {


-- 
Adam Wozniak (KG6GZR)   COM DEV Wireless - Digital and Software Systems
awozniak@comdev.cc      805 Aerovista Place, San Luis Obispo, CA 93401
                        http://www.comdev.cc
                        Voice: (805) 544-1089       Fax: (805) 544-2055

Re: JFFS2 list_dirty corruption

[Thomas Gleixner]

On Thursday, 21. February 2002 00:58, Adam Wozniak wrote:
> Ok, maybe this is a problem:
>
Sorry, you're right. I should send such mails better in the morning.
It happened on the transcript from my NAND modified stuff.
Here is the correct patch:

--- mtd/fs/jffs2/scan.c	2002/01/09 13:25:58	1.57
+++ mtd/fs/jffs2/scan.c	2002/02/21 00:17:19
@@ -127,10 +127,17 @@
                         if (jeb->free_size > 2*sizeof(struct 
jffs2_raw_inode) &&
                                 (!c->nextblock || c->nextblock->free_size < 
jeb->free_size)) {
                                 /* Better candidate for the next writes to 
go to */
-                                if (c->nextblock)
+                                if (c->nextblock) {
+					/* We must delete, because mark_node_obsolete
+					   could have added this block to dirty_list already */
+                                        list_del(&c->nextblock->list);
                                         list_add(&c->nextblock->list, 
&c->dirty_list);
+				}
                                 c->nextblock = jeb;
                         } else {
+				/* We must delete, because mark_node_obsolete
+				   could have added this block to dirty_list already */
+				list_del(&jeb->list);
                                 list_add(&jeb->list, &c->dirty_list);
                         }
 		} else {
-- 
Thomas
P.S. David: The jffs2-nand-version is correct.
__________________________________________________
Thomas Gleixner, autronix automation GmbH
auf dem berg 3, d-88690 uhldingen-muehlhofen
fon: +49 7556 919891 , fax: +49 7556 919886
mail: gleixner@autronix.de, http://www.autronix.de  

Re: JFFS2 list_dirty corruption

[David Woodhouse]

gleixner@autronix.de said:
> > While hacking on JFFS2 for NAND I found a possibility, where
> > scan_medium corrupts list_dirty. 
> This patch resolves the problem: 

That's OK, but I think it's cleaner if we prevent the mark_node_obsolete() 
code from touching the lists in the first place. This way, we can also 
prevent it from trying to physically mark them obsolete too - if that 
didn't happen the first time, we shouldn't be doing it on mount. 

This problem didn't turn up on NOR flash because we do mark the nodes 
obsolete on the flash, so the mount code never had to bother with them.

Please could you confirm this also fixes the problem?

Index: include/linux/jffs2_fs_sb.h
===================================================================
RCS file: /home/cvs/mtd/include/linux/jffs2_fs_sb.h,v
retrieving revision 1.18
diff -u -p -r1.18 jffs2_fs_sb.h
--- include/linux/jffs2_fs_sb.h	2002/01/09 11:44:23	1.18
+++ include/linux/jffs2_fs_sb.h	2002/02/21 08:33:56
@@ -12,6 +12,7 @@
 #define INOCACHE_HASHSIZE 1
 
 #define JFFS2_SB_FLAG_RO 1
+#define JFFS2_SB_FLAG_MOUNTING 2
 
 /* A struct for the overall file system control.  Pointers to
    jffs2_sb_info structs are named `c' in the source code.  
Index: fs/jffs2/build.c
===================================================================
RCS file: /home/cvs/mtd/fs/jffs2/build.c,v
retrieving revision 1.22
diff -u -p -r1.22 build.c
--- fs/jffs2/build.c	2002/01/09 16:30:57	1.22
+++ fs/jffs2/build.c	2002/02/21 08:33:56
@@ -293,6 +293,7 @@ int jffs2_do_mount_fs(struct jffs2_sb_in
 	INIT_LIST_HEAD(&c->bad_list);
 	INIT_LIST_HEAD(&c->bad_used_list);
 	c->highest_ino = 1;
+	c->flags |= JFFS2_SB_FLAG_MOUNTING;
 
 	if (jffs2_build_filesystem(c)) {
 		D1(printk(KERN_DEBUG "build_fs failed\n"));
@@ -301,5 +302,8 @@ int jffs2_do_mount_fs(struct jffs2_sb_in
 		kfree(c->blocks);
 		return -EIO;
 	}
+	
+	c->flags &= ~JFFS2_SB_FLAG_MOUNTING;
+
 	return 0;
 }
Index: fs/jffs2/nodemgmt.c
===================================================================
RCS file: /home/cvs/mtd/fs/jffs2/nodemgmt.c,v
retrieving revision 1.52
diff -u -p -r1.52 nodemgmt.c
--- fs/jffs2/nodemgmt.c	2002/01/19 23:16:41	1.52
+++ fs/jffs2/nodemgmt.c	2002/02/21 08:33:57
@@ -318,6 +318,15 @@ void jffs2_mark_node_obsolete(struct jff
 
 	ACCT_PARANOIA_CHECK(jeb);
 
+	if (c->flags & JFFS2_SB_FLAG_MOUNTING) {
+		/* Mount in progress. Don't muck about with the block
+		   lists because they're not ready yet, and don't actually
+		   obliterate nodes that look obsolete. If they weren't 
+		   marked obsolete on the flash at the time they _became_
+		   obsolete, there was probably a reason for that. */
+		spin_unlock_bh(&c->erase_completion_lock);
+		return;
+	}
 	if (jeb == c->nextblock) {
 		D2(printk(KERN_DEBUG "Not moving nextblock 0x%08x to dirty/erase_pending list\n", jeb->offset));
 	} else if (jeb == c->gcblock) {


--
dwmw2

Re: JFFS2 list_dirty corruption

[Thomas Gleixner]

On Thursday, 21. February 2002 09:59, David Woodhouse wrote:

> Please could you confirm this also fixes the problem?

Yep, it does.

BTW: David, did you get my mail tonight? I had some strange notifications 
from my mailsystem.

-- 
Thomas
__________________________________________________
Thomas Gleixner, autronix automation GmbH
auf dem berg 3, d-88690 uhldingen-muehlhofen
fon: +49 7556 919891 , fax: +49 7556 919886
mail: gleixner@autronix.de, http://www.autronix.de  

Re: JFFS2 list_dirty corruption

[David Woodhouse]

gleixner@autronix.de said:
>  BTW: David, did you get my mail tonight? I had some strange
> notifications  from my mailsystem. 

I got lots :) Going through it in order.

--
dwmw2