Re: [PATCH v2] ovl: keep err zero after successful ovl_cache_get()
Amir Goldstein <[email protected]> Fri, 15 May 2026 21:49:37 +0200
| Newsgroups | org.kernel.vger.linux-unionfs,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <CAOQ4uxjEZzy3wOM9pm6BKB9v8862LjHbPT8SwU5TKx3wHsnwSw@mail.gmail.com> |
On Thu, May 14, 2026 at 4:43 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 > Cc: [email protected] > Signed-off-by: Nirmoy Das <[email protected]> > --- > 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/[email protected]/ > > 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.