linux-fscrypt.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [syzbot] [fscrypt?] WARNING in fscrypt_fname_siphash
@ 2024-05-29 20:41 syzbot
  2024-05-30  7:41 ` [PATCH] ext4: add casefolded file check Lizhi Xu
  0 siblings, 1 reply; 25+ messages in thread
From: syzbot @ 2024-05-29 20:41 UTC (permalink / raw)
  To: coreteam, davem, ebiggers, fw, jaegeuk, kadlec, kuba,
	linux-fscrypt, linux-kernel, netdev, netfilter-devel, pablo,
	syzkaller-bugs, tytso

Hello,

syzbot found the following issue on:

HEAD commit:    74eca356f6d4 Merge tag 'ceph-for-6.10-rc1' of https://gith..
git tree:       upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=15460752980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=ee7b962709a5f5a5
dashboard link: https://syzkaller.appspot.com/bug?extid=340581ba9dceb7e06fb3
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=15da8cd8980000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14111972980000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/a5f8df213fa4/disk-74eca356.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/864334770567/vmlinux-74eca356.xz
kernel image: https://storage.googleapis.com/syzbot-assets/b30965ded6d8/bzImage-74eca356.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/b77754f322c2/mount_0.gz

The issue was bisected to:

commit f9006acc8dfe59e25aa75729728ac57a8d84fc32
Author: Florian Westphal <fw@strlen.de>
Date:   Wed Apr 21 07:51:08 2021 +0000

    netfilter: arp_tables: pass table pointer via nf_hook_ops

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=12dc9ee8980000
final oops:     https://syzkaller.appspot.com/x/report.txt?x=11dc9ee8980000
console output: https://syzkaller.appspot.com/x/log.txt?x=16dc9ee8980000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
Fixes: f9006acc8dfe ("netfilter: arp_tables: pass table pointer via nf_hook_ops")

EXT4-fs (loop0): mounted filesystem 00000000-0000-0000-0000-000000000000 r/w without journal. Quota mode: writeback.
fscrypt: AES-256-CBC-CTS using implementation "cts-cbc-aes-aesni"
------------[ cut here ]------------
WARNING: CPU: 1 PID: 5079 at fs/crypto/fname.c:567 fscrypt_fname_siphash+0xc2/0x100 fs/crypto/fname.c:567
Modules linked in:
CPU: 1 PID: 5079 Comm: syz-executor422 Not tainted 6.9.0-syzkaller-12358-g74eca356f6d4 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/02/2024
RIP: 0010:fscrypt_fname_siphash+0xc2/0x100 fs/crypto/fname.c:567
Code: 0f b6 04 28 84 c0 75 3d 41 8b 34 24 49 83 c6 40 4c 89 ff 4c 89 f2 5b 41 5c 41 5d 41 5e 41 5f e9 b4 97 52 09 e8 3f 7a 72 ff 90 <0f> 0b 90 eb a8 89 d9 80 e1 07 38 c1 7c 86 48 89 df e8 d8 f0 d4 ff
RSP: 0018:ffffc90003c7f430 EFLAGS: 00010293
RAX: ffffffff822399b1 RBX: 0000000000000000 RCX: ffff888022a61e00
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffffc90003c7f5f0 R08: ffffffff82239955 R09: ffffffff82541c38
R10: 0000000000000007 R11: ffff888022a61e00 R12: ffffc90003c7f580
R13: dffffc0000000000 R14: ffff88807fcb3000 R15: ffff88807fd0b4b0
FS:  000055558ddca380(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000045ede0 CR3: 0000000074e92000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 __ext4fs_dirhash+0x3db/0x1530 fs/ext4/hash.c:268
 ext4fs_dirhash+0x193/0x320 fs/ext4/hash.c:322
 htree_dirblock_to_tree+0x727/0x10e0 fs/ext4/namei.c:1124
 ext4_htree_fill_tree+0x744/0x1400 fs/ext4/namei.c:1219
 ext4_dx_readdir fs/ext4/dir.c:597 [inline]
 ext4_readdir+0x2b1c/0x3500 fs/ext4/dir.c:142
 iterate_dir+0x65e/0x820 fs/readdir.c:110
 __do_sys_getdents64 fs/readdir.c:409 [inline]
 __se_sys_getdents64+0x20d/0x4f0 fs/readdir.c:394
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f91a3441b99
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 17 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffd41154a58 EFLAGS: 00000246 ORIG_RAX: 00000000000000d9
RAX: ffffffffffffffda RBX: 6f72746e6f632f2e RCX: 00007f91a3441b99
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000004
RBP: 00007f91a34b55f0 R08: 000055558ddcb4c0 R09: 000055558ddcb4c0
R10: 000055558ddcb4c0 R11: 0000000000000246 R12: 00007ffd41154a80
R13: 00007ffd41154ca8 R14: 431bde82d7b634db R15: 00007f91a348a03b
 </TASK>


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] ext4: add casefolded file check
  2024-05-29 20:41 [syzbot] [fscrypt?] WARNING in fscrypt_fname_siphash syzbot
@ 2024-05-30  7:41 ` Lizhi Xu
  2024-05-31  1:05   ` Eric Biggers
  0 siblings, 1 reply; 25+ messages in thread
From: Lizhi Xu @ 2024-05-30  7:41 UTC (permalink / raw)
  To: syzbot+340581ba9dceb7e06fb3
  Cc: coreteam, davem, ebiggers, fw, jaegeuk, kadlec, kuba,
	linux-fscrypt, linux-kernel, netdev, netfilter-devel, pablo,
	syzkaller-bugs, tytso

The file name that needs to calculate the siphash must have both flags casefolded
and dir at the same time, so before calculating it, confirm that the flag meets
the conditions.

Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/ext4/hash.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/ext4/hash.c b/fs/ext4/hash.c
index deabe29da7fb..c8840cfc01dd 100644
--- a/fs/ext4/hash.c
+++ b/fs/ext4/hash.c
@@ -265,6 +265,10 @@ static int __ext4fs_dirhash(const struct inode *dir, const char *name, int len,
 		__u64	combined_hash;
 
 		if (fscrypt_has_encryption_key(dir)) {
+			if (!IS_CASEFOLDED(dir)) {
+				ext4_warning_inode(dir, "Siphash requires Casefolded file");
+				return -2;
+			}
 			combined_hash = fscrypt_fname_siphash(dir, &qname);
 		} else {
 			ext4_warning_inode(dir, "Siphash requires key");
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] ext4: add casefolded file check
  2024-05-30  7:41 ` [PATCH] ext4: add casefolded file check Lizhi Xu
@ 2024-05-31  1:05   ` Eric Biggers
  2024-05-31  1:47     ` Lizhi Xu
                       ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Eric Biggers @ 2024-05-31  1:05 UTC (permalink / raw)
  To: Lizhi Xu
  Cc: syzbot+340581ba9dceb7e06fb3, coreteam, davem, fw, jaegeuk, kadlec,
	kuba, linux-fscrypt, linux-kernel, netdev, netfilter-devel, pablo,
	syzkaller-bugs, tytso

