> From: Paul Walmsley <paul@xxxxxxxxx> > To: Orjan Friberg <of@xxxxxxxxxxxx> > Cc: David Woodhouse <dwmw2@xxxxxxxxxxxxx>, Richard Purdie <rpurdie@xxxxxxxxxxxxxx>, "linux-omap@xxxxxxxxxxxxxxx" <linux-omap@xxxxxxxxxxxxxxx>, "linux-mtd@xxxxxxxxxxxxxxxxxxx" <linux-mtd@xxxxxxxxxxxxxxxxxxx> > Date: 2012/01/26 17:14 > Subject: Re: CONFIG_PREEMPT and JFFS2 oops > Sent by: linux-mtd-bounces@xxxxxxxxxxxxxxxxxxx > > On Thu, 26 Jan 2012, Orjan Friberg wrote: > > > Ok, so comprbuf comes from jffs2_compress and becomes NULL for some reason > > (hence the oops). > > > > Initially I had CMODE_FAVOUR_LZO. With that, things only worked with > > PREEMPT_NONE. However, when changing to CMODE_PRIORITY or CMODE_NONE things > > do seem to work *with* PREEMPT. > > > > For what it's worth (with PREEMPT on): > > > > CMODE_FAVOUR_LZO with LZO disabled oopses. > > CMODE_FAVOUR_LZO with only ZLIB enabled oopses. > > CMODE_FAVOUR_LZO with ZLIB/LZO/RTIME/RUBIN disabled does not oops. > > > > Thus, the bug seems to be in the *selection* of compression algorithm (when > > there is at least one algoritm in the list), rather than in the specific > > compression algorithms themselves. > > The locking in jffs2_compress() isn't right. this->compr_buf could be > freed or reassigned while the compressor is busy using it. If I'm reading > the code correctly, this problem could also cause data and metadata > corruption. > > Want to give the following untested patch a try? Of course, it could > destroy everything. So it shouldn't be used on anything important. But > it might fix the problem you're seeing. The patch will need significant > testing and review by JFFS2 experts before getting merged. > > > - Paul > > > From: Paul Walmsley <paul@xxxxxxxxx> > Date: Thu, 26 Jan 2012 08:12:09 -0700 > Subject: [PATCH] fs: jffs2: compression: fix some (but not all) races in > jffs2_compress() > > There is a nasty race in jffs2_compress() when JFFS2_COMPR_MODE_SIZE > or JFFS2_COMPR_MODE_FAVOURLZO is selected and multiple jffs2 > filesystems are in use. The compressor buffer is shared among all > users of the compressor, and the buffer is freed and allocated without > holding any lock. This could result in NULL pointer dereferences, or, > worse, corrupted data. > > There doesn't appear to be any point in having a compression buffer > shared by all users of the compressor. So remove this, and instead > use a buffer that is local to the jffs2_compress() call. This > simplifies the locking in this function considerably. > > There's at least one race left in this function, between it and > jffs2_unregister_compressor(). That's left for someone else to fix. > Until then, it is suggested that compressors should not be registered > or unregister while any JFFS2 filesystems are mounted. > > This patch is COMPLETELY UNTESTED. It could easily DESTROY THE > FILESYSTEM and CORRUPT DATA, so don't use it unless it's been tested > thoroughly and the code has been reviewed by JFFS2 experts. I think I found one bug by looking at the patch. You need 2 buffers, one that holds the latest compressed data and one working buffer. Jocke -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
