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  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 7 Jan 2021 11:06:51 -0800
From:   Eric Biggers <ebiggers@...nel.org>
To:     Jan Kara <jack@...e.cz>
Cc:     linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org,
        linux-f2fs-devel@...ts.sourceforge.net, linux-xfs@...r.kernel.org,
        Theodore Ts'o <tytso@....edu>, Christoph Hellwig <hch@....de>
Subject: Re: [PATCH 08/13] ext4: simplify i_state checks in
 __ext4_update_other_inode_time()

On Thu, Jan 07, 2021 at 02:24:12PM +0100, Jan Kara wrote:
> On Mon 04-01-21 16:54:47, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@...gle.com>
> > 
> > Since I_DIRTY_TIME and I_DIRTY_INODE are mutually exclusive in i_state,
> > there's no need to check for I_DIRTY_TIME && !I_DIRTY_INODE.  Just check
> > for I_DIRTY_TIME.
> > 
> > Signed-off-by: Eric Biggers <ebiggers@...gle.com>
> > ---
> >  fs/ext4/inode.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> > index 4cc6c7834312f..9e34541715968 100644
> > --- a/fs/ext4/inode.c
> > +++ b/fs/ext4/inode.c
> > @@ -4962,14 +4962,12 @@ static void __ext4_update_other_inode_time(struct super_block *sb,
> >  		return;
> >  
> >  	if ((inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW |
> > -			       I_DIRTY_INODE)) ||
> > -	    ((inode->i_state & I_DIRTY_TIME) == 0))
> > +			       I_DIRTY_TIME)) != I_DIRTY_TIME)
> >  		return;
> 
> This is OK.
> 
> >  	spin_lock(&inode->i_lock);
> > -	if (((inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW |
> > -				I_DIRTY_INODE)) == 0) &&
> > -	    (inode->i_state & I_DIRTY_TIME)) {
> > +	if ((inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW |
> > +			       I_DIRTY_TIME)) != I_DIRTY_TIME) {
> 
> But this condition is negated AFAICT. We should have == I_DIRTY_TIME here
> AFAICT.

Indeed, I'll fix that.  Thanks for catching this!

- Eric

Powered by blists - more mailing lists