On Thu, May 30, 2024 at 03:41:50PM +0800, 'Lizhi Xu' via syzkaller-bugs wrote:
> The file name that needs to calculate the siphash must have both flags casefolded
> and dir at the same time, so before calculating it, confirm that the flag meets
> the conditions.
> 
> Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
>  fs/ext4/hash.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/ext4/hash.c b/fs/ext4/hash.c
> index deabe29da7fb..c8840cfc01dd 100644
> --- a/fs/ext4/hash.c
> +++ b/fs/ext4/hash.c
> @@ -265,6 +265,10 @@ static int __ext4fs_dirhash(const struct inode *dir, const char *name, int len,
>  		__u64	combined_hash;
>  
>  		if (fscrypt_has_encryption_key(dir)) {
> +			if (!IS_CASEFOLDED(dir)) {
> +				ext4_warning_inode(dir, "Siphash requires Casefolded file");
> +				return -2;
> +			}
>  			combined_hash = fscrypt_fname_siphash(dir, &qname);
>  		} else {
>  			ext4_warning_inode(dir, "Siphash requires key");

First, this needs to be sent to the ext4 mailing list (and not to irrelevant
mailing lists such as netdev).  Please use ./scripts/get_maintainer.pl, as is
recommended by Documentation/process/submitting-patches.rst.

Second, ext4 already checks for the directory being casefolded before allowing
siphash.  This is done by dx_probe().  Evidently syzbot found some way around
that, so what needs to be done is figure out why that happened and what is the
best fix to prevent it.  This is not necessarily the patch you've proposed, as
the real issue might actually be a missing check at some earlier time like when
reading the inode from disk or when mounting the filesystem.

- Eric

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] ext4: add casefolded file check
  2024-05-31  1:05   ` Eric Biggers
@ 2024-05-31  1:47     ` Lizhi Xu
  2024-05-31  2:20       ` Eric Biggers
  2024-05-31  3:07     ` [PATCH V2] ext4: add casefolded feature check before setup encrypted info Lizhi Xu
  2024-05-31  3:30     ` [PATCH V2] ext4: add casefolded feature check before setup encrypted info Lizhi Xu
  2 siblings, 1 reply; 25+ messages in thread
From: Lizhi Xu @ 2024-05-31  1:47 UTC (permalink / raw)
  To: ebiggers
  Cc: coreteam, davem, fw, jaegeuk, kadlec, kuba, linux-fscrypt,
	linux-kernel, lizhi.xu, netdev, netfilter-devel, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso

On Thu, 30 May 2024 18:05:13 -0700, Eric Biggers wrote:
> > The file name that needs to calculate the siphash must have both flags casefolded
> > and dir at the same time, so before calculating it, confirm that the flag meets
> > the conditions.
> >
> > Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
> > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > ---
> >  fs/ext4/hash.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/fs/ext4/hash.c b/fs/ext4/hash.c
> > index deabe29da7fb..c8840cfc01dd 100644
> > --- a/fs/ext4/hash.c
> > +++ b/fs/ext4/hash.c
> > @@ -265,6 +265,10 @@ static int __ext4fs_dirhash(const struct inode *dir, const char *name, int len,
> >  		__u64	combined_hash;
> >
> >  		if (fscrypt_has_encryption_key(dir)) {
> > +			if (!IS_CASEFOLDED(dir)) {
> > +				ext4_warning_inode(dir, "Siphash requires Casefolded file");
> > +				return -2;
> > +			}
> >  			combined_hash = fscrypt_fname_siphash(dir, &qname);
> >  		} else {
> >  			ext4_warning_inode(dir, "Siphash requires key");
> 
> First, this needs to be sent to the ext4 mailing list (and not to irrelevant
> mailing lists such as netdev).  Please use ./scripts/get_maintainer.pl, as is
> recommended by Documentation/process/submitting-patches.rst.
> 
> Second, ext4 already checks for the directory being casefolded before allowing
> siphash.  This is done by dx_probe().  Evidently syzbot found some way around
> that, so what needs to be done is figure out why that happened and what is the
> best fix to prevent it.  This is not necessarily the patch you've proposed, as
> the real issue might actually be a missing check at some earlier time like when
> reading the inode from disk or when mounting the filesystem.
I have confirmed that there is no casefolded feature when creating the directory.
I agree with your statement that it should be checked for casefold features when
mounting or reading from disk.

Lizhi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] ext4: add casefolded file check
  2024-05-31  1:47     ` Lizhi Xu
@ 2024-05-31  2:20       ` Eric Biggers
  0 siblings, 0 replies; 25+ messages in thread
From: Eric Biggers @ 2024-05-31  2:20 UTC (permalink / raw)
  To: Lizhi Xu
  Cc: coreteam, davem, fw, jaegeuk, kadlec, kuba, linux-fscrypt,
	linux-kernel, netdev, netfilter-devel, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso

On Fri, May 31, 2024 at 09:47:47AM +0800, 'Lizhi Xu' via syzkaller-bugs wrote:
> On Thu, 30 May 2024 18:05:13 -0700, Eric Biggers wrote:
> > > The file name that needs to calculate the siphash must have both flags casefolded
> > > and dir at the same time, so before calculating it, confirm that the flag meets
> > > the conditions.
> > >
> > > Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
> > > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > > ---
> > >  fs/ext4/hash.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/fs/ext4/hash.c b/fs/ext4/hash.c
> > > index deabe29da7fb..c8840cfc01dd 100644
> > > --- a/fs/ext4/hash.c
> > > +++ b/fs/ext4/hash.c
> > > @@ -265,6 +265,10 @@ static int __ext4fs_dirhash(const struct inode *dir, const char *name, int len,
> > >  		__u64	combined_hash;
> > >
> > >  		if (fscrypt_has_encryption_key(dir)) {
> > > +			if (!IS_CASEFOLDED(dir)) {
> > > +				ext4_warning_inode(dir, "Siphash requires Casefolded file");
> > > +				return -2;
> > > +			}
> > >  			combined_hash = fscrypt_fname_siphash(dir, &qname);
> > >  		} else {
> > >  			ext4_warning_inode(dir, "Siphash requires key");
> > 
> > First, this needs to be sent to the ext4 mailing list (and not to irrelevant
> > mailing lists such as netdev).  Please use ./scripts/get_maintainer.pl, as is
> > recommended by Documentation/process/submitting-patches.rst.
> > 
> > Second, ext4 already checks for the directory being casefolded before allowing
> > siphash.  This is done by dx_probe().  Evidently syzbot found some way around
> > that, so what needs to be done is figure out why that happened and what is the
> > best fix to prevent it.  This is not necessarily the patch you've proposed, as
> > the real issue might actually be a missing check at some earlier time like when
> > reading the inode from disk or when mounting the filesystem.
> I have confirmed that there is no casefolded feature when creating the directory.
> I agree with your statement that it should be checked for casefold features when
> mounting or reading from disk.
> 

I haven't looked at the syzbot reproducer, but I'm guessing that the
DX_HASH_SIPHASH is coming from s_def_hash_version in the filesystem superblock.
It's not valid to have DX_HASH_SIPHASH there, and it probably would make more
sense to validate that at mount time.

- Eric

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH V2] ext4: add casefolded feature check before setup encrypted info
  2024-05-31  1:05   ` Eric Biggers
  2024-05-31  1:47     ` Lizhi Xu
@ 2024-05-31  3:07     ` Lizhi Xu
  2024-05-31  3:11       ` Eric Biggers
  2024-05-31  8:58       ` kernel test robot
  2024-05-31  3:30     ` [PATCH V2] ext4: add casefolded feature check before setup encrypted info Lizhi Xu
  2 siblings, 2 replies; 25+ messages in thread
From: Lizhi Xu @ 2024-05-31  3:07 UTC (permalink / raw)
  To: ebiggers
  Cc: coreteam, davem, fw, jaegeuk, kadlec, kuba, linux-fscrypt,
	linux-kernel, lizhi.xu, netdev, netfilter-devel, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso

Due to the current file system not supporting the casefolded feature, only 
i_crypt_info was initialized when creating encrypted information, without actually
setting the sighash. Therefore, when creating an inode, if the system does not 
support the casefolded feature, encrypted information will not be created.

Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/ext4/ialloc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index e9bbb1da2d0a..47b75589fdf4 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -983,7 +983,8 @@ struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
 		ei->i_projid = make_kprojid(&init_user_ns, EXT4_DEF_PROJID);
 
 	if (!(i_flags & EXT4_EA_INODE_FL)) {
-		err = fscrypt_prepare_new_inode(dir, inode, &encrypt);
+		if (ext4_has_feature_casefold(inode->i_sb))
+			err = fscrypt_prepare_new_inode(dir, inode, &encrypt);
 		if (err)
 			goto out;
 	}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH V2] ext4: add casefolded feature check before setup encrypted info
  2024-05-31  3:07     ` [PATCH V2] ext4: add casefolded feature check before setup encrypted info Lizhi Xu
@ 2024-05-31  3:11       ` Eric Biggers
  2024-05-31  8:58       ` kernel test robot
  1 sibling, 0 replies; 25+ messages in thread
From: Eric Biggers @ 2024-05-31  3:11 UTC (permalink / raw)
  To: Lizhi Xu
  Cc: coreteam, davem, fw, jaegeuk, kadlec, kuba, linux-fscrypt,
	linux-kernel, netdev, netfilter-devel, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso

On Fri, May 31, 2024 at 11:07:40AM +0800, 'Lizhi Xu' via syzkaller-bugs wrote:
> Due to the current file system not supporting the casefolded feature, only 
> i_crypt_info was initialized when creating encrypted information, without actually
> setting the sighash. Therefore, when creating an inode, if the system does not 
> support the casefolded feature, encrypted information will not be created.
> 
> Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
>  fs/ext4/ialloc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index e9bbb1da2d0a..47b75589fdf4 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -983,7 +983,8 @@ struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
>  		ei->i_projid = make_kprojid(&init_user_ns, EXT4_DEF_PROJID);
>  
>  	if (!(i_flags & EXT4_EA_INODE_FL)) {
> -		err = fscrypt_prepare_new_inode(dir, inode, &encrypt);
> +		if (ext4_has_feature_casefold(inode->i_sb))
> +			err = fscrypt_prepare_new_inode(dir, inode, &encrypt);
>  		if (err)
>  			goto out;

No, this is not correct at all.  This just disables encryption on filesystems
with the casefold feature.

As I said before, please also use the correct mailing lists.

- Eric

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V2] ext4: add casefolded feature check before setup encrypted info
  2024-05-31  1:05   ` Eric Biggers
  2024-05-31  1:47     ` Lizhi Xu
  2024-05-31  3:07     ` [PATCH V2] ext4: add casefolded feature check before setup encrypted info Lizhi Xu
@ 2024-05-31  3:30     ` Lizhi Xu
  2024-05-31  3:34       ` Eric Biggers
  2 siblings, 1 reply; 25+ messages in thread
From: Lizhi Xu @ 2024-05-31  3:30 UTC (permalink / raw)
  To: ebiggers
  Cc: coreteam, davem, fw, jaegeuk, kadlec, kuba, linux-fscrypt,
	linux-kernel, lizhi.xu, netdev, netfilter-devel, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso,
	adilger.kernel, linux-ext4

On Thu, 30 May 2024 20:11:33 -0700, Eric Biggers wrote:
> > Due to the current file system not supporting the casefolded feature, only 
> > i_crypt_info was initialized when creating encrypted information, without actually
> > setting the sighash. Therefore, when creating an inode, if the system does not 
> > support the casefolded feature, encrypted information will not be created.
> > 
> > Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
> > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > ---
> >  fs/ext4/ialloc.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> > index e9bbb1da2d0a..47b75589fdf4 100644
> > --- a/fs/ext4/ialloc.c
> > +++ b/fs/ext4/ialloc.c
> > @@ -983,7 +983,8 @@ struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
> >  		ei->i_projid = make_kprojid(&init_user_ns, EXT4_DEF_PROJID);
> >  
> >  	if (!(i_flags & EXT4_EA_INODE_FL)) {
> > -		err = fscrypt_prepare_new_inode(dir, inode, &encrypt);
> > +		if (ext4_has_feature_casefold(inode->i_sb))
> > +			err = fscrypt_prepare_new_inode(dir, inode, &encrypt);
> >  		if (err)
> >  			goto out;
> 
> No, this is not correct at all.  This just disables encryption on filesystems
> with the casefold feature.
If filesystems not support casefold feature, Why do I need to setup encrypted
information when creating a directory? Can encrypted information not include *hash?
> 
> As I said before, please also use the correct mailing lists.
Added.

Lizhi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V2] ext4: add casefolded feature check before setup encrypted info
  2024-05-31  3:30     ` [PATCH V2] ext4: add casefolded feature check before setup encrypted info Lizhi Xu
@ 2024-05-31  3:34       ` Eric Biggers
  2024-05-31  8:56         ` [PATCH V3] ext4: check hash version and filesystem casefolded consistent Lizhi Xu
  0 siblings, 1 reply; 25+ messages in thread
From: Eric Biggers @ 2024-05-31  3:34 UTC (permalink / raw)
  To: Lizhi Xu
  Cc: coreteam, davem, fw, jaegeuk, kadlec, kuba, linux-fscrypt,
	linux-kernel, netdev, netfilter-devel, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso,
	adilger.kernel, linux-ext4

On Fri, May 31, 2024 at 11:30:44AM +0800, 'Lizhi Xu' via syzkaller-bugs wrote:
> On Thu, 30 May 2024 20:11:33 -0700, Eric Biggers wrote:
> > > Due to the current file system not supporting the casefolded feature, only 
> > > i_crypt_info was initialized when creating encrypted information, without actually
> > > setting the sighash. Therefore, when creating an inode, if the system does not 
> > > support the casefolded feature, encrypted information will not be created.
> > > 
> > > Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
> > > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > > ---
> > >  fs/ext4/ialloc.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> > > index e9bbb1da2d0a..47b75589fdf4 100644
> > > --- a/fs/ext4/ialloc.c
> > > +++ b/fs/ext4/ialloc.c
> > > @@ -983,7 +983,8 @@ struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
> > >  		ei->i_projid = make_kprojid(&init_user_ns, EXT4_DEF_PROJID);
> > >  
> > >  	if (!(i_flags & EXT4_EA_INODE_FL)) {
> > > -		err = fscrypt_prepare_new_inode(dir, inode, &encrypt);
> > > +		if (ext4_has_feature_casefold(inode->i_sb))
> > > +			err = fscrypt_prepare_new_inode(dir, inode, &encrypt);
> > >  		if (err)
> > >  			goto out;
> > 
> > No, this is not correct at all.  This just disables encryption on filesystems
> > with the casefold feature.
> If filesystems not support casefold feature, Why do I need to setup encrypted
> information when creating a directory? Can encrypted information not include *hash?

Encryption is a separate feature.  It is supported both with and without
casefold.

- Eric

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH V3] ext4: check hash version and filesystem casefolded consistent
  2024-05-31  3:34       ` Eric Biggers
@ 2024-05-31  8:56         ` Lizhi Xu
  2024-06-04  9:27           ` Dan Carpenter
  0 siblings, 1 reply; 25+ messages in thread
From: Lizhi Xu @ 2024-05-31  8:56 UTC (permalink / raw)
  To: ebiggers
  Cc: adilger.kernel, coreteam, davem, fw, jaegeuk, kadlec, kuba,
	linux-ext4, linux-fscrypt, linux-kernel, lizhi.xu, netdev,
	netfilter-devel, pablo, syzbot+340581ba9dceb7e06fb3,
	syzkaller-bugs, tytso

When mounting the ext4 filesystem, if the hash version and casefolded are not
consistent, exit the mounting.

Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/ext4/super.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index c682fb927b64..c0036e3922c2 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5262,6 +5262,9 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 		goto failed_mount;
 
 	ext4_hash_info_init(sb);
+	if (es->s_def_hash_version == DX_HASH_SIPHASH && 
+	    !ext4_has_feature_casefold(sb))
+		goto failed_mount;
 
 	err = ext4_handle_clustersize(sb);
 	if (err)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH V2] ext4: add casefolded feature check before setup encrypted info
  2024-05-31  3:07     ` [PATCH V2] ext4: add casefolded feature check before setup encrypted info Lizhi Xu
  2024-05-31  3:11       ` Eric Biggers
@ 2024-05-31  8:58       ` kernel test robot
  2024-05-31  9:06         ` [PATCH V4] ext4: check hash version and filesystem casefolded consistent Lizhi Xu
  1 sibling, 1 reply; 25+ messages in thread
From: kernel test robot @ 2024-05-31  8:58 UTC (permalink / raw)
  To: Lizhi Xu, ebiggers
  Cc: llvm, oe-kbuild-all, coreteam, davem, fw, jaegeuk, kadlec, kuba,
	linux-fscrypt, linux-kernel, lizhi.xu, netdev, netfilter-devel,
	pablo, syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso

Hi Lizhi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on netfilter-nf/main linus/master v6.10-rc1 next-20240529]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lizhi-Xu/ext4-add-casefolded-feature-check-before-setup-encrypted-info/20240531-111129
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20240531030740.1024475-1-lizhi.xu%40windriver.com
patch subject: [PATCH V2] ext4: add casefolded feature check before setup encrypted info
config: riscv-defconfig (https://download.01.org/0day-ci/archive/20240531/202405311607.yQR7dozp-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project bafda89a0944d947fc4b3b5663185e07a397ac30)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240531/202405311607.yQR7dozp-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405311607.yQR7dozp-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from fs/ext4/ialloc.c:21:
   In file included from include/linux/buffer_head.h:12:
   In file included from include/linux/blk_types.h:10:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:8:
   In file included from include/linux/cacheflush.h:5:
   In file included from arch/riscv/include/asm/cacheflush.h:9:
   In file included from include/linux/mm.h:2208:
   include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     522 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> fs/ext4/ialloc.c:986:7: warning: variable 'err' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     986 |                 if (ext4_has_feature_casefold(inode->i_sb))
         |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/ext4/ialloc.c:988:7: note: uninitialized use occurs here
     988 |                 if (err)
         |                     ^~~
   fs/ext4/ialloc.c:986:3: note: remove the 'if' if its condition is always true
     986 |                 if (ext4_has_feature_casefold(inode->i_sb))
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     987 |                         err = fscrypt_prepare_new_inode(dir, inode, &encrypt);
   fs/ext4/ialloc.c:939:15: note: initialize the variable 'err' to silence this warning
     939 |         int ret2, err;
         |                      ^
         |                       = 0
   2 warnings generated.


vim +986 fs/ext4/ialloc.c

   912	
   913	/*
   914	 * There are two policies for allocating an inode.  If the new inode is
   915	 * a directory, then a forward search is made for a block group with both
   916	 * free space and a low directory-to-inode ratio; if that fails, then of
   917	 * the groups with above-average free space, that group with the fewest
   918	 * directories already is chosen.
   919	 *
   920	 * For other inodes, search forward from the parent directory's block
   921	 * group to find a free inode.
   922	 */
   923	struct inode *__ext4_new_inode(struct mnt_idmap *idmap,
   924				       handle_t *handle, struct inode *dir,
   925				       umode_t mode, const struct qstr *qstr,
   926				       __u32 goal, uid_t *owner, __u32 i_flags,
   927				       int handle_type, unsigned int line_no,
   928				       int nblocks)
   929	{
   930		struct super_block *sb;
   931		struct buffer_head *inode_bitmap_bh = NULL;
   932		struct buffer_head *group_desc_bh;
   933		ext4_group_t ngroups, group = 0;
   934		unsigned long ino = 0;
   935		struct inode *inode;
   936		struct ext4_group_desc *gdp = NULL;
   937		struct ext4_inode_info *ei;
   938		struct ext4_sb_info *sbi;
   939		int ret2, err;
   940		struct inode *ret;
   941		ext4_group_t i;
   942		ext4_group_t flex_group;
   943		struct ext4_group_info *grp = NULL;
   944		bool encrypt = false;
   945	
   946		/* Cannot create files in a deleted directory */
   947		if (!dir || !dir->i_nlink)
   948			return ERR_PTR(-EPERM);
   949	
   950		sb = dir->i_sb;
   951		sbi = EXT4_SB(sb);
   952	
   953		if (unlikely(ext4_forced_shutdown(sb)))
   954			return ERR_PTR(-EIO);
   955	
   956		ngroups = ext4_get_groups_count(sb);
   957		trace_ext4_request_inode(dir, mode);
   958		inode = new_inode(sb);
   959		if (!inode)
   960			return ERR_PTR(-ENOMEM);
   961		ei = EXT4_I(inode);
   962	
   963		/*
   964		 * Initialize owners and quota early so that we don't have to account
   965		 * for quota initialization worst case in standard inode creating
   966		 * transaction
   967		 */
   968		if (owner) {
   969			inode->i_mode = mode;
   970			i_uid_write(inode, owner[0]);
   971			i_gid_write(inode, owner[1]);
   972		} else if (test_opt(sb, GRPID)) {
   973			inode->i_mode = mode;
   974			inode_fsuid_set(inode, idmap);
   975			inode->i_gid = dir->i_gid;
   976		} else
   977			inode_init_owner(idmap, inode, dir, mode);
   978	
   979		if (ext4_has_feature_project(sb) &&
   980		    ext4_test_inode_flag(dir, EXT4_INODE_PROJINHERIT))
   981			ei->i_projid = EXT4_I(dir)->i_projid;
   982		else
   983			ei->i_projid = make_kprojid(&init_user_ns, EXT4_DEF_PROJID);
   984	
   985		if (!(i_flags & EXT4_EA_INODE_FL)) {
 > 986			if (ext4_has_feature_casefold(inode->i_sb))
   987				err = fscrypt_prepare_new_inode(dir, inode, &encrypt);
   988			if (err)
   989				goto out;
   990		}
   991	
   992		err = dquot_initialize(inode);
   993		if (err)
   994			goto out;
   995	
   996		if (!handle && sbi->s_journal && !(i_flags & EXT4_EA_INODE_FL)) {
   997			ret2 = ext4_xattr_credits_for_new_inode(dir, mode, encrypt);
   998			if (ret2 < 0) {
   999				err = ret2;
  1000				goto out;
  1001			}
  1002			nblocks += ret2;
  1003		}
  1004	
  1005		if (!goal)
  1006			goal = sbi->s_inode_goal;
  1007	
  1008		if (goal && goal <= le32_to_cpu(sbi->s_es->s_inodes_count)) {
  1009			group = (goal - 1) / EXT4_INODES_PER_GROUP(sb);
  1010			ino = (goal - 1) % EXT4_INODES_PER_GROUP(sb);
  1011			ret2 = 0;
  1012			goto got_group;
  1013		}
  1014	
  1015		if (S_ISDIR(mode))
  1016			ret2 = find_group_orlov(sb, dir, &group, mode, qstr);
  1017		else
  1018			ret2 = find_group_other(sb, dir, &group, mode);
  1019	
  1020	got_group:
  1021		EXT4_I(dir)->i_last_alloc_group = group;
  1022		err = -ENOSPC;
  1023		if (ret2 == -1)
  1024			goto out;
  1025	
  1026		/*
  1027		 * Normally we will only go through one pass of this loop,
  1028		 * unless we get unlucky and it turns out the group we selected
  1029		 * had its last inode grabbed by someone else.
  1030		 */
  1031		for (i = 0; i < ngroups; i++, ino = 0) {
  1032			err = -EIO;
  1033	
  1034			gdp = ext4_get_group_desc(sb, group, &group_desc_bh);
  1035			if (!gdp)
  1036				goto out;
  1037	
  1038			/*
  1039			 * Check free inodes count before loading bitmap.
  1040			 */
  1041			if (ext4_free_inodes_count(sb, gdp) == 0)
  1042				goto next_group;
  1043	
  1044			if (!(sbi->s_mount_state & EXT4_FC_REPLAY)) {
  1045				grp = ext4_get_group_info(sb, group);
  1046				/*
  1047				 * Skip groups with already-known suspicious inode
  1048				 * tables
  1049				 */
  1050				if (!grp || EXT4_MB_GRP_IBITMAP_CORRUPT(grp))
  1051					goto next_group;
  1052			}
  1053	
  1054			brelse(inode_bitmap_bh);
  1055			inode_bitmap_bh = ext4_read_inode_bitmap(sb, group);
  1056			/* Skip groups with suspicious inode tables */
  1057			if (((!(sbi->s_mount_state & EXT4_FC_REPLAY))
  1058			     && EXT4_MB_GRP_IBITMAP_CORRUPT(grp)) ||
  1059			    IS_ERR(inode_bitmap_bh)) {
  1060				inode_bitmap_bh = NULL;
  1061				goto next_group;
  1062			}
  1063	
  1064	repeat_in_this_group:
  1065			ret2 = find_inode_bit(sb, group, inode_bitmap_bh, &ino);
  1066			if (!ret2)
  1067				goto next_group;
  1068	
  1069			if (group == 0 && (ino + 1) < EXT4_FIRST_INO(sb)) {
  1070				ext4_error(sb, "reserved inode found cleared - "
  1071					   "inode=%lu", ino + 1);
  1072				ext4_mark_group_bitmap_corrupted(sb, group,
  1073						EXT4_GROUP_INFO_IBITMAP_CORRUPT);
  1074				goto next_group;
  1075			}
  1076	
  1077			if ((!(sbi->s_mount_state & EXT4_FC_REPLAY)) && !handle) {
  1078				BUG_ON(nblocks <= 0);
  1079				handle = __ext4_journal_start_sb(NULL, dir->i_sb,
  1080					 line_no, handle_type, nblocks, 0,
  1081					 ext4_trans_default_revoke_credits(sb));
  1082				if (IS_ERR(handle)) {
  1083					err = PTR_ERR(handle);
  1084					ext4_std_error(sb, err);
  1085					goto out;
  1086				}
  1087			}
  1088			BUFFER_TRACE(inode_bitmap_bh, "get_write_access");
  1089			err = ext4_journal_get_write_access(handle, sb, inode_bitmap_bh,
  1090							    EXT4_JTR_NONE);
  1091			if (err) {
  1092				ext4_std_error(sb, err);
  1093				goto out;
  1094			}
  1095			ext4_lock_group(sb, group);
  1096			ret2 = ext4_test_and_set_bit(ino, inode_bitmap_bh->b_data);
  1097			if (ret2) {
  1098				/* Someone already took the bit. Repeat the search
  1099				 * with lock held.
  1100				 */
  1101				ret2 = find_inode_bit(sb, group, inode_bitmap_bh, &ino);
  1102				if (ret2) {
  1103					ext4_set_bit(ino, inode_bitmap_bh->b_data);
  1104					ret2 = 0;
  1105				} else {
  1106					ret2 = 1; /* we didn't grab the inode */
  1107				}
  1108			}
  1109			ext4_unlock_group(sb, group);
  1110			ino++;		/* the inode bitmap is zero-based */
  1111			if (!ret2)
  1112				goto got; /* we grabbed the inode! */
  1113	
  1114			if (ino < EXT4_INODES_PER_GROUP(sb))
  1115				goto repeat_in_this_group;
  1116	next_group:
  1117			if (++group == ngroups)
  1118				group = 0;
  1119		}
  1120		err = -ENOSPC;
  1121		goto out;
  1122	
  1123	got:
  1124		BUFFER_TRACE(inode_bitmap_bh, "call ext4_handle_dirty_metadata");
  1125		err = ext4_handle_dirty_metadata(handle, NULL, inode_bitmap_bh);
  1126		if (err) {
  1127			ext4_std_error(sb, err);
  1128			goto out;
  1129		}
  1130	
  1131		BUFFER_TRACE(group_desc_bh, "get_write_access");
  1132		err = ext4_journal_get_write_access(handle, sb, group_desc_bh,
  1133						    EXT4_JTR_NONE);
  1134		if (err) {
  1135			ext4_std_error(sb, err);
  1136			goto out;
  1137		}
  1138	
  1139		/* We may have to initialize the block bitmap if it isn't already */
  1140		if (ext4_has_group_desc_csum(sb) &&
  1141		    gdp->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT)) {
  1142			struct buffer_head *block_bitmap_bh;
  1143	
  1144			block_bitmap_bh = ext4_read_block_bitmap(sb, group);
  1145			if (IS_ERR(block_bitmap_bh)) {
  1146				err = PTR_ERR(block_bitmap_bh);
  1147				goto out;
  1148			}
  1149			BUFFER_TRACE(block_bitmap_bh, "get block bitmap access");
  1150			err = ext4_journal_get_write_access(handle, sb, block_bitmap_bh,
  1151							    EXT4_JTR_NONE);
  1152			if (err) {
  1153				brelse(block_bitmap_bh);
  1154				ext4_std_error(sb, err);
  1155				goto out;
  1156			}
  1157	
  1158			BUFFER_TRACE(block_bitmap_bh, "dirty block bitmap");
  1159			err = ext4_handle_dirty_metadata(handle, NULL, block_bitmap_bh);
  1160	
  1161			/* recheck and clear flag under lock if we still need to */
  1162			ext4_lock_group(sb, group);
  1163			if (ext4_has_group_desc_csum(sb) &&
  1164			    (gdp->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT))) {
  1165				gdp->bg_flags &= cpu_to_le16(~EXT4_BG_BLOCK_UNINIT);
  1166				ext4_free_group_clusters_set(sb, gdp,
  1167					ext4_free_clusters_after_init(sb, group, gdp));
  1168				ext4_block_bitmap_csum_set(sb, gdp, block_bitmap_bh);
  1169				ext4_group_desc_csum_set(sb, group, gdp);
  1170			}
  1171			ext4_unlock_group(sb, group);
  1172			brelse(block_bitmap_bh);
  1173	
  1174			if (err) {
  1175				ext4_std_error(sb, err);
  1176				goto out;
  1177			}
  1178		}
  1179	
  1180		/* Update the relevant bg descriptor fields */
  1181		if (ext4_has_group_desc_csum(sb)) {
  1182			int free;
  1183			struct ext4_group_info *grp = NULL;
  1184	
  1185			if (!(sbi->s_mount_state & EXT4_FC_REPLAY)) {
  1186				grp = ext4_get_group_info(sb, group);
  1187				if (!grp) {
  1188					err = -EFSCORRUPTED;
  1189					goto out;
  1190				}
  1191				down_read(&grp->alloc_sem); /*
  1192							     * protect vs itable
  1193							     * lazyinit
  1194							     */
  1195			}
  1196			ext4_lock_group(sb, group); /* while we modify the bg desc */
  1197			free = EXT4_INODES_PER_GROUP(sb) -
  1198				ext4_itable_unused_count(sb, gdp);
  1199			if (gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT)) {
  1200				gdp->bg_flags &= cpu_to_le16(~EXT4_BG_INODE_UNINIT);
  1201				free = 0;
  1202			}
  1203			/*
  1204			 * Check the relative inode number against the last used
  1205			 * relative inode number in this group. if it is greater
  1206			 * we need to update the bg_itable_unused count
  1207			 */
  1208			if (ino > free)
  1209				ext4_itable_unused_set(sb, gdp,
  1210						(EXT4_INODES_PER_GROUP(sb) - ino));
  1211			if (!(sbi->s_mount_state & EXT4_FC_REPLAY))
  1212				up_read(&grp->alloc_sem);
  1213		} else {
  1214			ext4_lock_group(sb, group);
  1215		}
  1216	
  1217		ext4_free_inodes_set(sb, gdp, ext4_free_inodes_count(sb, gdp) - 1);
  1218		if (S_ISDIR(mode)) {
  1219			ext4_used_dirs_set(sb, gdp, ext4_used_dirs_count(sb, gdp) + 1);
  1220			if (sbi->s_log_groups_per_flex) {
  1221				ext4_group_t f = ext4_flex_group(sbi, group);
  1222	
  1223				atomic_inc(&sbi_array_rcu_deref(sbi, s_flex_groups,
  1224								f)->used_dirs);
  1225			}
  1226		}
  1227		if (ext4_has_group_desc_csum(sb)) {
  1228			ext4_inode_bitmap_csum_set(sb, gdp, inode_bitmap_bh,
  1229						   EXT4_INODES_PER_GROUP(sb) / 8);
  1230			ext4_group_desc_csum_set(sb, group, gdp);
  1231		}
  1232		ext4_unlock_group(sb, group);
  1233	
  1234		BUFFER_TRACE(group_desc_bh, "call ext4_handle_dirty_metadata");
  1235		err = ext4_handle_dirty_metadata(handle, NULL, group_desc_bh);
  1236		if (err) {
  1237			ext4_std_error(sb, err);
  1238			goto out;
  1239		}
  1240	
  1241		percpu_counter_dec(&sbi->s_freeinodes_counter);
  1242		if (S_ISDIR(mode))
  1243			percpu_counter_inc(&sbi->s_dirs_counter);
  1244	
  1245		if (sbi->s_log_groups_per_flex) {
  1246			flex_group = ext4_flex_group(sbi, group);
  1247			atomic_dec(&sbi_array_rcu_deref(sbi, s_flex_groups,
  1248							flex_group)->free_inodes);
  1249		}
  1250	
  1251		inode->i_ino = ino + group * EXT4_INODES_PER_GROUP(sb);
  1252		/* This is the optimal IO size (for stat), not the fs block size */
  1253		inode->i_blocks = 0;
  1254		simple_inode_init_ts(inode);
  1255		ei->i_crtime = inode_get_mtime(inode);
  1256	
  1257		memset(ei->i_data, 0, sizeof(ei->i_data));
  1258		ei->i_dir_start_lookup = 0;
  1259		ei->i_disksize = 0;
  1260	
  1261		/* Don't inherit extent flag from directory, amongst others. */
  1262		ei->i_flags =
  1263			ext4_mask_flags(mode, EXT4_I(dir)->i_flags & EXT4_FL_INHERITED);
  1264		ei->i_flags |= i_flags;
  1265		ei->i_file_acl = 0;
  1266		ei->i_dtime = 0;
  1267		ei->i_block_group = group;
  1268		ei->i_last_alloc_group = ~0;
  1269	
  1270		ext4_set_inode_flags(inode, true);
  1271		if (IS_DIRSYNC(inode))
  1272			ext4_handle_sync(handle);
  1273		if (insert_inode_locked(inode) < 0) {
  1274			/*
  1275			 * Likely a bitmap corruption causing inode to be allocated
  1276			 * twice.
  1277			 */
  1278			err = -EIO;
  1279			ext4_error(sb, "failed to insert inode %lu: doubly allocated?",
  1280				   inode->i_ino);
  1281			ext4_mark_group_bitmap_corrupted(sb, group,
  1282						EXT4_GROUP_INFO_IBITMAP_CORRUPT);
  1283			goto out;
  1284		}
  1285		inode->i_generation = get_random_u32();
  1286	
  1287		/* Precompute checksum seed for inode metadata */
  1288		if (ext4_has_metadata_csum(sb)) {
  1289			__u32 csum;
  1290			__le32 inum = cpu_to_le32(inode->i_ino);
  1291			__le32 gen = cpu_to_le32(inode->i_generation);
  1292			csum = ext4_chksum(sbi, sbi->s_csum_seed, (__u8 *)&inum,
  1293					   sizeof(inum));
  1294			ei->i_csum_seed = ext4_chksum(sbi, csum, (__u8 *)&gen,
  1295						      sizeof(gen));
  1296		}
  1297	
  1298		ext4_clear_state_flags(ei); /* Only relevant on 32-bit archs */
  1299		ext4_set_inode_state(inode, EXT4_STATE_NEW);
  1300	
  1301		ei->i_extra_isize = sbi->s_want_extra_isize;
  1302		ei->i_inline_off = 0;
  1303		if (ext4_has_feature_inline_data(sb) &&
  1304		    (!(ei->i_flags & EXT4_DAX_FL) || S_ISDIR(mode)))
  1305			ext4_set_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
  1306		ret = inode;
  1307		err = dquot_alloc_inode(inode);
  1308		if (err)
  1309			goto fail_drop;
  1310	
  1311		/*
  1312		 * Since the encryption xattr will always be unique, create it first so
  1313		 * that it's less likely to end up in an external xattr block and
  1314		 * prevent its deduplication.
  1315		 */
  1316		if (encrypt) {
  1317			err = fscrypt_set_context(inode, handle);
  1318			if (err)
  1319				goto fail_free_drop;
  1320		}
  1321	
  1322		if (!(ei->i_flags & EXT4_EA_INODE_FL)) {
  1323			err = ext4_init_acl(handle, inode, dir);
  1324			if (err)
  1325				goto fail_free_drop;
  1326	
  1327			err = ext4_init_security(handle, inode, dir, qstr);
  1328			if (err)
  1329				goto fail_free_drop;
  1330		}
  1331	
  1332		if (ext4_has_feature_extents(sb)) {
  1333			/* set extent flag only for directory, file and normal symlink*/
  1334			if (S_ISDIR(mode) || S_ISREG(mode) || S_ISLNK(mode)) {
  1335				ext4_set_inode_flag(inode, EXT4_INODE_EXTENTS);
  1336				ext4_ext_tree_init(handle, inode);
  1337			}
  1338		}
  1339	
  1340		if (ext4_handle_valid(handle)) {
  1341			ei->i_sync_tid = handle->h_transaction->t_tid;
  1342			ei->i_datasync_tid = handle->h_transaction->t_tid;
  1343		}
  1344	
  1345		err = ext4_mark_inode_dirty(handle, inode);
  1346		if (err) {
  1347			ext4_std_error(sb, err);
  1348			goto fail_free_drop;
  1349		}
  1350	
  1351		ext4_debug("allocating inode %lu\n", inode->i_ino);
  1352		trace_ext4_allocate_inode(inode, dir, mode);
  1353		brelse(inode_bitmap_bh);
  1354		return ret;
  1355	
  1356	fail_free_drop:
  1357		dquot_free_inode(inode);
  1358	fail_drop:
  1359		clear_nlink(inode);
  1360		unlock_new_inode(inode);
  1361	out:
  1362		dquot_drop(inode);
  1363		inode->i_flags |= S_NOQUOTA;
  1364		iput(inode);
  1365		brelse(inode_bitmap_bh);
  1366		return ERR_PTR(err);
  1367	}
  1368	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH V4] ext4: check hash version and filesystem casefolded consistent
  2024-05-31  8:58       ` kernel test robot
@ 2024-05-31  9:06         ` Lizhi Xu
  2024-05-31 18:55           ` Eric Biggers
  0 siblings, 1 reply; 25+ messages in thread
From: Lizhi Xu @ 2024-05-31  9:06 UTC (permalink / raw)
  To: lkp
  Cc: coreteam, davem, ebiggers, fw, jaegeuk, kadlec, kuba,
	linux-fscrypt, linux-kernel, lizhi.xu, llvm, netdev,
	netfilter-devel, oe-kbuild-all, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso

When mounting the ext4 filesystem, if the hash version and casefolded are not
consistent, exit the mounting.

Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/ext4/super.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index c682fb927b64..0ad326504c50 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5262,6 +5262,11 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 		goto failed_mount;
 
 	ext4_hash_info_init(sb);
+	if (es->s_def_hash_version == DX_HASH_SIPHASH && 
+	    !ext4_has_feature_casefold(sb)) {
+		err = -EINVAL;
+		goto failed_mount;
+	}
 
 	err = ext4_handle_clustersize(sb);
 	if (err)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH V4] ext4: check hash version and filesystem casefolded consistent
  2024-05-31  9:06         ` [PATCH V4] ext4: check hash version and filesystem casefolded consistent Lizhi Xu
@ 2024-05-31 18:55           ` Eric Biggers
  2024-06-01 11:37             ` [PATCH V5] " Lizhi Xu
  0 siblings, 1 reply; 25+ messages in thread
From: Eric Biggers @ 2024-05-31 18:55 UTC (permalink / raw)
  To: Lizhi Xu
  Cc: lkp, coreteam, davem, fw, jaegeuk, kadlec, kuba, linux-fscrypt,
	linux-kernel, llvm, netdev, netfilter-devel, oe-kbuild-all, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso

On Fri, May 31, 2024 at 05:06:11PM +0800, 'Lizhi Xu' via syzkaller-bugs wrote:
> When mounting the ext4 filesystem, if the hash version and casefolded are not
> consistent, exit the mounting.
> 
> Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
>  fs/ext4/super.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c682fb927b64..0ad326504c50 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5262,6 +5262,11 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  		goto failed_mount;
>  
>  	ext4_hash_info_init(sb);
> +	if (es->s_def_hash_version == DX_HASH_SIPHASH && 
> +	    !ext4_has_feature_casefold(sb)) {
> +		err = -EINVAL;
> +		goto failed_mount;
> +	}

For the third time: you need to use the correct mailing lists.
Please follow Documentation/process/submitting-patches.rst.

- Eric

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH V5] ext4: check hash version and filesystem casefolded consistent
  2024-05-31 18:55           ` Eric Biggers
