Re: [PATCH] erofs-utils: mkfs: emit an inode's xattrs in a canonical order

Hi Martin,

On Mon, Aug 03, 2026 at 01:49:05PM +0200, Martin Pitt wrote:
> Hello Gao,
> 
> Gao Xiang [2026-08-02 21:30 +0800]:
> > > Thanks for the patch!
> > > 
> > > I wonder if the following diff works too (but untested):
> > > 
> > > Since I'd like to unify comp_shared_xattritem, if yes, could you resend
> > > a new version (or if some bug happens) as this so I could merge this.
> > > 
> > 
> > Sorry... It should be 
> 
> That's a nice idea, thanks!
> 
> > diff --git a/lib/xattr.c b/lib/xattr.c
> > index a9486e4..6a8775b 100644
> > --- a/lib/xattr.c
> > +++ b/lib/xattr.c
> > @@ -400,17 +400,44 @@ static struct erofs_xattritem *erofs_get_selabel_xattr(struct erofs_sb_info *sbi
> >  	return NULL;
> >  }
> >  
> > +static int erofs_comp_xattritem(const void *a, const void *b)
> > +{
> > [...]
> > +	ret = memcmp(ia->kvbuf, ib->kvbuf, min(la, lb));
> > +	if (ret != 0)
> > +		return ret;
> > +	return cmpsgn(la, lb);
> 
> This actually fixes the already existing sorting on main, too: The previous
> `la > lb` never returned -1, so the sorting was half-broken.

Yes..

> 
> > +	 * Keep each inode's xattrs ordered by name.  listxattr(2) makes no
> > +	 * promise about the order it reports, and tmpfs varies it from inode
> > +	 * to inode, so appending in listing order would emit the same set of
> > +	 * xattrs differently from run to run and make images unreproducible.
> > +	 */
> > +	list_for_each_entry(pos, hlist, list)
> > +		if (erofs_comp_xattritem(item, pos->item) < 0)
> 
> The items need &, otherwise it reinterprets the first 8 bytes of struct
> erofs_xattritem, and you get bogus results. I fixed that.
> 
> There is a plot twist: Last week, when I investigated that and wrote the
> reproducer, I was still on Fedora 44's 7.1.4 kernel, and the reproducer
> reliably failed. Now I updated to 7.1.5, and it passes. This is probably the
> effect of https://lkml.iu.edu/2602.2/00479.html and/or
> https://lkml.org/lkml/2026/2/27/1141 , but it might explain why you may not
> have seen the result. Funny timing! In other words, with recent kernels
> tmpfs now reports the xattrs in insertion order instead of random.
> 
> But it still differs between running on *different* file systems, so the
> justification stands, just the reproducer changed. I changed it to accept a set
> of directories, defaulting to /tmp (which is usually tmpfs on modern distros)
> and /var/tmp (which ought to be disk-backed, btrfs in my case). With the fix,
> the erofs image comes out identical in both cases, while it differed between
> backing file systems even on 7.1.5 (just that *within* the tmpfs runs it is
> stable now). I also updated it to set 10 xattrs instead of 3, for more
> confidence.

I think we need to add a formal xattr reproducible testcase to
experimental-tests branch.  If you have time you could help add one
to ensure the order; or I could also find time too.

Thanks,
Gao Xiang