Linux Overlay Filesystem development
 help / color / mirror / Atom feed
* [PATCH] ovl: Fix NULL dereference in ovl_get_index_name_fh
@ 2026-08-24  9:10 Vladimir Riabchun
  2026-08-24  9:22 ` Vladimir Riabchun
  0 siblings, 1 reply; 4+ messages in thread
From: Vladimir Riabchun @ 2026-08-24  9:10 UTC (permalink / raw)
  To: miklos, amir73il, linux-unionfs; +Cc: vladimir.riabchun

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

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.

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.

Fixes: 5b02bfc1e7e3 ("ovl: do not encode lower fh with upper sb_writers held")
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
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;
-- 
2.47.1


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

* Re: [PATCH] ovl: Fix NULL dereference in ovl_get_index_name_fh
  2026-08-24  9:10 [PATCH] ovl: Fix NULL dereference in ovl_get_index_name_fh Vladimir Riabchun
@ 2026-08-24  9:22 ` Vladimir Riabchun
  2026-08-24 10:24   ` Amir Goldstein
  0 siblings, 1 reply; 4+ messages in thread
From: Vladimir Riabchun @ 2026-08-24  9:22 UTC (permalink / raw)
  To: miklos, amir73il, linux-unionfs; +Cc: wangzhaolong1



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/20250429001308.370040-1-wangzhaolong1@huawei.com/#t
Please, take a look at it as well.
> 
> 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.
> 
> 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.
> 
> Fixes: 5b02bfc1e7e3 ("ovl: do not encode lower fh with upper sb_writers held")
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
> 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


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

* Re: [PATCH] ovl: Fix NULL dereference in ovl_get_index_name_fh
  2026-08-24  9:22 ` Vladimir Riabchun
@ 2026-08-24 10:24   ` Amir Goldstein
  2026-09-01 16:38     ` Vladimir Riabchun
  0 siblings, 1 reply; 4+ messages in thread
From: Amir Goldstein @ 2026-08-24 10:24 UTC (permalink / raw)
  To: Vladimir Riabchun; +Cc: miklos, linux-unionfs, wangzhaolong1

On Mon, Aug 24, 2026 at 11:22 AM Vladimir Riabchun
<vladimir.riabchun@virtuozzo.com> 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/20250429001308.370040-1-wangzhaolong1@huawei.com/#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 <vladimir.riabchun@virtuozzo.com>
> > ---
> > 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
>

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

* Re: [PATCH] ovl: Fix NULL dereference in ovl_get_index_name_fh
  2026-08-24 10:24   ` Amir Goldstein
@ 2026-09-01 16:38     ` Vladimir Riabchun
  0 siblings, 0 replies; 4+ messages in thread
From: Vladimir Riabchun @ 2026-09-01 16:38 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: miklos, linux-unionfs, wangzhaolong1



On 8/24/26 12:24, Amir Goldstein wrote:
> On Mon, Aug 24, 2026 at 11:22 AM Vladimir Riabchun
> <vladimir.riabchun@virtuozzo.com> 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/20250429001308.370040-1-wangzhaolong1@huawei.com/#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.

After the investigation, it appears that the problem is in VZ kernel.
We changed capable -> ve_capable, which takes into account container
context and namespaces. This allowed the following situation:

- Mount created inside container. capable(CAP_DAC_READ_SEARCH) is 0, but
    ve_capable(CAP_DAC_READ_SEARCH) is 1. Container creds are stored in
    ofs->creator_cred. All the code now thinks that index feature is present.
- Trigger copy_up for directory for host. In the beginning of ovl_copy_up_one
    we have both capable and ve_capable = 1, but after switching creds to
    container ones both checks return 0 and sad things happen.

So it is completely VZ-specific and upstream can't get this situation using
capable check.

The only thing that could be done here is to add some sort of WARN check
to report ovl_can_decode_fh returning NULL while c->indexed.

Thanks for your time and help.

> 
>>>
>>> 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 <vladimir.riabchun@virtuozzo.com>
>>> ---
>>> 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
>>

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo


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

end of thread, other threads:[~2026-09-01 16:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-24  9:10 [PATCH] ovl: Fix NULL dereference in ovl_get_index_name_fh Vladimir Riabchun
2026-08-24  9:22 ` Vladimir Riabchun
2026-08-24 10:24   ` Amir Goldstein
2026-09-01 16:38     ` Vladimir Riabchun

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