@ 2024-06-01 11:37             ` Lizhi Xu
  2024-06-03 14:50               ` Gabriel Krisman Bertazi
  0 siblings, 1 reply; 25+ messages in thread
From: Lizhi Xu @ 2024-06-01 11:37 UTC (permalink / raw)
  To: lkp
  Cc: coreteam, davem, ebiggers, fw, jaegeuk, kadlec, kuba,
	linux-fscrypt, linux-kernel, lizhi.xu, llvm, netdev,
	netfilter-devel, oe-kbuild-all, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso,
	adilger.kernel, linux-ext4

When mounting the ext4 filesystem, if the hash version and casefolded are not
consistent, exit the mounting.

Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/ext4/super.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index c682fb927b64..0ad326504c50 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5262,6 +5262,11 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 		goto failed_mount;
 
 	ext4_hash_info_init(sb);
+	if (es->s_def_hash_version == DX_HASH_SIPHASH && 
+	    !ext4_has_feature_casefold(sb)) {
+		err = -EINVAL;
+		goto failed_mount;
+	}
 
 	err = ext4_handle_clustersize(sb);
 	if (err)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH V5] ext4: check hash version and filesystem casefolded consistent
  2024-06-01 11:37             ` [PATCH V5] " Lizhi Xu
@ 2024-06-03 14:50               ` Gabriel Krisman Bertazi
  2024-06-04  1:17                 ` Lizhi Xu
  0 siblings, 1 reply; 25+ messages in thread
From: Gabriel Krisman Bertazi @ 2024-06-03 14:50 UTC (permalink / raw)
  To: Lizhi Xu
  Cc: lkp, coreteam, davem, ebiggers, fw, jaegeuk, kadlec, kuba,
	linux-fscrypt, linux-kernel, llvm, netdev, netfilter-devel,
	oe-kbuild-all, pablo, syzbot+340581ba9dceb7e06fb3, syzkaller-bugs,
	tytso, adilger.kernel, linux-ext4

Lizhi Xu <lizhi.xu@windriver.com> writes:

> When mounting the ext4 filesystem, if the hash version and casefolded are not
> consistent, exit the mounting.
>
> Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
>  fs/ext4/super.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index c682fb927b64..0ad326504c50 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5262,6 +5262,11 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  		goto failed_mount;
>  
>  	ext4_hash_info_init(sb);
> +	if (es->s_def_hash_version == DX_HASH_SIPHASH && 
> +	    !ext4_has_feature_casefold(sb)) {

Can we ever have DX_HASH_SIPHASH set up in the super block?  I thought
it was used solely for directories where ext4_hash_in_dirent(inode) is
true.

If this is only for the case of a superblock corruption, perhaps we
should always reject the mount, whether casefold is enabled or not?

-- 
Gabriel Krisman Bertazi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V5] ext4: check hash version and filesystem casefolded consistent
  2024-06-03 14:50               ` Gabriel Krisman Bertazi
