Linux kernel -stable discussions
 help / color / mirror / Atom feed
* [PATCH] ovl: keep err zero after successful ovl_cache_get()
@ 2026-05-14 11:13 Nirmoy Das
  2026-05-14 12:36 ` Amir Goldstein
  2026-05-14 14:42 ` [PATCH v2] " Nirmoy Das
  0 siblings, 2 replies; 10+ messages in thread
From: Nirmoy Das @ 2026-05-14 11:13 UTC (permalink / raw)
  To: Miklos Szeredi, Amir Goldstein
  Cc: Christian Brauner, linux-unionfs, linux-kernel, Nirmoy Das,
	syzbot+a16fb0cce329a320661c, stable

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] ovl: keep err zero after successful ovl_cache_get()
  2026-05-14 11:13 [PATCH] ovl: keep err zero after successful ovl_cache_get() Nirmoy Das
@ 2026-05-14 12:36 ` Amir Goldstein
  2026-05-14 14:37   ` Nirmoy Das
  2026-05-14 14:42 ` [PATCH v2] " Nirmoy Das
  1 sibling, 1 reply; 10+ messages in thread
From: Amir Goldstein @ 2026-05-14 12:36 UTC (permalink / raw)
  To: Nirmoy Das
  Cc: Miklos Szeredi, Christian Brauner, linux-unionfs, linux-kernel,
	syzbot+a16fb0cce329a320661c, stable

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ovl: keep err zero after successful ovl_cache_get()
  2026-05-14 12:36 ` Amir Goldstein
@ 2026-05-14 14:37   ` Nirmoy Das
  0 siblings, 0 replies; 10+ messages in thread
From: Nirmoy Das @ 2026-05-14 14:37 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Miklos Szeredi, Christian Brauner, linux-unionfs, linux-kernel,
	syzbot+a16fb0cce329a320661c, stable

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] ovl: keep err zero after successful ovl_cache_get()
  2026-05-14 11:13 [PATCH] ovl: keep err zero after successful ovl_cache_get() Nirmoy Das
  2026-05-14 12:36 ` Amir Goldstein
@ 2026-05-14 14:42 ` Nirmoy Das
  2026-05-14 15:26   ` Amir Goldstein
  2026-05-15 19:49   ` Amir Goldstein
  1 sibling, 2 replies; 10+ messages in thread
From: Nirmoy Das @ 2026-05-14 14:42 UTC (permalink / raw)
  To: Miklos Szeredi, Amir Goldstein
  Cc: Christian Brauner, linux-unionfs, linux-kernel,
	syzbot+a16fb0cce329a320661c, stable, 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] ovl: keep err zero after successful ovl_cache_get()
  2026-05-14 14:42 ` [PATCH v2] " Nirmoy Das
@ 2026-05-14 15:26   ` Amir Goldstein
  2026-05-14 19:19     ` Amir Goldstein
  2026-05-15 19:49   ` Amir Goldstein
  1 sibling, 1 reply; 10+ messages in thread
From: Amir Goldstein @ 2026-05-14 15:26 UTC (permalink / raw)
  To: Nirmoy Das
  Cc: Miklos Szeredi, Christian Brauner, linux-unionfs, linux-kernel,
	syzbot+a16fb0cce329a320661c, stable

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
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] ovl: keep err zero after successful ovl_cache_get()
  2026-05-14 15:26   ` Amir Goldstein
@ 2026-05-14 19:19     ` Amir Goldstein
  2026-05-15 11:15       ` Nirmoy Das
  0 siblings, 1 reply; 10+ messages in thread
From: Amir Goldstein @ 2026-05-14 19:19 UTC (permalink / raw)
  To: Nirmoy Das
  Cc: Miklos Szeredi, Christian Brauner, linux-unionfs, linux-kernel,
	syzbot+a16fb0cce329a320661c, stable

On Thu, May 14, 2026 at 5:26 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> 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.

Nirmoy,

I pushed fortify patches to ovl-fixes on my github [1].

Can you verify that the assertions trigger if you revert your fix
and run the reproducer?

I imagine they would trigger much more frequently than the KASAN
warnings do.

Thanks,
Amir.

