Re: [PATCH AUTOSEL 6.1 35/35] ext4: deal with legacy signed xattr name hash values

[Linus Torvalds <torvalds@linux-foundation.org>]

[-- Attachment #1: Type: text/plain, Size: 828 bytes --]

On Tue, Jan 24, 2023 at 8:50 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> This patch does not work correctly without '-funsigned-char', and I
> don't think that has been back-ported to stable kernels.
>
> That said, the patch *almost* works.

So I'm  not convinced this should be back-ported at all, but it's
certainly true that going back and forth between the two cases would
be problematic.

Maybe the right thing to do would be for me to just do that explicit
'unsigned char' even in kernels that don't need it, and also add a
'pr_warn_once()' to make people aware of this case if it ever happens
outside of the xfstests.

So a more complete patch might be something like the attached (which
also changes the polarity of the signed hash test, in order to make
the pr_warn_once() simpler).

            Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1026 bytes --]

 fs/ext4/xattr.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 69a1b8c6a2ec..a2f04a3808db 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -482,11 +482,12 @@ ext4_xattr_inode_verify_hashes(struct inode *ea_inode,
 		 */
 		e_hash = ext4_xattr_hash_entry_signed(entry->e_name, entry->e_name_len,
 							&tmp_data, 1);
-		if (e_hash == entry->e_hash)
-			return 0;
-
 		/* Still no match - bad */
-		return -EFSCORRUPTED;
+		if (e_hash != entry->e_hash)
+			return -EFSCORRUPTED;
+
+		/* Let people know about old hash */
+		pr_warn_once("ext4: filesystem with signed xattr name hash");
 	}
 	return 0;
 }
@@ -3096,7 +3097,7 @@ static __le32 ext4_xattr_hash_entry(char *name, size_t name_len, __le32 *value,
 	while (name_len--) {
 		hash = (hash << NAME_HASH_SHIFT) ^
 		       (hash >> (8*sizeof(hash) - NAME_HASH_SHIFT)) ^
-		       *name++;
+		       (unsigned char)*name++;
 	}
 	while (value_count--) {
 		hash = (hash << VALUE_HASH_SHIFT) ^