linux-kernel - Re: [PATCH RFC] ext3 data=guarded v5

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: <1241016181.20099.31.camel@think.oraclecorp.com>
Date:	Wed, 29 Apr 2009 10:43:01 -0400
From:	Chris Mason <chris.mason@...cle.com>
To:	Jan Kara <jack@...e.cz>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	"Theodore Ts'o" <tytso@....edu>,
	Linux Kernel Developers List <linux-kernel@...r.kernel.org>,
	Ext4 Developers List <linux-ext4@...r.kernel.org>,
	Mike Galbraith <efault@....de>
Subject: Re: [PATCH RFC] ext3 data=guarded v5

On Wed, 2009-04-29 at 10:08 -0400, Chris Mason wrote:
> On Wed, 2009-04-29 at 10:56 +0200, Jan Kara wrote:
> 
> > > diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
> > > index fcfa243..1e90107 100644
> > > --- a/fs/ext3/inode.c
> > > +++ b/fs/ext3/inode.c
> > > @@ -38,6 +38,7 @@
> > >  #include <linux/bio.h>
> > >  #include <linux/fiemap.h>
> > >  #include <linux/namei.h>
> > > +#include <linux/workqueue.h>
> > >  #include "xattr.h"
> > >  #include "acl.h"
> > >  
> > > @@ -179,6 +180,105 @@ static int ext3_journal_test_restart(handle_t *handle, struct inode *inode)
> > >  }
> > >  
> > >  /*
> > > + * after a data=guarded IO is done, we need to update the
> > > + * disk i_size to reflect the data we've written.  If there are
> > > + * no more ordered data extents left in the tree, we need to
> >                                            ^^^^^^^^ the list
> > > + * get rid of the orphan entry making sure the file's
> > > + * block pointers match the i_size after a crash
> > > + *
> > > + * When we aren't in data=guarded mode, this just does an ext3_orphan_del.
> > > + *
> > > + * It returns the result of ext3_orphan_del.
> > > + *
> > > + * handle may be null if we are just cleaning up the orphan list in
> > > + * memory.
> > > + *
> > > + * pass must_log == 1 when the inode must be logged in order to get
> > > + * an i_size update on disk
> > > + */
> > > +static int ordered_orphan_del(handle_t *handle, struct inode *inode,
> > > +			      int must_log)
> > > +{
> >   I'm afraid this function is racy.
> > 1) We probably need i_mutex to protect against unlink happening in parallel
> >    (after we check i_nlink but before we all ext3_orphan_del).
> 
> This would mean IO completion (clearing PG_writeback) would have to wait
> on the inode mutex, which we can't quite do in O_SYNC and O_DIRECT.
> But, what I can do is check i_nlink after the ext3_orphan_del call and
> put the inode back on the orphan list if it has gone to zero.

Ugh, that won't work, we'll just race with link and risk an orphan that
never gets removed.  I'll make a version of ext3_orphan_del that expects
the super lock held and use that instead.

-chris


--
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/