[1] https://github.com/amir73il/linux/commits/ovl-fixes/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] ovl: keep err zero after successful ovl_cache_get()
  2026-05-14 19:19     ` Amir Goldstein
@ 2026-05-15 11:15       ` Nirmoy Das
  2026-05-15 16:39         ` Amir Goldstein
  0 siblings, 1 reply; 10+ messages in thread
From: Nirmoy Das @ 2026-05-15 11:15 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Miklos Szeredi, Christian Brauner, linux-unionfs, linux-kernel,
	syzbot+a16fb0cce329a320661c, stable

Hi Amir,

On 14.05.26 22:19, Amir Goldstein wrote:
> On Thu, May 14, 2026 at 5:26 PM Amir Goldstein <amir73il@gmail.com> wrote:
>> 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.
> Nirmoy,
>
> I pushed fortify patches to ovl-fixes on my github [1].
>
> Can you verify that the assertions trigger if you revert your fix
> and run the reproducer?
>
> I imagine they would trigger much more frequently than the KASAN
> warnings do.


Yes, the assertion triggers with your ovl-fixes branch after reverting 
my fix.

9541f25af774 Revert "ovl: keep err zero after successful ovl_cache_get()"
1c067d912e47 ovl: add assertions in dir cache code
98e3a2d258e9 ovl: fix race between copy-up and open of a directory
4f80bb375112 ovl: keep err zero after successful ovl_cache_get()
18de6460b6bd ovl: opt-in for fortified ERR_PTR()
690bd87e1fef err_ptr.h: introduce ERR_PTR_SAFE()
7fd2df204f34 Linux 7.1-rc2

Running the syz reproducer with panic_on_warn=1 triggered:

[   55.404636] ------------[ cut here ]------------
[   55.404646] WARNING: fs/overlayfs/readdir.c:511 at 
ovl_iterate+0x4c0/0x5bc, CPU#2: syz-ovl-iterate/14575
[   55.406875] CPU: 2 UID: 0 PID: 14575 Comm: syz-ovl-iterate Not 
tainted 7.1.0-rc2-g9541f25af774 #1 PREEMPT
[   55.408328] pc : ovl_iterate+0x4c0/0x5bc
[   55.408632] lr : ovl_iterate+0x4b4/0x5bc
[   55.413504] x2 : 0000000000000000 x1 : 0000000000000000 x0 : 
ffffffffc152db40
[   55.414036] Call trace:
[   55.414209]  ovl_iterate+0x4c0/0x5bc (P)
[   55.414503]  wrap_directory_iterator+0x60/0x90
[   55.414809]  shared_ovl_iterate+0x18/0x24
[   55.415125]  iterate_dir+0x10c/0x3a4
[   55.415365]  __arm64_sys_getdents64+0xe0/0x1e4
[   55.417312] Kernel panic - not syncing: kernel: panic_on_warn set ...

Regards,

Nirmoy

>
> Thanks,
> Amir.
>
> [1] https://github.com/amir73il/linux/commits/ovl-fixes/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] ovl: keep err zero after successful ovl_cache_get()
  2026-05-15 11:15       ` Nirmoy Das
@ 2026-05-15 16:39         ` Amir Goldstein
  2026-05-15 19:17           ` Nirmoy Das
  0 siblings, 1 reply; 10+ messages in thread
From: Amir Goldstein @ 2026-05-15 16:39 UTC (permalink / raw)
  To: Nirmoy Das
  Cc: Miklos Szeredi, Christian Brauner, linux-unionfs, linux-kernel,
	syzbot+a16fb0cce329a320661c, stable

On Fri, May 15, 2026 at 1:16 PM Nirmoy Das <nirmoyd@nvidia.com> wrote:
>
> Hi Amir,
>
> On 14.05.26 22:19, Amir Goldstein wrote:
> > On Thu, May 14, 2026 at 5:26 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >> 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.
> > Nirmoy,
> >
> > I pushed fortify patches to ovl-fixes on my github [1].
> >
> > Can you verify that the assertions trigger if you revert your fix
> > and run the reproducer?
> >
> > I imagine they would trigger much more frequently than the KASAN
> > warnings do.
>
>
> Yes, the assertion triggers with your ovl-fixes branch after reverting
> my fix.
>
> 9541f25af774 Revert "ovl: keep err zero after successful ovl_cache_get()"
> 1c067d912e47 ovl: add assertions in dir cache code
> 98e3a2d258e9 ovl: fix race between copy-up and open of a directory
> 4f80bb375112 ovl: keep err zero after successful ovl_cache_get()
> 18de6460b6bd ovl: opt-in for fortified ERR_PTR()
> 690bd87e1fef err_ptr.h: introduce ERR_PTR_SAFE()
> 7fd2df204f34 Linux 7.1-rc2
>
> Running the syz reproducer with panic_on_warn=1 triggered:
>
> [   55.404636] ------------[ cut here ]------------
> [   55.404646] WARNING: fs/overlayfs/readdir.c:511 at
> ovl_iterate+0x4c0/0x5bc, CPU#2: syz-ovl-iterate/14575
> [   55.406875] CPU: 2 UID: 0 PID: 14575 Comm: syz-ovl-iterate Not
> tainted 7.1.0-rc2-g9541f25af774 #1 PREEMPT
> [   55.408328] pc : ovl_iterate+0x4c0/0x5bc
> [   55.408632] lr : ovl_iterate+0x4b4/0x5bc
> [   55.413504] x2 : 0000000000000000 x1 : 0000000000000000 x0 :
> ffffffffc152db40
> [   55.414036] Call trace:
> [   55.414209]  ovl_iterate+0x4c0/0x5bc (P)
> [   55.414503]  wrap_directory_iterator+0x60/0x90
> [   55.414809]  shared_ovl_iterate+0x18/0x24
> [   55.415125]  iterate_dir+0x10c/0x3a4
> [   55.415365]  __arm64_sys_getdents64+0xe0/0x1e4
> [   55.417312] Kernel panic - not syncing: kernel: panic_on_warn set ...
>