@ 2024-06-04  1:17                 ` Lizhi Xu
  2024-06-04 19:06                   ` Gabriel Krisman Bertazi
  0 siblings, 1 reply; 25+ messages in thread
From: Lizhi Xu @ 2024-06-04  1:17 UTC (permalink / raw)
  To: krisman
  Cc: adilger.kernel, coreteam, davem, ebiggers, fw, jaegeuk, kadlec,
	kuba, linux-ext4, linux-fscrypt, linux-kernel, lizhi.xu, lkp,
	llvm, netdev, netfilter-devel, oe-kbuild-all, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso

On Mon, 03 Jun 2024 10:50:51 -0400, Gabriel Krisman Bertazi wrote:
> > When mounting the ext4 filesystem, if the hash version and casefolded are not
> > consistent, exit the mounting.
> >
> > Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
> > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > ---
> >  fs/ext4/super.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > index c682fb927b64..0ad326504c50 100644
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> > @@ -5262,6 +5262,11 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> >  		goto failed_mount;
> >  
> >  	ext4_hash_info_init(sb);
> > +	if (es->s_def_hash_version == DX_HASH_SIPHASH && 
> > +	    !ext4_has_feature_casefold(sb)) {
> 
> Can we ever have DX_HASH_SIPHASH set up in the super block?  I thought
> it was used solely for directories where ext4_hash_in_dirent(inode) is
> true.
The value of s'def_hash_version is obtained by reading the super block from the
buffer cache of the block device in ext4_load_super().
> 
> If this is only for the case of a superblock corruption, perhaps we
> should always reject the mount, whether casefold is enabled or not?
Based on the existing information, it cannot be confirmed whether the superblock
is corrupt, but one thing is clear: if the default hash version of the superblock
is set to DX_HASH_SIPHASH, but the casefold feature is not set at the same time,
it is definitely an error.

Lizhi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V3] ext4: check hash version and filesystem casefolded consistent
  2024-05-31  8:56         ` [PATCH V3] ext4: check hash version and filesystem casefolded consistent Lizhi Xu
@ 2024-06-04  9:27           ` Dan Carpenter
  2024-06-04  9:36             ` Lizhi Xu
  0 siblings, 1 reply; 25+ messages in thread
From: Dan Carpenter @ 2024-06-04  9:27 UTC (permalink / raw)
  To: oe-kbuild, Lizhi Xu, ebiggers
  Cc: lkp, oe-kbuild-all, adilger.kernel, coreteam, davem, fw, jaegeuk,
	kadlec, kuba, linux-ext4, linux-fscrypt, linux-kernel, lizhi.xu,
	netdev, netfilter-devel, pablo, syzbot+340581ba9dceb7e06fb3,
	syzkaller-bugs, tytso

Hi Lizhi,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Lizhi-Xu/ext4-check-hash-version-and-filesystem-casefolded-consistent/20240531-170046
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20240531085647.2918240-1-lizhi.xu%40windriver.com
patch subject: [PATCH V3] ext4: check hash version and filesystem casefolded consistent
config: i386-randconfig-141-20240601 (https://download.01.org/0day-ci/archive/20240602/202406020752.Ii2MU4KP-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202406020752.Ii2MU4KP-lkp@intel.com/

smatch warnings:
fs/ext4/super.c:5287 __ext4_fill_super() warn: missing error code 'err'

vim +/err +5287 fs/ext4/super.c

d4fab7b28e2f5d7 Theodore Ts'o           2023-04-27  5280  	err = ext4_block_group_meta_init(sb, silent);
d4fab7b28e2f5d7 Theodore Ts'o           2023-04-27  5281  	if (err)
0d1ee42f27d30ee Alexandre Ratchov       2006-10-11  5282  		goto failed_mount;
0b8e58a140cae2b Andreas Dilger          2009-06-03  5283  
db9345d9e6f075e Jason Yan               2023-03-23  5284  	ext4_hash_info_init(sb);
66b3f078839bbdb Lizhi Xu                2024-05-31  5285  	if (es->s_def_hash_version == DX_HASH_SIPHASH && 
66b3f078839bbdb Lizhi Xu                2024-05-31  5286  	    !ext4_has_feature_casefold(sb))
66b3f078839bbdb Lizhi Xu                2024-05-31 @5287  		goto failed_mount;


Should this be an error path?  err = something?

ac27a0ec112a089 Dave Kleikamp           2006-10-11  5288  
d4fab7b28e2f5d7 Theodore Ts'o           2023-04-27  5289  	err = ext4_handle_clustersize(sb);
d4fab7b28e2f5d7 Theodore Ts'o           2023-04-27  5290  	if (err)
281b59959707dfa Theodore Ts'o           2011-09-09  5291  		goto failed_mount;
960fd856fdc3b08 Theodore Ts'o           2013-07-05  5292  
d4fab7b28e2f5d7 Theodore Ts'o           2023-04-27  5293  	err = ext4_check_geometry(sb, es);
d4fab7b28e2f5d7 Theodore Ts'o           2023-04-27  5294  	if (err)
bfe0a5f47ada40d Theodore Ts'o           2018-06-17  5295  		goto failed_mount;
bfe0a5f47ada40d Theodore Ts'o           2018-06-17  5296  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V3] ext4: check hash version and filesystem casefolded consistent
  2024-06-04  9:27           ` Dan Carpenter
