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]
Date:	Mon, 30 Apr 2012 09:19:37 -0700
From:	djwong <djwong@...ibm.com>
To:	"Ted Ts'o" <tytso@....edu>,
	Andreas Dilger <adilger.kernel@...ger.ca>,
	Sunil Mushran <sunil.mushran@...cle.com>,
	Martin K Petersen <martin.petersen@...cle.com>,
	Greg Freemyer <greg.freemyer@...il.com>,
	Amir Goldstein <amir73il@...il.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Andi Kleen <andi@...stfloor.org>,
	Mingming Cao <cmm@...ibm.com>,
	Joel Becker <jlbec@...lplan.org>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	linux-ext4@...r.kernel.org, Coly Li <colyli@...il.com>
Subject: Re: [PATCH 05/23] ext4: Calculate and verify superblock checksum

On Fri, Apr 27, 2012 at 04:05:00PM -0400, Ted Ts'o wrote:
> On Tue, Mar 06, 2012 at 12:48:28PM -0800, Darrick J. Wong wrote:
> > diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
> > index aca1790..90f7c2e 100644
> > --- a/fs/ext4/ext4_jbd2.c
> > +++ b/fs/ext4/ext4_jbd2.c
> > @@ -138,16 +138,23 @@ int __ext4_handle_dirty_metadata(const char *where, unsigned int line,
> >  }
> >  
> >  int __ext4_handle_dirty_super(const char *where, unsigned int line,
> > -			      handle_t *handle, struct super_block *sb)
> > +			      handle_t *handle, struct super_block *sb,
> > +			      int now)
> >  {
> >  	struct buffer_head *bh = EXT4_SB(sb)->s_sbh;
> >  	int err = 0;
> >  
> >  	if (ext4_handle_valid(handle)) {
> > +		ext4_superblock_csum_set(sb,
> > +				(struct ext4_super_block *)bh->b_data);
> >  		err = jbd2_journal_dirty_metadata(handle, bh);
> >  		if (err)
> >  			ext4_journal_abort_handle(where, line, __func__,
> >  						  bh, handle, err);
> > +	} else if (now) {
> > +		ext4_superblock_csum_set(sb,
> > +				(struct ext4_super_block *)bh->b_data);
> > +		mark_buffer_dirty(bh);
> >  	} else
> >  		sb->s_dirt = 1;
> >  	return err;
> 
> What's the point of having the ext4_handle_dirty_super_now() variant?
> 
> I don't see the point of having this?

I believe I was trying to get rid of the open-coded mark_buffer_dirty(sb)
calls.  There isn't much of reason to have the now=0 path, though; if a caller
really wants to mark s_dirt and nothing else, there's ext4_mark_super_dirty()
for that.  That said, I wonder about that -- is it desirable to be able to say
"I've changed the superblock, now update the checksum but don't do anything to
write it out to disk right now"?

After a few months' break, it seems clear to me that we could make the "else if
(now)" clause the "else" clause and get rid of the now parameter.  Anything
that really wants to set s_dirt=1 with no further action can call
ext4_mark_super_dirty().

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

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