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  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, 16 May 2019 09:52:42 +0200
From:   Jan Kara <jack@...e.cz>
To:     "cgxu519@...o.com.cn" <cgxu519@...o.com.cn>
Cc:     Jan Kara <jack@...e.cz>, linux-ext4@...r.kernel.org
Subject: Re: [PATCH 2/3] ext2: Merge loops in ext2_xattr_set()

On Thu 16-05-19 09:13:34, cgxu519@...o.com.cn wrote:
> On Wed, 2019-05-15 at 16:01 +0200, Jan Kara wrote:
> > There are two very similar loops when searching xattr to set. Just merge
> > them.
> > 
> > Signed-off-by: Jan Kara <jack@...e.cz>
> > ---
> >  fs/ext2/xattr.c | 32 +++++++++++---------------------
> >  1 file changed, 11 insertions(+), 21 deletions(-)
> > 
> > diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
> > index fb2e008d4406..26a049ca89fb 100644
> > --- a/fs/ext2/xattr.c
> > +++ b/fs/ext2/xattr.c
> > @@ -437,27 +437,7 @@ ext2_xattr_set(struct inode *inode, int name_index, const
> > char *name,
> >  			goto cleanup;
> >  		}
> >  		/* Find the named attribute. */
> > -		here = FIRST_ENTRY(bh);
> > -		while (!IS_LAST_ENTRY(here)) {
> > -			struct ext2_xattr_entry *next = EXT2_XATTR_NEXT(here);
> > -			if ((char *)next >= end)
> > -				goto bad_block;
> > -			if (!here->e_value_block && here->e_value_size) {
> > -				size_t offs = le16_to_cpu(here->e_value_offs);
> > -				if (offs < min_offs)
> > -					min_offs = offs;
> > -			}
> > -			not_found = name_index - here->e_name_index;
> > -			if (!not_found)
> > -				not_found = name_len - here->e_name_len;
> > -			if (!not_found)
> > -				not_found = memcmp(name, here->e_name,name_len);
> > -			if (not_found <= 0)
> > -				break;
> > -			here = next;
> > -		}
> > -		last = here;
> > -		/* We still need to compute min_offs and last. */
> > +		last = FIRST_ENTRY(bh);
> >  		while (!IS_LAST_ENTRY(last)) {
> >  			struct ext2_xattr_entry *next = EXT2_XATTR_NEXT(last);
> >  			if ((char *)next >= end)
> > @@ -467,8 +447,18 @@ ext2_xattr_set(struct inode *inode, int name_index, const
> > char *name,
> >  				if (offs < min_offs)
> >  					min_offs = offs;
> >  			}
> > +			if (not_found) {
> > +				if (name_index == last->e_name_index &&
> > +				    name_len == last->e_name_len &&
> > +				    !memcmp(name, last->e_name,name_len)) {
> > +					not_found = 0;
> > +					here = last;
> > +				}
> > +			}
> >  			last = next;
> >  		}
> > +		if (not_found)
> > +			here = last;
> 
> Entry name is sorted so I think for new entry we should find right place for it
> not just appending to last.

Ah, good catch! I actually didn't find a place which would use the fact
that names are sorted (and that's why xfstests passed fine as well) but
you're right that the old code worked that way and we should keep that.
Thanks!

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

Powered by blists - more mailing lists