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

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 <[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?
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: [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;
>   }

I will send a v2 with this suggestion.

Regards,

Nirmoy

> Thanks,
> Amir.