Re: [PATCH stable/linux-5.10.y 3/7] fs: use backing_file container for internal files with "fake" f_path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] If `init_file()` fails in `alloc_empty_backing_file()`, the `kzalloc`'d memory is incorrectly freed to `filp_cachep`, causing SLUB corruption.
- [Medium] The `backing_file` container allocation lacks `__GFP_ACCOUNT`, bypassing memcg memory accounting.
--

commit 253dd7739e2dba161f4b1907e20e7723d9f6a637
Author: Amir Goldstein <amir73il@gmail.com>

fs: use backing_file container for internal files with "fake" f_path

Overlayfs uses open_with_fake_path() to allocate internal kernel files.
This commit introduces a backing_file container to hold the fake ovl path
along with the real path.

> diff --git a/fs/file_table.c b/fs/file_table.c
> index be24d724b4075..6daef2e2b2ada 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
[ ... ]
> @@ -189,6 +211,30 @@ struct file *alloc_empty_file_noaccount(int flags, const struct cred *cred)
>  	return f;
>  }
>  
> +/*
> + * Variant of alloc_empty_file() that allocates a backing_file container
> + * and doesn't check and modify nr_files.
> + *
> + * This is only for kernel internal use, and the allocate file must not be
> + * installed into file tables or such.
> + */
> +struct file *alloc_empty_backing_file(int flags, const struct cred *cred)
> +{
> +	struct backing_file *ff;
> +	int error;
> +
> +	ff = kzalloc(sizeof(struct backing_file), GFP_KERNEL);

[Severity: Medium]
Since this replaces an allocation that previously came from the filp_cachep
slab cache (which uses SLAB_ACCOUNT), should this kzalloc() include the
__GFP_ACCOUNT flag?

Without __GFP_ACCOUNT, this memory might bypass memcg accounting, allowing a
container to allocate kernel memory without being charged against its limit.

> +	if (unlikely(!ff))
> +		return ERR_PTR(-ENOMEM);
> +
> +	error = init_file(&ff->file, flags, cred);

[Severity: High]
If init_file() fails (for example, due to an error in security_file_alloc()),
it calls file_free_rcu().

Because FMODE_BACKING has not been set in f_mode yet, file_free_rcu() will
evaluate (f->f_mode & FMODE_BACKING) as false:

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

Will this cause the kzalloc'd ff pointer to be incorrectly freed into the
filp_cachep slab cache, leading to SLUB corruption?

> +	if (unlikely(error))
> +		return ERR_PTR(error);
> +
> +	ff->file.f_mode |= FMODE_BACKING | FMODE_NOACCOUNT;
> +	return &ff->file;
> +}

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