Re: [PATCH] ovl: Fix NULL dereference in ovl_get_index_name_fh

On Mon, Aug 24, 2026 at 11:22 AM Vladimir Riabchun
<[email protected]> wrote:
>
>
>
> On 8/24/26 11:10, Vladimir Riabchun wrote:
> > We observed several crashes in overlayfs with the following trace:
> > [ 2300.870181] BUG: kernel NULL pointer dereference, address: 0000000000000005
> > [ 2300.871537] #PF: supervisor read access in kernel mode
> > [ 2300.872577] #PF: error_code(0x0000) - not-present page
> > [ 2300.873595] PGD 800000010a18f067 P4D 800000010a18f067 PUD 10730c067 PMD 0
> > [ 2300.874900] Oops: Oops: 0000 [#1] SMP PTI
> > [ 2300.875742] CPU: 1 UID: 0 PID: 5419 Comm: criu ve: 0 Kdump: loaded Not tainted 6.12.0-211.39.1.16.6.vz10.x86_64 #1 16.6 PREEMPT(voluntary)
> > [ 2300.878057] Hardware name: Acronis OpenStack Compute/Virtuozzo, BIOS 1.16.1-1.vz9.2 04/01/2014
> > [ 2300.879684] RIP: 0010:ovl_get_index_name_fh+0x18/0x70 [overlay]
> > [ 2300.880859] Code: 66 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f 00 0f 1f 44 00 00 41 54 49 89 f4 be c0 0d 00 00 55 53 48 89 fb <0f> b6 7f 05 48 01 ff e8 bc 42 2f ea 48 85 c0 74 33 0f b6 53 05 48
> > [ 2300.884128] RSP: 0018:ffffd32582e0f7c0 EFLAGS: 00010246
> > [ 2300.885153] RAX: 0000000000000010 RBX: 0000000000000000 RCX: ffff8ba4497480a8
> > [ 2300.886480] RDX: ffff8ba45535dc00 RSI: 0000000000000dc0 RDI: 0000000000000000
> > [ 2300.887824] RBP: ffff8ba68f9daa80 R08: ffff8ba4497480a8 R09: 000000000000030e
> > [ 2300.889139] R10: 0000000000000000 R11: ffff8ba44862d6d0 R12: ffffd32582e0f800
> > [ 2300.890498] R13: 0000000000000000 R14: ffff8ba497cb0110 R15: ffff8ba517548cc0
> > [ 2300.891838] FS:  00007f5a2f492880(0000) GS:ffff8bab9fc80000(0000) knlGS:0000000000000000
> > [ 2300.893376] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 2300.894476] CR2: 0000000000000005 CR3: 000000033403a005 CR4: 00000000003726f0
> > [ 2300.895807] Call Trace:
> > [ 2300.896397]  <TASK>
> > [ 2300.896938]  ? show_trace_log_lvl+0x1b0/0x2f0
> > [ 2300.897854]  ? show_trace_log_lvl+0x1b0/0x2f0
> > [ 2300.898748]  ? ovl_create_index+0x82/0x260 [overlay]
> > [ 2300.899766]  ? __die_body.cold+0x8/0x12
> > [ 2300.900582]  ? page_fault_oops+0x148/0x160
> > [ 2300.901562]  ? exc_page_fault+0x7f/0x150
> > [ 2300.902406]  ? asm_exc_page_fault+0x26/0x30
> > [ 2300.903298]  ? ovl_get_index_name_fh+0x18/0x70 [overlay]
> > [ 2300.904435]  ovl_create_index+0x82/0x260 [overlay]
> > [ 2300.905406]  ovl_copy_up_workdir+0x30a/0x330 [overlay]
> > [ 2300.906427]  ovl_do_copy_up+0x10d/0x2e0 [overlay]
> > [ 2300.907387]  ovl_copy_up_one+0x456/0x720 [overlay]
> > [ 2300.908376]  ? xa_load+0x7b/0xb0
> > [ 2300.909090]  ? _raw_spin_unlock+0xe/0x30
> > [ 2300.909913]  ? list_lru_add+0x11b/0x160
> > [ 2300.910736]  ovl_copy_up_flags+0x81/0x110 [overlay]
> > [ 2300.911740]  ovl_create_object+0x51/0x130 [overlay]
> > [ 2300.912719]  vfs_mkdir+0x194/0x250
> > [ 2300.913470]  do_mkdirat+0x14c/0x180
> > [ 2300.914223]  __x64_sys_mkdir+0x46/0x70
> > [ 2300.915019]  do_syscall_64+0x7d/0x160
> > [ 2300.915798]  ? do_sys_openat2+0x9c/0xe0
> > [ 2300.916672]  ? syscall_exit_work+0xf3/0x120
> > [ 2300.917540]  ? syscall_exit_to_user_mode+0x32/0x190
> > [ 2300.918518]  ? do_syscall_64+0x89/0x160
> > [ 2300.919316]  ? syscall_exit_work+0xf3/0x120
> > [ 2300.920181]  ? syscall_exit_to_user_mode+0x32/0x190
> > [ 2300.921173]  ? do_syscall_64+0x89/0x160
> > [ 2300.921991]  ? exc_page_fault+0x7f/0x150
> > [ 2300.922814]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> > [ 2300.923823] RIP: 0033:0x7f5a2f8de3eb
>
> Seems to be the same case as in
> https://lore.kernel.org/all/[email protected]/#t
> Please, take a look at it as well.

