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

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

[Nirmoy Das]

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: syzbot+a16fb0cce329a320661c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a16fb0cce329a320661c
Cc: stable@vger.kernel.org
Signed-off-by: Nirmoy Das <nirmoyd@nvidia.com>
---
 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);
 
 		od->cache = cache;
 		ovl_seek_cursor(od, ctx->pos);
-- 
2.43.0

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

[Amir Goldstein]

Hi Nirmoy!

Nice catch!

On Thu, May 14, 2026 at 1:14 PM Nirmoy Das <nirmoyd@nvidia.com> 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: syzbot+a16fb0cce329a320661c@syzkaller.appspotmail.com
> 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: stable@vger.kernel.org
> Signed-off-by: Nirmoy Das <nirmoyd@nvidia.com>
> ---
>  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.

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

[Nirmoy Das]

Hi Amir,

On 14.05.26 15:36, Amir Goldstein wrote:
> Hi Nirmoy!
>
> Nice catch!
>
> On Thu, May 14, 2026 at 1:14 PM Nirmoy Das <nirmoyd@nvidia.com> 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: syzbot+a16fb0cce329a320661c@syzkaller.appspotmail.com
>> 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?
I think the previous patch was masking this.

KASAN says "maybe wild-memory-access in range". If I'm not wrong, we 
would see "use-after-free" or "slab-use-after-free" for a real UAF.

Reproducing on aarch64 virtme-ng + KASAN with the unpatched kernel I get:

Unable to handle kernel paging request at virtual address ffffffffc1c02d50
KASAN: maybe wild-memory-access in range
[0x0003fffe0e016a80-0x0003fffe0e016a87]
[ffffffffc1c02d50] pgd=0..., p4d=..., pud=..., pmd=0000000000000000

The patch fixes the issue. Without the fix the reproducer hits the 
crashes around ~1500 iteration. With the fix applied it runs > 5000 
iterations with no error.

>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Nirmoy Das <nirmoyd@nvidia.com>
>> ---
>>   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;
>   }

I will send a v2 with this suggestion.

Regards,

Nirmoy

> Thanks,
> Amir.

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

[Nirmoy Das]

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: syzbot+a16fb0cce329a320661c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a16fb0cce329a320661c
Cc: stable@vger.kernel.org
Signed-off-by: Nirmoy Das <nirmoyd@nvidia.com>
---
v2:
 - Drop the now-redundant 'int err = 0' initializer and the trailing
   'return err' in ovl_iterate_merged(); err is only used inside the
   loop's update-check, so the function can just return 0 on success.
   (Amir Goldstein)
 - Link to v1:
   https://lore.kernel.org/all/20260514111354.3552538-1-nirmoyd@nvidia.com/

 fs/overlayfs/readdir.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 1dcc75b3a90f9..e7fe29cb6028b 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -838,15 +838,14 @@ 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;
 
 		cache = ovl_cache_get(dentry);
-		err = PTR_ERR(cache);
 		if (IS_ERR(cache))
-			return err;
+			return PTR_ERR(cache);
 
 		od->cache = cache;
 		ovl_seek_cursor(od, ctx->pos);
@@ -869,7 +868,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;
 }
 
 static bool ovl_need_adjust_d_ino(struct file *file)
-- 
2.43.0

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

[Amir Goldstein]

On Thu, May 14, 2026 at 4:43 PM Nirmoy Das <nirmoyd@nvidia.com> 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: syzbot+a16fb0cce329a320661c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=a16fb0cce329a320661c
> Cc: stable@vger.kernel.org
> Signed-off-by: Nirmoy Das <nirmoyd@nvidia.com>
> ---
> v2:
>  - Drop the now-redundant 'int err = 0' initializer and the trailing
>    'return err' in ovl_iterate_merged(); err is only used inside the
>    loop's update-check, so the function can just return 0 on success.
>    (Amir Goldstein)
>  - Link to v1:
>    https://lore.kernel.org/all/20260514111354.3552538-1-nirmoyd@nvidia.com/
>

I queue this up and will work on fortifying patches.

Thanks for the thorough investigation and for nailing this!
Amir.

>  fs/overlayfs/readdir.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index 1dcc75b3a90f9..e7fe29cb6028b 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -838,15 +838,14 @@ 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;
>
>                 cache = ovl_cache_get(dentry);
> -               err = PTR_ERR(cache);
>                 if (IS_ERR(cache))
> -                       return err;
> +                       return PTR_ERR(cache);
>
>                 od->cache = cache;
>                 ovl_seek_cursor(od, ctx->pos);
> @@ -869,7 +868,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;
>  }
>
>  static bool ovl_need_adjust_d_ino(struct file *file)
> --
> 2.43.0
>