@ 2024-06-04  9:36             ` Lizhi Xu
  0 siblings, 0 replies; 25+ messages in thread
From: Lizhi Xu @ 2024-06-04  9:36 UTC (permalink / raw)
  To: dan.carpenter
  Cc: adilger.kernel, coreteam, davem, ebiggers, fw, jaegeuk, kadlec,
	kuba, linux-ext4, linux-fscrypt, linux-kernel, lizhi.xu, lkp,
	netdev, netfilter-devel, oe-kbuild-all, oe-kbuild, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso

On Tue, 4 Jun 2024 12:27:06 +0300, Dan Carpenter wrote: 
> kernel test robot noticed the following build warnings:
> 
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Lizhi-Xu/ext4-check-hash-version-and-filesystem-casefolded-consistent/20240531-170046
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
> patch link:    https://lore.kernel.org/r/20240531085647.2918240-1-lizhi.xu%40windriver.com
> patch subject: [PATCH V3] ext4: check hash version and filesystem casefolded consistent
> config: i386-randconfig-141-20240601 (https://download.01.org/0day-ci/archive/20240602/202406020752.Ii2MU4KP-lkp@intel.com/config)
> compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202406020752.Ii2MU4KP-lkp@intel.com/
> 
> smatch warnings:
> fs/ext4/super.c:5287 __ext4_fill_super() warn: missing error code 'err'
> 
> vim +/err +5287 fs/ext4/super.c
> 
> d4fab7b28e2f5d7 Theodore Ts'o           2023-04-27  5280  	err = ext4_block_group_meta_init(sb, silent);
> d4fab7b28e2f5d7 Theodore Ts'o           2023-04-27  5281  	if (err)
> 0d1ee42f27d30ee Alexandre Ratchov       2006-10-11  5282  		goto failed_mount;
> 0b8e58a140cae2b Andreas Dilger          2009-06-03  5283
> db9345d9e6f075e Jason Yan               2023-03-23  5284  	ext4_hash_info_init(sb);
> 66b3f078839bbdb Lizhi Xu                2024-05-31  5285  	if (es->s_def_hash_version == DX_HASH_SIPHASH &&
> 66b3f078839bbdb Lizhi Xu                2024-05-31  5286  	    !ext4_has_feature_casefold(sb))
> 66b3f078839bbdb Lizhi Xu                2024-05-31 @5287  		goto failed_mount;
This warning has been resolved in the following patch: 
https://lore.kernel.org/all/20240601113749.473058-1-lizhi.xu@windriver.com/

Lizhi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V5] ext4: check hash version and filesystem casefolded consistent
  2024-06-04  1:17                 ` Lizhi Xu
