Bug in jffs2/scan.c re: incorrect assumption of "all clean"block

[Artem B. Bityuckiy]

Well, I've looked trough your debug output and the code and think the
following patch should fix the bug.

On Mon, 2005-02-14 at 12:24 -0600, Steven J. Magnani wrote:
> Artem -
> 
> I'm assuming when you say "JFFS2 dbg L1 output" you mean the kernel log
> contents when CONFIG_JFFS2_FS_DEBUG=1. That seems to be WAY too much
> output for the kernel to handle.
> 
> I built a kernel with just scan.c at L1 debugging, the output is
> attached. This is completely reproducible the first time I mount this
> image created by mkfs.jffs2.
> 
> The part is NOR flash (Intel 28F320C3).
> 
> Steve
> 
> -----Original Message-----
> From: Artem B. Bityuckiy [mailto:dedekind at infradead.org] 
> Sent: Monday, February 14, 2005 7:24 AM
> To: Steven J. Magnani
> Cc: linux-mtd at lists.infradead.org
> Subject: Re: Bug in jffs2/scan.c re: incorrect assumption of "all
> clean"block
> 
> 
> Could you please send your JFFS2 dbg L1 output?
> 
> Is your flash NOR or NAND?
> 
> On Thu, 2005-02-10 at 16:44 -0600, Steven J. Magnani wrote:
> > Hello MTD folk,
> > 
> > I'm not hooked into the MTD list so please cc: me on any replies. Also
> 
> > please note that this is my first experience with this code, so excuse
> 
> > me if I've gone off base.
> > 
> > I believe there is a bug in fs/jffs2/scan.c that causes it to 
> > incorrectly assume a block with a cleanmarker is "all clean". This 
> > causes an "Eep. Block 0xXXXXXXXX taken from free_list had free_size of
> 
> > 0xXXXXXXXX!!" to go off when jffs2_do_reserve_space() is called later.
> > 
> > There are 4 conditions that have to be met in a particular block for 
> > the bug to surface:
> > 
> > * The block has a cleanmarker
> > * The end of the block has to be free space (0xFF)
> > * The block has JFFS2_NODETYPE_INODEs
> > * The block does NOT have any JFFS2_NODETYPE_DIRENTs
> > 
> > The test that governs the "all clean" assumption is as follows (cf. 
> > scan.c 1.118 line 435)
> > 
> > 	if (buf_ofs == jeb->offset && jeb->used_size ==
> > PAD(c->cleanmarker_size) && 
> > 	    c->cleanmarker_size && !jeb->dirty_size &&
> > !jeb->first_node->next_in_ino) {
> > 
> > The bug is that there is no test that checks whether any 
> > JFFS2_NODETYPE_INODEs have been seen. Since only 
> > JFFS2_NODETYPE_DIRENTs are counted as used, and JFFS2_NODETYPE_INODEs 
> > are counted as unchecked, I think that the correction should look like
> 
> > this:
> > 
> > 	if (buf_ofs == jeb->offset && ((jeb->used_size +
> > jeb->unchecked_size) == PAD(c->cleanmarker_size)) &&
> > 	    c->cleanmarker_size && !jeb->dirty_size &&
> > !jeb->first_node->next_in_ino) {
> > 
> > 
> > This seems consistent with what is done at line 631:
> > 
> > 	if ((jeb->used_size + jeb->unchecked_size) ==
> > PAD(c->cleanmarker_size) && !jeb->dirty_size 
> > 		&& (!jeb->first_node || !jeb->first_node->next_in_ino) )
> > 
> > ...although the two are still not checking quite the same set of 
> > conditions.
> > 
> > Comments? I can provide a JFFS2 image that triggers the problem if 
> > someone is interested in stepping through it.
> > 
> > Steve Magnani
> > Digital Design Corporation
> > www.digidescorp.com
> > 
> > 
> > 
> > 
> > ______________________________________________________
> > Linux MTD discussion mailing list 
> > http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
-- 
Best Regards,
Artem B. Bityuckiy,
St.-Petersburg, Russia.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: scan.c.diff
Type: text/x-patch
Size: 651 bytes
Desc: not available
Url : http://lists.infradead.org/pipermail/linux-mtd/attachments/20050215/6ff052ab/attachment.bin