Re: [PATCH] ext2: Make sbi an proper alias for EXT2_SB(sb) in ext2_fill_super()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] ext2: 1-byte allocation in ext2_fill_super() via kzalloc_obj(*sb->s_fs_info)
--

commit 4e1ea984c67eb1fcebf92c70f142ca114ddb07b9
Author: Nathan Chancellor <nathan@kernel.org>

ext2: Make sbi an proper alias for EXT2_SB(sb) in ext2_fill_super()

This commit reworks the allocation and assignment of sbi in ext2_fill_super()
so that sbi is assigned directly from EXT2_SB(sb), allowing Clang's thread
safety context analysis to recognize that sbi and EXT2_SB(sb) refer to the
same object.

Link: https://github.com/llvm/llvm-project/commit/217e229a1f32785fccdd3b3f51429f0d309dea45 [1]
Link: https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#no-alias-analysis [2]

> diff --git a/fs/ext2/super.c b/fs/ext2/super.c
> index fb217ef2d69f0..b55f9ce3267f1 100644
> --- a/fs/ext2/super.c
> +++ b/fs/ext2/super.c
> @@ -883,17 +883,18 @@ static int ext2_fill_super(struct super_block *sb, struct fs_context *fc)
>  	__le32 features;
>  	int err;
>  
> -	sbi = kzalloc_obj(*sbi);
> -	if (!sbi)
> +	sb->s_fs_info = kzalloc_obj(*sb->s_fs_info);
> +	if (!sb->s_fs_info)
>  		return -ENOMEM;
> +	sbi = EXT2_SB(sb);

[Severity: Critical]
Does passing *sb->s_fs_info to kzalloc_obj() in ext2_fill_super() allocate
only 1 byte instead of sizeof(struct ext2_sb_info), causing an out-of-bounds
write on every ext2 mount?

In include/linux/fs/super_types.h, s_fs_info is declared as void *:

struct super_block {
	...
	void			*s_fs_info;	/* Filesystem private info */
	...
};

In include/linux/slab.h, kzalloc_obj() uses typeof(P) and sizeof(TYPE) to
compute the allocation size:

#define kzalloc_obj(P, ...) \
	__alloc_objs(kzalloc, default_gfp(__VA_ARGS__), typeof(P), 1)

#define __alloc_objs(KMALLOC, GFP, TYPE, COUNT)				\
({									\
	const size_t __obj_size = size_mul(sizeof(TYPE), COUNT);	\
	(TYPE *)KMALLOC(__obj_size, GFP);				\
})

Because sb->s_fs_info has type void *, dereferencing *sb->s_fs_info evaluates
typeof(*sb->s_fs_info) to void (and triggers a compiler warning when
dereferencing a void * pointer). Under GNU C, sizeof(void) evaluates to 1, so
kzalloc_obj(*sb->s_fs_info) allocates 1 byte before ext2_fill_super() writes
to fields of sbi = EXT2_SB(sb).

Should this pass *sbi (or struct ext2_sb_info) to kzalloc_obj() instead:

	sb->s_fs_info = kzalloc_obj(*sbi);

so that the full struct ext2_sb_info is allocated while still assigning
sbi = EXT2_SB(sb) afterward?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023-ext2-fix-context-analysis-clang-24-v1-1-2b35c33cd0fa@kernel.org?part=1