@ 2024-06-04 19:06                   ` Gabriel Krisman Bertazi
  2024-06-05  1:16                     ` Lizhi Xu
                                       ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Gabriel Krisman Bertazi @ 2024-06-04 19:06 UTC (permalink / raw)
  To: Lizhi Xu
  Cc: adilger.kernel, coreteam, davem, ebiggers, fw, jaegeuk, kadlec,
	kuba, linux-ext4, linux-fscrypt, linux-kernel, lkp, llvm, netdev,
	netfilter-devel, oe-kbuild-all, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso

Lizhi Xu <lizhi.xu@windriver.com> writes:

> On Mon, 03 Jun 2024 10:50:51 -0400, Gabriel Krisman Bertazi wrote:
>> > When mounting the ext4 filesystem, if the hash version and casefolded are not
>> > consistent, exit the mounting.
>> >
>> > Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
>> > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
>> > ---
>> >  fs/ext4/super.c | 5 +++++
>> >  1 file changed, 5 insertions(+)
>> >
>> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> > index c682fb927b64..0ad326504c50 100644
>> > --- a/fs/ext4/super.c
>> > +++ b/fs/ext4/super.c
>> > @@ -5262,6 +5262,11 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>> >  		goto failed_mount;
>> >  
>> >  	ext4_hash_info_init(sb);
>> > +	if (es->s_def_hash_version == DX_HASH_SIPHASH && 
>> > +	    !ext4_has_feature_casefold(sb)) {
>> 
>> Can we ever have DX_HASH_SIPHASH set up in the super block?  I thought
>> it was used solely for directories where ext4_hash_in_dirent(inode) is
>> true.
> The value of s'def_hash_version is obtained by reading the super block from the
> buffer cache of the block device in ext4_load_super().

Yes, I know.  My point is whether this check should just be:

if (es->s_def_hash_version == DX_HASH_SIPHASH)
	goto failed_mount;

Since, IIUC, DX_HASH_SIPHASH is done per-directory and not written to
the sb.

>> If this is only for the case of a superblock corruption, perhaps we
>> should always reject the mount, whether casefold is enabled or not?
> Based on the existing information, it cannot be confirmed whether the superblock
> is corrupt, but one thing is clear: if the default hash version of the superblock
> is set to DX_HASH_SIPHASH, but the casefold feature is not set at the same time,
> it is definitely an error.


-- 
Gabriel Krisman Bertazi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V5] ext4: check hash version and filesystem casefolded consistent
  2024-06-04 19:06                   ` Gabriel Krisman Bertazi
@ 2024-06-05  1:16                     ` Lizhi Xu
  2024-06-05  1:23                     ` [PATCH V6] fs/ext4: Filesystem without casefold feature cannot be mounted with spihash Lizhi Xu
  2024-06-06  6:27                     ` [PATCH V5] ext4: check hash version and filesystem casefolded consistent Eric Biggers
  2 siblings, 0 replies; 25+ messages in thread
