Re: [PATCH] ovl: keep err zero after successful ovl_cache_get()

Hi Nirmoy!

Nice catch!

On Thu, May 14, 2026 at 1:14 PM Nirmoy Das <[email protected]> wrote:
>
> ovl_iterate_merged() stores PTR_ERR(cache) in err before checking
> IS_ERR(cache). On success err holds the truncated cache pointer and
> can be returned as a bogus non-zero error.
>
> The syzbot reproducer reaches this through overlay-on-overlay readdir:
>
>   getdents64
>     iterate_dir(outer overlay file)
>       ovl_iterate_merged()
>         ovl_cache_get()
>           ovl_dir_read_merged()
>             ovl_dir_read()
>               iterate_dir(inner overlay file)
>                 ovl_iterate_merged()
>
> Only compute PTR_ERR(cache) on the error path.
>
> Fixes: d25e4b739f83 ("ovl: refactor ovl_iterate() and port to cred guard")
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=a16fb0cce329a320661c

Does this fix really close the bug?
The report is a UAF, which was fixed by the other patch.
Right?

> Cc: [email protected]
> Signed-off-by: Nirmoy Das <[email protected]>
> ---
>  fs/overlayfs/readdir.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index 1dcc75b3a90f9..0d471064cfea1 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -844,9 +844,8 @@ static int ovl_iterate_merged(struct file *file, struct dir_context *ctx)
>                 struct ovl_dir_cache *cache;
>
>                 cache = ovl_cache_get(dentry);
> -               err = PTR_ERR(cache);
>                 if (IS_ERR(cache))
> -                       return err;
> +                       return PTR_ERR(cache);
>

This is good but also no point for returning err at end on function at all:

--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -838,7 +838,7 @@ static int ovl_iterate_merged(struct file *file,
struct dir_context *ctx)
        struct ovl_dir_file *od = file->private_data;
        struct dentry *dentry = file->f_path.dentry;
        struct ovl_cache_entry *p;
-       int err = 0;
+       int err;

        if (!od->cache) {
                struct ovl_dir_cache *cache;
@@ -869,7 +869,7 @@ static int ovl_iterate_merged(struct file *file,
struct dir_context *ctx)
                od->cursor = p->l_node.next;
                ctx->pos++;
        }
-       return err;
+       return 0;
 }

Thanks,
Amir.