Thanks for testing!

Did it trigger faster than the KASAN warning?
I'd imagine that it would?

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] ovl: keep err zero after successful ovl_cache_get()
  2026-05-15 16:39         ` Amir Goldstein
@ 2026-05-15 19:17           ` Nirmoy Das
  0 siblings, 0 replies; 10+ messages in thread
From: Nirmoy Das @ 2026-05-15 19:17 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Miklos Szeredi, Christian Brauner, linux-unionfs, linux-kernel,
	syzbot+a16fb0cce329a320661c, stable

Hi Amir,

On 15.05.26 19:39, Amir Goldstein wrote:
> On Fri, May 15, 2026 at 1:16 PM Nirmoy Das <nirmoyd@nvidia.com> wrote:
>> Hi Amir,
>>
>> On 14.05.26 22:19, Amir Goldstein wrote:
>>> On Thu, May 14, 2026 at 5:26 PM Amir Goldstein <amir73il@gmail.com> wrote:
>>>> 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.
>>> Nirmoy,
>>>
>>> I pushed fortify patches to ovl-fixes on my github [1].
>>>
>>> Can you verify that the assertions trigger if you revert your fix
>>> and run the reproducer?
>>>
>>> I imagine they would trigger much more frequently than the KASAN
>>> warnings do.
>>
>> Yes, the assertion triggers with your ovl-fixes branch after reverting
>> my fix.
>>
>> 9541f25af774 Revert "ovl: keep err zero after successful ovl_cache_get()"
>> 1c067d912e47 ovl: add assertions in dir cache code
>> 98e3a2d258e9 ovl: fix race between copy-up and open of a directory
>> 4f80bb375112 ovl: keep err zero after successful ovl_cache_get()
>> 18de6460b6bd ovl: opt-in for fortified ERR_PTR()
>> 690bd87e1fef err_ptr.h: introduce ERR_PTR_SAFE()
>> 7fd2df204f34 Linux 7.1-rc2
>>
>> Running the syz reproducer with panic_on_warn=1 triggered:
>>
>> [   55.404636] ------------[ cut here ]------------
>> [   55.404646] WARNING: fs/overlayfs/readdir.c:511 at
>> ovl_iterate+0x4c0/0x5bc, CPU#2: syz-ovl-iterate/14575
>> [   55.406875] CPU: 2 UID: 0 PID: 14575 Comm: syz-ovl-iterate Not
>> tainted 7.1.0-rc2-g9541f25af774 #1 PREEMPT
>> [   55.408328] pc : ovl_iterate+0x4c0/0x5bc
>> [   55.408632] lr : ovl_iterate+0x4b4/0x5bc
>> [   55.413504] x2 : 0000000000000000 x1 : 0000000000000000 x0 :
>> ffffffffc152db40
>> [   55.414036] Call trace:
>> [   55.414209]  ovl_iterate+0x4c0/0x5bc (P)
>> [   55.414503]  wrap_directory_iterator+0x60/0x90
>> [   55.414809]  shared_ovl_iterate+0x18/0x24
>> [   55.415125]  iterate_dir+0x10c/0x3a4
>> [   55.415365]  __arm64_sys_getdents64+0xe0/0x1e4
>> [   55.417312] Kernel panic - not syncing: kernel: panic_on_warn set ...
>>
> Thanks for testing!
>
> Did it trigger faster than the KASAN warning?
> I'd imagine that it would?


I lost my setup. I think it was quicker but I don't conclusive data. I 
will update you next week.


Regards,

Nirmoy

>
> Thanks,
> Amir.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] ovl: keep err zero after successful ovl_cache_get()
  2026-05-14 14:42 ` [PATCH v2] " Nirmoy Das
  2026-05-14 15:26   ` Amir Goldstein
@ 2026-05-15 19:49   ` Amir Goldstein
  1 sibling, 0 replies; 10+ messages in thread
From: Amir Goldstein @ 2026-05-15 19:49 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Miklos Szeredi, linux-unionfs, linux-kernel,
	syzbot+a16fb0cce329a320661c, stable, Nirmoy Das

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/
>
>  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
>

Christian,

Would you mind picking this one via vfs-fixes?

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-05-15 19:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-14 11:13 [PATCH] ovl: keep err zero after successful ovl_cache_get() Nirmoy Das
2026-05-14 12:36 ` Amir Goldstein
2026-05-14 14:37   ` Nirmoy Das
2026-05-14 14:42 ` [PATCH v2] " Nirmoy Das
2026-05-14 15:26   ` Amir Goldstein
2026-05-14 19:19     ` Amir Goldstein
2026-05-15 11:15       ` Nirmoy Das
2026-05-15 16:39         ` Amir Goldstein
2026-05-15 19:17           ` Nirmoy Das
2026-05-15 19:49   ` Amir Goldstein

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox