fs/ext4/hash.c | 4 ++++ 1 file changed, 4 insertions(+)
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.0On 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
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
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
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.0Hi 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
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
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
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
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.0Hi 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
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.0On 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
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.0Lizhi 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
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
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
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
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
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
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>
"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
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>
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
© 2016 - 2025 Red Hat, Inc.