Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 18 Jan 2023 23:19:24 -0800
From:   Eric Biggers <ebiggers@kernel.org>
To:     Andreas Dilger <adilger@dilger.ca>
Cc:     Theodore Ts'o <tytso@mit.edu>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Ext4 Developers List <linux-ext4@vger.kernel.org>,
        Eric Whitney <enwlinux@gmail.com>,
        "Jason A. Donenfeld" <Jason@zx2c4.com>,
        Masahiro Yamada <masahiroy@kernel.org>
Subject: Re: Detecting default signedness of char in ext4 (despite
 -funsigned-char)
Message-ID: <Y8ju/CuPWA2lTR3W@sol.localdomain>
References: <Y8bpkm3jA3bDm3eL@debian-BULLSEYE-live-builder-AMD64>
 <7DE6598D-B60D-466F-8771-5FEC0FDEC57F@dilger.ca>
 <Y8dtze3ZLGaUi8pi@sol.localdomain>
 <CAHk-=whUNjwqZXa-MH9KMmc_CpQpoFKFjAB9ZKHuu=TbsouT4A@mail.gmail.com>
 <Y8eAJIKikCTJrlcr@sol.localdomain>
 <Y8hUCIVImjqCmEWv@mit.edu>
 <CAHk-=wiGdxWtHRZftcqyPf8WbenyjniesKyZ=o73UyxfK9BL-A@mail.gmail.com>
 <Y8hpZRmHJwdutRr2@mit.edu>
 <3A9E6D2E-F98F-461C-834D-D4E269CC737F@dilger.ca>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <3A9E6D2E-F98F-461C-834D-D4E269CC737F@dilger.ca>
Precedence: bulk

On Wed, Jan 18, 2023 at 03:20:03PM -0700, Andreas Dilger wrote:
> > In terms of what should we do for next steps, if we only pick signed,
> > then it's possible if there are some edge case users who actually did
> > use non-ASCII characters in the xattr name on PowerPC, ARM, or S/390,
> > they would be broken.  That's simpler, and if we think there are
> > darned few of them, I guess we could do that.
> > 
> > That being said, it's not that much more work to use a flag in the
> > superblock to indicate whether or not we should be explicitly casting
> > *name to either a signed or unsigned char, and then setting that flag
> > automagically to avoid problems on people who started the file system
> > on say, x86 before the signed to unsigned char transition, and who
> > started natively on a PowerPC, ARM, or S/390.
> > 
> > The one bit which makes this a bit more complex is either way, we need
> > to change both the kernel and e2fsprogs, which is why if we do the
> > signed/unsigned xattr hash flag, it's important to set the flag value
> > to be whatever the "default" signeded would be on that architecture
> > pre 6.2-rc1.
> 
> It makes sense to use the existing UNSIGNED/SIGNED flag in the superblock
> for the dir hash also for the xattr hash.  That would give the historical
> value for the xattr hashes prior to the 6.2 unsigned char change, and is
> correct for all filesystems and xattrs *except* non-ASCII xattr names
> created on 6.2+ kernels (which should indeed be relatively few cases).
> 
> e2fsck could do the same, and would again be correct for all xattrs names
> except those created with kernel 6.2+.  It could check both the signed
> and unsigned forms and correct those few that are reversed compared to
> the superblock flag, which should be rare, but isn't much work and avoids
> incorrectly clearing the "corrupted" xattrs.

Reusing the existing SIGNED/UNSIGNED dirhash superblock flag wouldn't work in
the case where a filesystem is created on x86, then flashed to an ARM device.
That's exactly how Android works.  The dirhash is then explicitly SIGNED,
whereas the xattr hash is implicitly unsigned.

Yes, that case has always been broken if there were non-ASCII xattr names in the
filesystem to begin with, before it was flashed to the device.  That doesn't
really happen though.  What *is* plausible is that some random application
running on the device uses non-ASCII xattr names.

So to be safe, a new pair of SIGNED/UNSIGNED superblock flags for the xattr hash
would be needed.  The existing pair would be for the dirhash only.

- Eric