From: Lizhi Xu @ 2024-06-05  1:16 UTC (permalink / raw)
  To: krisman
  Cc: adilger.kernel, coreteam, davem, ebiggers, fw, jaegeuk, kadlec,
	kuba, linux-ext4, linux-fscrypt, linux-kernel, lizhi.xu, lkp,
	llvm, netdev, netfilter-devel, oe-kbuild-all, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso

On Tue, 04 Jun 2024 15:06:32 -0400, Gabriel Krisman Bertazi wrote:
> >> > When mounting the ext4 filesystem, if the hash version and casefolded are not
> >> > consistent, exit the mounting.
> >> >
> >> > Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
> >> > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> >> > ---
> >> >  fs/ext4/super.c | 5 +++++
> >> >  1 file changed, 5 insertions(+)
> >> >
> >> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> >> > index c682fb927b64..0ad326504c50 100644
> >> > --- a/fs/ext4/super.c
> >> > +++ b/fs/ext4/super.c
> >> > @@ -5262,6 +5262,11 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> >> >  		goto failed_mount;
> >> >  
> >> >  	ext4_hash_info_init(sb);
> >> > +	if (es->s_def_hash_version == DX_HASH_SIPHASH && 
> >> > +	    !ext4_has_feature_casefold(sb)) {
> >> 
> >> Can we ever have DX_HASH_SIPHASH set up in the super block?  I thought
> >> it was used solely for directories where ext4_hash_in_dirent(inode) is
> >> true.
> > The value of s'def_hash_version is obtained by reading the super block from the
> > buffer cache of the block device in ext4_load_super().
> 
> Yes, I know.  My point is whether this check should just be:
Based on the existing information, it cannot be confirmed that it is incorrect
to separately determine the value of s_def_hash_version as DX_HASH_SIPHASH.
Additionally, I have come up with a better solution, and I will issue the next
fixed version in a while.
> 
> if (es->s_def_hash_version == DX_HASH_SIPHASH)
> 	goto failed_mount;
> 
> Since, IIUC, DX_HASH_SIPHASH is done per-directory and not written to
> the sb.
> 
> >> If this is only for the case of a superblock corruption, perhaps we
> >> should always reject the mount, whether casefold is enabled or not?
> > Based on the existing information, it cannot be confirmed whether the superblock
> > is corrupt, but one thing is clear: if the default hash version of the superblock
> > is set to DX_HASH_SIPHASH, but the casefold feature is not set at the same time,
> > it is definitely an error.

Lizhi

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH V6] fs/ext4: Filesystem without casefold feature cannot be mounted with spihash
  2024-06-04 19:06                   ` Gabriel Krisman Bertazi
  2024-06-05  1:16                     ` Lizhi Xu
@ 2024-06-05  1:23                     ` Lizhi Xu
  2024-08-22 15:00                       ` Theodore Ts'o
  2024-06-06  6:27                     ` [PATCH V5] ext4: check hash version and filesystem casefolded consistent Eric Biggers
  2 siblings, 1 reply; 25+ messages in thread
From: Lizhi Xu @ 2024-06-05  1:23 UTC (permalink / raw)
  To: krisman
  Cc: adilger.kernel, coreteam, davem, ebiggers, fw, jaegeuk, kadlec,
	kuba, linux-ext4, linux-fscrypt, linux-kernel, lizhi.xu, lkp,
	llvm, netdev, netfilter-devel, oe-kbuild-all, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso

When mounting the ext4 filesystem, if the default hash version is set to 
DX_HASH_SIPHASH but the casefold feature is not set, exit the mounting.

Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/ext4/super.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index c682fb927b64..d0645af3e66e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3593,6 +3593,14 @@ int ext4_feature_set_ok(struct super_block *sb, int readonly)
 			 "mounted without CONFIG_UNICODE");
 		return 0;
 	}
+#else
+	if (EXT4_SB(sb)->s_es->s_def_hash_version == DX_HASH_SIPHASH &&
+	    !ext4_has_feature_casefold(sb)) {
+		ext4_msg(sb, KERN_ERR,
+			 "Filesystem without casefold feature cannot be "
+			 "mounted with spihash");
+		return 0;
+	}
 #endif
 
 	if (readonly)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH V5] ext4: check hash version and filesystem casefolded consistent
  2024-06-04 19:06                   ` Gabriel Krisman Bertazi
  2024-06-05  1:16                     ` Lizhi Xu
  2024-06-05  1:23                     ` [PATCH V6] fs/ext4: Filesystem without casefold feature cannot be mounted with spihash Lizhi Xu
@ 2024-06-06  6:27                     ` Eric Biggers
  2 siblings, 0 replies; 25+ messages in thread
From: Eric Biggers @ 2024-06-06  6:27 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi
  Cc: Lizhi Xu, adilger.kernel, coreteam, davem, fw, jaegeuk, kadlec,
	kuba, linux-ext4, linux-fscrypt, linux-kernel, lkp, llvm, netdev,
	netfilter-devel, oe-kbuild-all, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs, tytso

On Tue, Jun 04, 2024 at 03:06:32PM -0400, Gabriel Krisman Bertazi wrote:
> Lizhi Xu <lizhi.xu@windriver.com> writes:
> 
> > On Mon, 03 Jun 2024 10:50:51 -0400, Gabriel Krisman Bertazi wrote:
> >> > When mounting the ext4 filesystem, if the hash version and casefolded are not
> >> > consistent, exit the mounting.
> >> >
> >> > Reported-by: syzbot+340581ba9dceb7e06fb3@syzkaller.appspotmail.com
> >> > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> >> > ---
> >> >  fs/ext4/super.c | 5 +++++
> >> >  1 file changed, 5 insertions(+)
> >> >
> >> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> >> > index c682fb927b64..0ad326504c50 100644
> >> > --- a/fs/ext4/super.c
> >> > +++ b/fs/ext4/super.c
> >> > @@ -5262,6 +5262,11 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> >> >  		goto failed_mount;
> >> >  
> >> >  	ext4_hash_info_init(sb);
> >> > +	if (es->s_def_hash_version == DX_HASH_SIPHASH && 
> >> > +	    !ext4_has_feature_casefold(sb)) {
> >> 
> >> Can we ever have DX_HASH_SIPHASH set up in the super block?  I thought
> >> it was used solely for directories where ext4_hash_in_dirent(inode) is
> >> true.
> > The value of s'def_hash_version is obtained by reading the super block from the
> > buffer cache of the block device in ext4_load_super().
> 
> Yes, I know.  My point is whether this check should just be:
> 
> if (es->s_def_hash_version == DX_HASH_SIPHASH)
> 	goto failed_mount;
> 
> Since, IIUC, DX_HASH_SIPHASH is done per-directory and not written to
> the sb.
> 

That seems right to me.  SipHash can never be the default because it's only used
on directories that are both encrypted and casefolded.

- Eric

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V6] fs/ext4: Filesystem without casefold feature cannot be mounted with spihash
  2024-06-05  1:23                     ` [PATCH V6] fs/ext4: Filesystem without casefold feature cannot be mounted with spihash Lizhi Xu
