[syzbot] [fscrypt?] WARNING in fscrypt_fname_siphash

[syzbot]

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 <f...@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+340581...@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 syzk...@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

[Lizhi Xu]

#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 74eca356f6d4

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");

[syzbot]

Hello,

syzbot tried to test the proposed patch but the build/boot failed:

lost connection to test machine





syzkaller build log:
go env (err=<nil>)
GO111MODULE='auto'
GOARCH='amd64'
GOBIN=''
GOCACHE='/syzkaller/.cache/go-build'
GOENV='/syzkaller/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/syzkaller/jobs-2/linux/gopath/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/syzkaller/jobs-2/linux/gopath'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.4'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/syzkaller/jobs-2/linux/gopath/src/github.com/google/syzkaller/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2598401150=/tmp/go-build -gno-record-gcc-switches'

git status (err=<nil>)
HEAD detached at a10a183e2
nothing to commit, working tree clean


tput: No value for $TERM and no -T specified
tput: No value for $TERM and no -T specified
Makefile:31: run command via tools/syz-env for best compatibility, see:
Makefile:32: https://github.com/google/syzkaller/blob/master/docs/contributing.md#using-syz-env
go list -f '{{.Stale}}' ./sys/syz-sysgen | grep -q false || go install ./sys/syz-sysgen
make .descriptions
tput: No value for $TERM and no -T specified
tput: No value for $TERM and no -T specified
Makefile:31: run command via tools/syz-env for best compatibility, see:
Makefile:32: https://github.com/google/syzkaller/blob/master/docs/contributing.md#using-syz-env
bin/syz-sysgen
touch .descriptions
GOOS=linux GOARCH=amd64 go build "-ldflags=-s -w -X github.com/google/syzkaller/prog.GitRevision=a10a183e260f0ea1a0c37e84ca5c60f28c13e3fd -X 'github.com/google/syzkaller/prog.gitRevisionDate=20240524-152400'" "-tags=syz_target syz_os_linux syz_arch_amd64 " -o ./bin/linux_amd64/syz-fuzzer github.com/google/syzkaller/syz-fuzzer
GOOS=linux GOARCH=amd64 go build "-ldflags=-s -w -X github.com/google/syzkaller/prog.GitRevision=a10a183e260f0ea1a0c37e84ca5c60f28c13e3fd -X 'github.com/google/syzkaller/prog.gitRevisionDate=20240524-152400'" "-tags=syz_target syz_os_linux syz_arch_amd64 " -o ./bin/linux_amd64/syz-execprog github.com/google/syzkaller/tools/syz-execprog
mkdir -p ./bin/linux_amd64
g++ -o ./bin/linux_amd64/syz-executor executor/executor.cc \
-m64 -O2 -pthread -Wall -Werror -Wparentheses -Wunused-const-variable -Wframe-larger-than=16384 -Wno-stringop-overflow -Wno-array-bounds -Wno-format-overflow -Wno-unused-but-set-variable -Wno-unused-command-line-argument -static-pie -std=c++14 -I. -Iexecutor/_include -fpermissive -w -DGOOS_linux=1 -DGOARCH_amd64=1 \
-DHOSTGOOS_linux=1 -DGIT_REVISION=\"a10a183e260f0ea1a0c37e84ca5c60f28c13e3fd\"



Tested on:

commit: 74eca356 Merge tag 'ceph-for-6.10-rc1' of https://gith..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

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

patch: https://syzkaller.appspot.com/x/patch.diff?x=1220bb3a980000

[Lizhi Xu]

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+340581...@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizh...@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

[Eric Biggers]

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

[Lizhi Xu]

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

[Eric Biggers]

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

[Lizhi Xu]

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+340581...@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizh...@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

[Eric Biggers]

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

[Lizhi Xu]

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+340581...@syzkaller.appspotmail.com
> > Signed-off-by: Lizhi Xu <lizh...@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

[Eric Biggers]

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

- Eric

[Lizhi Xu]

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

Reported-by: syzbot+340581...@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizh...@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

[kernel test robot]

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...@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...@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 <l...@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405311607...@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

[Lizhi Xu]

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

Reported-by: syzbot+340581...@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizh...@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;
+ }

[Eric Biggers]

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

- Eric

[Lizhi Xu]

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

Reported-by: syzbot+340581...@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizh...@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;
+ }

[Gabriel Krisman Bertazi]

Lizhi Xu <lizh...@windriver.com> writes:

> When mounting the ext4 filesystem, if the hash version and casefolded are not
> consistent, exit the mounting.
>
> Reported-by: syzbot+340581...@syzkaller.appspotmail.com
> Signed-off-by: Lizhi Xu <lizh...@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

[Lizhi Xu]

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+340581...@syzkaller.appspotmail.com
> > Signed-off-by: Lizhi Xu <lizh...@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

[Dan Carpenter]

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...@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 <l...@intel.com>

| Reported-by: Dan Carpenter <dan.ca...@linaro.org>
| Closes: https://lore.kernel.org/r/202406020752...@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

[Lizhi Xu]

This warning has been resolved in the following patch:
https://lore.kernel.org/all/20240601113749.4...@windriver.com/

Lizhi

[Gabriel Krisman Bertazi]

Lizhi Xu <lizh...@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+340581...@syzkaller.appspotmail.com
>> > Signed-off-by: Lizhi Xu <lizh...@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

[Lizhi Xu]

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+340581...@syzkaller.appspotmail.com
> >> > Signed-off-by: Lizhi Xu <lizh...@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

[Lizhi Xu]

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+340581...@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizh...@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

[Eric Biggers]

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