Thanks for finding this partial dup.
Miklos' comments partly apply to the fixes in fs/overlayfs/copy_up.c
Those seem to be moot when passing 0 size to ovl_*setxattr().

> >
> > It seems that after commit 5b02bfc1e7e3 ("ovl: do not encode lower fh
> > with upper sb_writers held") fh may be NULL and it is dereferenced
> > without check in ovl_get_index_name_fh.
> > In our case NULL value came from !capable(CAP_DAC_READ_SEARCH)
> > in ovl_can_decode_fh.

The index feature should not be enabled if the mounter does not pass
the ovl_can_decode_fh() permission check in ovl_make_workdir() and
ovl_copy_up_one() should be called with mounter creds, so what is
really happening in your splat?
Why did !capable(CAP_DAC_READ_SEARCH) fail?
Do we have a bug upstream? a bug in 6.12 LTS or a bug in vz kernel?
Please explain.

> >
> > Fix this by checking fh before using its fields,
> > if fh is NULL - use buf = NULL and len = 0.
> > For ovl_get_index_name_fh use empty string with length 1 so
> > the following bin2hex and kfree(name.name) are fine.

That's no good.
The index has to be a valid fh name
An empty index name makes no sense that's why those helpers
are gated with the index feature which pre-checks the can_decode
condition on mount.

Thanks,
Amir.

> >
> > Fixes: 5b02bfc1e7e3 ("ovl: do not encode lower fh with upper sb_writers held")
> > Signed-off-by: Vladimir Riabchun <[email protected]>
> > ---
> > There is one more place where I can see the possibility of this problem:
> > ovl_get_indexdir calls ovl_get_origin_fh which can return NULL, fh is
> > then passed to ovl_verify_origin_fh -> ovl_verify_set_fh -> ovl_verify_fh,
> > which doesn't check fh for NULL and dereferences it.
> >
> > I'm not very familiar with the code, so I'd like to hear what should be
> > returned there in !fh case (my guess - return 0 if ofh is NULL as well
> > and -SOME_ERROR otherwise).
> >
> >   fs/overlayfs/copy_up.c | 5 +++--
> >   fs/overlayfs/namei.c   | 6 ++++--
> >   2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> > index 4388004a319d..63a783473b60 100644
> > --- a/fs/overlayfs/copy_up.c
> > +++ b/fs/overlayfs/copy_up.c
> > @@ -492,7 +492,7 @@ int ovl_set_origin_fh(struct ovl_fs *ofs, const struct ovl_fh *fh,
> >       /*
> >        * Do not fail when upper doesn't support xattrs.
> >        */
> > -     err = ovl_check_setxattr(ofs, upper, OVL_XATTR_ORIGIN, fh->buf,
> > +     err = ovl_check_setxattr(ofs, upper, OVL_XATTR_ORIGIN, fh ? fh->buf : NULL,
> >                                fh ? fh->fb.len : 0, 0);
> >
> >       /* Ignore -EPERM from setting "user.*" on symlink/special */
> > @@ -510,7 +510,8 @@ static int ovl_set_upper_fh(struct ovl_fs *ofs, struct dentry *upper,
> >       if (IS_ERR(fh))
> >               return PTR_ERR(fh);
> >
> > -     err = ovl_setxattr(ofs, index, OVL_XATTR_UPPER, fh->buf, fh->fb.len);
> > +     err = ovl_setxattr(ofs, index, OVL_XATTR_UPPER,
> > +             fh ? fh->buf : NULL, fh ? fh->fb.len : 0);
> >
> >       kfree(fh);
> >       return err;
> > diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> > index efa2a072aa69..81607a24589f 100644
> > --- a/fs/overlayfs/namei.c
> > +++ b/fs/overlayfs/namei.c
> > @@ -721,12 +721,14 @@ int ovl_verify_index(struct ovl_fs *ofs, struct dentry *index)
> >   int ovl_get_index_name_fh(const struct ovl_fh *fh, struct qstr *name)
> >   {
> >       char *n, *s;
> > +     size_t len = fh ? fh->fb.len : 1;
> > +     const char *buf = fh ? fh->buf : (const u8 *)"";
> >
> > -     n = kcalloc(fh->fb.len, 2, GFP_KERNEL);
> > +     n = kcalloc(len, 2, GFP_KERNEL);
> >       if (!n)
> >               return -ENOMEM;
> >
> > -     s  = bin2hex(n, fh->buf, fh->fb.len);
> > +     s  = bin2hex(n, buf, len);
> >       *name = (struct qstr) QSTR_INIT(n, s - n);
> >
> >       return 0;
>
> --
> Best regards, Riabchun Vladimir
> Linux Kernel Developer, Virtuozzo
>