@ 2024-08-22 15:00                       ` Theodore Ts'o
  2024-08-27 20:16                         ` [PATCH] ext4: Fix error message when rejecting the default hash Gabriel Krisman Bertazi
  0 siblings, 1 reply; 25+ messages in thread
From: Theodore Ts'o @ 2024-08-22 15:00 UTC (permalink / raw)
  To: krisman, Lizhi Xu
  Cc: Theodore Ts'o, adilger.kernel, coreteam, davem, ebiggers, fw,
	jaegeuk, kadlec, kuba, linux-ext4, linux-fscrypt, linux-kernel,
	lkp, llvm, netdev, netfilter-devel, oe-kbuild-all, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs


On Wed, 05 Jun 2024 09:23:35 +0800, Lizhi Xu wrote:
> When mounting the ext4 filesystem, if the default hash version is set to
> DX_HASH_SIPHASH but the casefold feature is not set, exit the mounting.
> 
> 

Applied, thanks!

[1/1] fs/ext4: Filesystem without casefold feature cannot be mounted with spihash
      commit: 985b67cd86392310d9e9326de941c22fc9340eec

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] ext4: Fix error message when rejecting the default hash
  2024-08-22 15:00                       ` Theodore Ts'o
@ 2024-08-27 20:16                         ` Gabriel Krisman Bertazi
  2024-09-05 14:53                           ` Theodore Ts'o
  0 siblings, 1 reply; 25+ messages in thread
From: Gabriel Krisman Bertazi @ 2024-08-27 20:16 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: Lizhi Xu, adilger.kernel, coreteam, davem, ebiggers, fw, jaegeuk,
	kadlec, kuba, linux-ext4, linux-fscrypt, linux-kernel, lkp, llvm,
	netdev, netfilter-devel, oe-kbuild-all, pablo,
	syzbot+340581ba9dceb7e06fb3, syzkaller-bugs

"Theodore Ts'o" <tytso@mit.edu> writes:

> On Wed, 05 Jun 2024 09:23:35 +0800, Lizhi Xu wrote:
>> When mounting the ext4 filesystem, if the default hash version is set to
>> DX_HASH_SIPHASH but the casefold feature is not set, exit the mounting.
>> 
>> 
>
> Applied, thanks!
>
> [1/1] fs/ext4: Filesystem without casefold feature cannot be mounted with spihash
>       commit: 985b67cd86392310d9e9326de941c22fc9340eec

Ted,

Since you took the above, can you please consider the following fixup?
I had pointed we shouldn't have siphash as the sb default hash at all:

based on your dev branch.

>8
Subject: [PATCH] ext4: Fix error message when rejecting the default hash

Commit 985b67cd8639 ("ext4: filesystems without casefold feature cannot
be mounted with siphash") properly rejects volumes where
s_def_hash_version is set to DX_HASH_SIPHASH, but the check and the
error message should not look into casefold setup - a filesystem should
never have DX_HASH_SIPHASH as the default hash.  Fix it and, since we
are there, move the check to ext4_hash_info_init.

Fixes:985b67cd8639 ("ext4: filesystems without casefold feature cannot
be mounted with siphash")
Signed-off-by: Gabriel Krisman Bertazi <krisman@suse.de>
---
 fs/ext4/ext4.h  |  1 +
 fs/ext4/super.c | 27 +++++++++++++++++----------
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 5845e4aa091a..4120f24880cb 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2462,6 +2462,7 @@ static inline __le16 ext4_rec_len_to_disk(unsigned len, unsigned blocksize)
 #define DX_HASH_HALF_MD4_UNSIGNED	4
 #define DX_HASH_TEA_UNSIGNED		5
 #define DX_HASH_SIPHASH			6
+#define DX_HASH_LAST 			DX_HASH_SIPHASH
 
 static inline u32 ext4_chksum(struct ext4_sb_info *sbi, u32 crc,
 			      const void *address, unsigned int length)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 25cd0d662e31..c6a34ad07ecc 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3582,13 +3582,6 @@ int ext4_feature_set_ok(struct super_block *sb, int readonly)
 			 "mounted without CONFIG_UNICODE");
 		return 0;
 	}
-	if (EXT4_SB(sb)->s_es->s_def_hash_version == DX_HASH_SIPHASH &&
-	    !ext4_has_feature_casefold(sb)) {
-		ext4_msg(sb, KERN_ERR,
-			 "Filesystem without casefold feature cannot be "
-			 "mounted with siphash");
-		return 0;
-	}
 
 	if (readonly)
 		return 1;
@@ -5094,16 +5087,27 @@ static int ext4_load_super(struct super_block *sb, ext4_fsblk_t *lsb,
 	return ret;
 }
 
-static void ext4_hash_info_init(struct super_block *sb)
+static int ext4_hash_info_init(struct super_block *sb)
 {
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct ext4_super_block *es = sbi->s_es;
 	unsigned int i;
 
+	sbi->s_def_hash_version = es->s_def_hash_version;
+
+	if (sbi->s_def_hash_version > DX_HASH_LAST) {
+		ext4_msg(sb, KERN_ERR,
+			 "Invalid default hash set in the superblock");
+		return -EINVAL;
+	} else if (sbi->s_def_hash_version == DX_HASH_SIPHASH) {
+		ext4_msg(sb, KERN_ERR,
+			 "SIPHASH is not a valid default hash value");
+		return -EINVAL;
+	}
+
 	for (i = 0; i < 4; i++)
 		sbi->s_hash_seed[i] = le32_to_cpu(es->s_hash_seed[i]);
 
-	sbi->s_def_hash_version = es->s_def_hash_version;
 	if (ext4_has_feature_dir_index(sb)) {
 		i = le32_to_cpu(es->s_flags);
 		if (i & EXT2_FLAGS_UNSIGNED_HASH)
@@ -5121,6 +5125,7 @@ static void ext4_hash_info_init(struct super_block *sb)
 #endif
 		}
 	}
+	return 0;
 }
 
 static int ext4_block_group_meta_init(struct super_block *sb, int silent)
@@ -5256,7 +5261,9 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 	if (err)
 		goto failed_mount;
 
-	ext4_hash_info_init(sb);
+	err = ext4_hash_info_init(sb);
+	if (err)
+		goto failed_mount;
 
 	err = ext4_handle_clustersize(sb);
 	if (err)
-- 
2.46.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] ext4: Fix error message when rejecting the default hash
  2024-08-27 20:16                         ` [PATCH] ext4: Fix error message when rejecting the default hash Gabriel Krisman Bertazi
@ 2024-09-05 14:53                           ` Theodore Ts'o
  0 siblings, 0 replies; 25+ messages in thread
From: Theodore Ts'o @ 2024-09-05 14:53 UTC (permalink / raw)
  To: Gabriel Krisman Bertazi
  Cc: Theodore Ts'o, Lizhi Xu, adilger.kernel, coreteam, davem,
	ebiggers, fw, jaegeuk, kadlec, kuba, linux-ext4, linux-fscrypt,
	linux-kernel, lkp, llvm, netdev, netfilter-devel, oe-kbuild-all,
	pablo, syzbot+340581ba9dceb7e06fb3, syzkaller-bugs


On Tue, 27 Aug 2024 16:16:36 -0400, Gabriel Krisman Bertazi wrote:
> "Theodore Ts'o" <tytso@mit.edu> writes:
> 
> > On Wed, 05 Jun 2024 09:23:35 +0800, Lizhi Xu wrote:
> >> When mounting the ext4 filesystem, if the default hash version is set to
> >> DX_HASH_SIPHASH but the casefold feature is not set, exit the mounting.
> >>
> >>
> >
> > Applied, thanks!
> >
> > [1/1] fs/ext4: Filesystem without casefold feature cannot be mounted with spihash
> >       commit: 985b67cd86392310d9e9326de941c22fc9340eec
> 
> [...]

Applied, thanks!

[1/1] ext4: Fix error message when rejecting the default hash
      commit: a2187431c395cdfbf144e3536f25468c64fc7cfa

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2024-09-05 14:54 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-29 20:41 [syzbot] [fscrypt?] WARNING in fscrypt_fname_siphash syzbot
2024-05-30  7:41 ` [PATCH] ext4: add casefolded file check Lizhi Xu
2024-05-31  1:05   ` Eric Biggers
2024-05-31  1:47     ` Lizhi Xu
2024-05-31  2:20       ` Eric Biggers
2024-05-31  3:07     ` [PATCH V2] ext4: add casefolded feature check before setup encrypted info Lizhi Xu
2024-05-31  3:11       ` Eric Biggers
2024-05-31  8:58       ` kernel test robot
2024-05-31  9:06         ` [PATCH V4] ext4: check hash version and filesystem casefolded consistent Lizhi Xu
2024-05-31 18:55           ` Eric Biggers
2024-06-01 11:37             ` [PATCH V5] " Lizhi Xu
2024-06-03 14:50               ` Gabriel Krisman Bertazi
2024-06-04  1:17                 ` Lizhi Xu
2024-06-04 19:06                   ` Gabriel Krisman Bertazi
2024-06-05  1:16                     ` Lizhi Xu
2024-06-05  1:23                     ` [PATCH V6] fs/ext4: Filesystem without casefold feature cannot be mounted with spihash Lizhi Xu
2024-08-22 15:00                       ` Theodore Ts'o
2024-08-27 20:16                         ` [PATCH] ext4: Fix error message when rejecting the default hash Gabriel Krisman Bertazi
2024-09-05 14:53                           ` Theodore Ts'o
2024-06-06  6:27                     ` [PATCH V5] ext4: check hash version and filesystem casefolded consistent Eric Biggers
2024-05-31  3:30     ` [PATCH V2] ext4: add casefolded feature check before setup encrypted info Lizhi Xu
2024-05-31  3:34       ` Eric Biggers
2024-05-31  8:56         ` [PATCH V3] ext4: check hash version and filesystem casefolded consistent Lizhi Xu
2024-06-04  9:27           ` Dan Carpenter
2024-06-04  9:36             ` Lizhi Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).