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
| ||
|
Date: Mon, 05 Dec 2011 15:07:23 -0600 From: Eric Sandeen <sandeen@...hat.com> To: "Ted Ts'o" <tytso@....edu> CC: Alex <alex.vizor@...il.com>, adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org, Dave Chinner <dchinner@...hat.com> Subject: Re: WARNING: at fs/inode.c:884 unlock_new_inode+0x34/0x59() On 12/5/11 2:54 PM, Eric Sandeen wrote: > On 11/27/11 3:34 PM, Ted Ts'o wrote: >> On Sun, Nov 27, 2011 at 11:24:03PM +0300, Alex wrote: >>> BTW, after last resume from disk fs was corrupted but fsck managed >>> to fix this error. So I think severity of this issue should be >>> raised. >> >> Can you reproduce this reliably? What was running at the time of the s2disk? >> >> What appears to be going on is that insert_inode_locked() is failing >> at fs/ext4/ialloc.c:887, probably because there's another inode with >> that inode number already on the superblock's hash list. The error >> codepath if insert_inode_locked() fail is incorrect; it's going to >> fail_drop, which tries dropping the inode's dquot (but we haven't >> calle ddquot_initialize)inode) yet) and calls unlock_new_inode(), but >> I_NEW hasn't been set because insert_inode_locked(). > > OK; this looks to be the result of: > > commit 250df6ed274d767da844a5d9f05720b804240197 > Author: Dave Chinner <dchinner@...hat.com> > Date: Tue Mar 22 22:23:36 2011 +1100 > > fs: protect inode->i_state with inode->i_lock > > (went in on 2.6.39) > > because before that, insert_inode_locked() used to unconditionally do: > > - inode->i_state |= I_NEW; > > but that's gone now. Now if the function fails it'll return the > inode w/o I_NEW set. > > ext2/3/4, jffs2, and jfs all call unlock_new_inode() on insert_inode_locked() > failure, and all would warn on this path. > > I'm still not clear on what's causing insert_inode_locked() to fail, > but it used to be harmless (or at least silent) before. > > I suppose it makes most sense to fix all callers to not clear I_NEW > on failure, unless it's too icky; it does seem weird to have I_NEW set > if we return with failure. > > -Eric > OTOH Al thought it would be reasonable to set I_NEW on failure as well, and then we wouldn't have to touch the callers. -Eric > > >> So the warning is easy to fix; we just need to have it jump to fail >> instead of fail_drop. But the bigger issue is why did >> insert_inode_locked() failed in the first place. >> >> Did this error happen *right* after the system resumed, or did some >> amount of time pass before the warning triggered? This could have >> happened because the in-memory (or possibly on-disk) copy of the inode >> allocation bitmap has gotten corrupted, for example. >> >> What was the nature of the file system corruption which e2fsck decided >> that it need to correct? >> >> Regards, >> >> - Ted >> -- \ -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists