linux-kernel - Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20100419102056.GS27497@kernel.dk>
Date:	Mon, 19 Apr 2010 12:20:56 +0200
From:	Jens Axboe <jens.axboe@...cle.com>
To:	Jörn Engel <joern@...fs.org>
Cc:	David Woodhouse <dwmw2@...radead.org>,
	linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Christoph Hellwig <hch@...radead.org>
Subject: Re: [PATCH] [MTD] Fix JFFS2 sync silent failure

On Mon, Apr 19 2010, Jörn Engel wrote:
> On Mon, 19 April 2010 09:38:44 +0200, Jens Axboe wrote:
> > On Sat, Apr 17 2010, Jörn Engel wrote:
> > > Moin David,
> > > 
> > > if I read the code correctly, JFFS2 will happily perform a NOP on
> > > sys_sync() again.  And this time it appears to be Jens' fault.
> > > 
> > > JFFS2 does not appear to set s_bdi anywhere.  And as of 32a88aa1,
> > > __sync_filesystem() will return 0 if s_bdi is not set.  As a result,
> > > sync_fs() is never called for jffs2 and whatever remains in the wbuf
> > > will not make it to the device.
> > > 
> > > The patch also adds a BUG_ON to catch this problem in any remaining or
> > > future offenders.  I am not sure about network filesystems, but at
> > > least bdev- and mtd-based ones should be caught.
> > > 
> > > Opinions?
> > 
> > I think that BUG_ON() would be a lot better as a printk() and fail mount
> > instead. There's really little point in killing the kernel for something
> > you could easily warn about and recover nicely.
> 
> *shrug*
> The BUG_ON directly above is not qualitatively different.  In both cases
> the only solution is to find and fix the bug in some other file,
> recompile and try again.  But ultimately I don't care, as long as we
> catch the bug before people lose their data.
> 
> Feel free to respin this patch.  You caused the problem in the first
> place. ;)
> 
> For the record, while I consider the two-liner that causes this mess a
> real turd, the rest of your patch was brilliant.  A shame I didn't catch
> this earlier.

Thanks, we definitely should have put a debug statement to catch this in
from day 1, good debugging should be an important part of any new
infrastructure.

Care to send your jffs2 patch separately to David? Then I'll commit a
modified variant for complaining about missing ->s_bdi on mount.

-- 
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/