On Tue, 25 Mar 2025, Marc Dionne wrote:
> Commit c54b386969a5 ("VFS: Change vfs_mkdir() to return the dentry.")
> changed cachefiles_get_directory, replacing "subdir" with a ERR_PTR
> from the result of cachefiles_inject_write_error, which is either 0
> or some error code. This causes an oops when the resulting pointer
> is passed to vfs_mkdir.
Thanks for fixing that - now that I look at my code again it is
obviously wrong :-(
Reviewed-by: NeilBrown <neilb@xxxxxx>
Thanks,
NeilBrown
>
> Use a similar pattern to what is used earlier in the function; replace
> subdir with either the return value from vfs_mkdir, or the ERR_PTR
> of the cachefiles_inject_write_error() return value, but only if it
> is non zero.
>
> Fixes: c54b386969a5 ("VFS: Change vfs_mkdir() to return the dentry.")
> cc: netfs@xxxxxxxxxxxxxxx
> Signed-off-by: Marc Dionne <marc.dionne@xxxxxxxxxxxx>
> ---
> fs/cachefiles/namei.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
> index 83a60126de0f..14d0cc894000 100644
> --- a/fs/cachefiles/namei.c
> +++ b/fs/cachefiles/namei.c
> @@ -128,10 +128,11 @@ struct dentry *cachefiles_get_directory(struct cachefiles_cache *cache,
> ret = security_path_mkdir(&path, subdir, 0700);
> if (ret < 0)
> goto mkdir_error;
> - subdir = ERR_PTR(cachefiles_inject_write_error());
> - if (!IS_ERR(subdir))
> + ret = cachefiles_inject_write_error();
> + if (ret == 0)
> subdir = vfs_mkdir(&nop_mnt_idmap, d_inode(dir), subdir, 0700);
> - ret = PTR_ERR(subdir);
> + else
> + subdir = ERR_PTR(ret);
> if (IS_ERR(subdir)) {
> trace_cachefiles_vfs_error(NULL, d_inode(dir), ret,
> cachefiles_trace_mkdir_error);
> --
> 2.48.1
>
>
