Re: [PATCH stable/linux-5.10.y 5/7] fs: prepare for adding LSM blob to backing_file

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Slab allocator corruption and kernel panic when security_file_alloc() fails during backing file initialization.
--

commit 5ea328a29bcb61b7c30ea4dc685a4910998da08f
Author: Amir Goldstein <amir73il@gmail.com>

fs: prepare for adding LSM blob to backing_file

In preparation to adding LSM blob to backing_file struct, factor out
helpers init_backing_file() and backing_file_free().

> diff --git a/fs/file_table.c b/fs/file_table.c
> index 6daef2e2b2ada..6792d0bce2461 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c

[ ... ]

> @@ -231,7 +242,14 @@ struct file *alloc_empty_backing_file(int flags, const struct cred *cred)
>  	if (unlikely(error))
>  		return ERR_PTR(error);
>  
> +	/* The f_mode flags must be set before fput(). */
>  	ff->file.f_mode |= FMODE_BACKING | FMODE_NOACCOUNT;

[Severity: High]
Can this lead to slab corruption if init_file() fails?

In alloc_empty_backing_file(), the backing_file is allocated via kzalloc():

fs/file_table.c:alloc_empty_backing_file() {
    ...
	ff = kzalloc(sizeof(struct backing_file), GFP_KERNEL);
	if (unlikely(!ff))
		return ERR_PTR(-ENOMEM);

	error = init_file(&ff->file, flags, cred);
    ...
}

If security_file_alloc() fails inside init_file(), it triggers the cleanup
path using file_free_rcu():

fs/file_table.c:init_file() {
    ...
	error = security_file_alloc(f);
	if (unlikely(error)) {
		file_free_rcu(&f->f_u.fu_rcuhead);
		return error;
	}
    ...
}

Because FMODE_BACKING is not set until after init_file() returns,
file_free_rcu() will take the else branch and attempt to free the kzalloc'd
pointer into the filp_cachep kmem cache:

fs/file_table.c:file_free_rcu() {
    ...
	if (unlikely(f->f_mode & FMODE_BACKING))
		kfree(backing_file(f));
	else
		kmem_cache_free(filp_cachep, f);
}

Should FMODE_BACKING be set before calling init_file(), or is there a
different way this cleanup should be handled?

> +	error = init_backing_file(ff);
> +	if (unlikely(error)) {
> +		fput(&ff->file);
> +		return ERR_PTR(error);
> +	}
> +
>  	return &ff->file;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629070653.580879-1-caixinchen1@huawei.com?part=5