OpenVZ / Virtuozzo kernel development (devel@openvz.org)
 help / color / mirror / Atom feed
From: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
To: Konstantin Khorenko <khorenko@virtuozzo.com>,
	Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
Cc: devel@openvz.org
Subject: Re: [Devel] [PATCH hci-8.1 2/2] ovl: Keep creator VE in overlayfs superblock
Date: Mon, 19 Oct 2026 11:00:30 +0200	[thread overview]
Message-ID: <244f9a36-6ce0-4002-947b-0c772b80d184@virtuozzo.com> (raw)
In-Reply-To: <179035911162.3337040.7476455317600827184.b4-review@b4>



On 9/25/26 19:58, Konstantin Khorenko wrote:
>> We observed a crash in overlayfs when checkpointing a container
>> with docker running inside:
>>
>> [ 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: ...
>> [ 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
>>
>> The root cause is VE namespace mismatch. Overlayfs was created
>> inside CT, so ovl_fill_super stored in-container credentials
>> to ofs->creator_cred.
>> Those creds are later used in ovl_override_creds to temporarily
>> swich creds and perform checks using them.
>>
>> One of these checks happened in ovl_get_origin_fh/ovl_can_decode_fh.
>> On fs creation this check passed because we had container VE
>> and container creds, so overlayfs thought that we support
>> file handles (ovl_can_decode_fh call in ovl_make_workdir).
>>
>> When we tried to do something with the overlayfs from host VE,
>> ve_capable check was performed with credentials from CT
>> and with host VE.
>> ovl_get_origin_fh returned unexpected NULL and all exploded.
>>
>> Store creator VE next to creator creds, so we can perform all
>> ve_capable checks with the correct VE.
>>
>> https://virtuozzo.atlassian.net/browse/VSTOR-142405
>>
>> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
>>
>> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
>> index 4388004a319d0..b7c06ae4beb4c 100644
>> --- a/fs/overlayfs/copy_up.c
>> +++ b/fs/overlayfs/copy_up.c
>> @@ -478,7 +478,7 @@ struct ovl_fh *ovl_get_origin_fh(struct ovl_fs *ofs, struct dentry *origin)
>>  	 * so we can use the overlay.origin xattr to distignuish between a copy
>>  	 * up and a pure upper inode.
>>  	 */
>> -	if (!ovl_can_decode_fh(origin->d_sb))
>> +	if (!ovl_can_decode_fh(origin->d_sb, ofs))
>>  		return NULL;
>>  
>>  	return ovl_encode_real_fh(ofs, d_inode(origin), false);
>> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
>> index efa2a072aa693..e33debf8b1ed8 100644
>> --- a/fs/overlayfs/namei.c
>> +++ b/fs/overlayfs/namei.c
>> @@ -166,7 +166,7 @@ struct dentry *ovl_decode_real_fh(struct ovl_fs *ofs, struct ovl_fh *fh,
>>  	struct dentry *real;
>>  	int bytes;
>>  
>> -	if (!ve_capable(CAP_DAC_READ_SEARCH))
>> +	if (!__ve_capable(ofs->creator_ve, CAP_DAC_READ_SEARCH))
>>  		return NULL;
>>  
>>  	/*
>> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
>> index 8577c7884fb92..9c8dd2f9cac62 100644
>> --- a/fs/overlayfs/overlayfs.h
>> +++ b/fs/overlayfs/overlayfs.h
>> @@ -427,7 +427,7 @@ static inline const struct cred *ovl_creds(struct super_block *sb)
>>  	return OVL_FS(sb)->creator_cred;
>>  }
>>  
>> -int ovl_can_decode_fh(struct super_block *sb);
>> +int ovl_can_decode_fh(struct super_block *sb, struct ovl_fs *ofs);
>>  struct dentry *ovl_indexdir(struct super_block *sb);
>>  bool ovl_index_all(struct super_block *sb);
>>  bool ovl_verify_lower(struct super_block *sb);
>> diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
>> index ab78001635e7c..a6ad8b8de48c0 100644
>> --- a/fs/overlayfs/ovl_entry.h
>> +++ b/fs/overlayfs/ovl_entry.h
>> @@ -77,6 +77,7 @@ struct ovl_fs {
>>  	struct ovl_config config;
>>  	/* creds of process who forced instantiation of super block */
>>  	const struct cred *creator_cred;
>> +	struct ve_struct *creator_ve;
>>  	bool tmpfile;
>>  	bool noxattr;
>>  	bool nofh;
>> diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
>> index 32c58fcfe52f0..c047f1265b8ef 100644
>> --- a/fs/overlayfs/params.c
>> +++ b/fs/overlayfs/params.c
>> @@ -8,6 +8,7 @@
>>  #include <linux/posix_acl_xattr.h>
>>  #include <linux/seq_file.h>
>>  #include <linux/xattr.h>
>> +#include <linux/ve.h>
>>  #include "overlayfs.h"
>>  #include "params.h"
>>  
>> @@ -761,6 +762,8 @@ void ovl_free_fs(struct ovl_fs *ofs)
>>  	kfree(ofs->config.workdir);
>>  	if (ofs->creator_cred)
>>  		put_cred(ofs->creator_cred);
>> +	if (ofs->creator_ve)
>> +		put_ve(ofs->creator_ve);
>>  	kfree(ofs);
>>  }
>>  
>> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
>> index fb47711c8fb9a..e87b6684ec70f 100644
>> --- a/fs/overlayfs/super.c
>> +++ b/fs/overlayfs/super.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/file.h>
>>  #include <linux/fs_context.h>
>>  #include <linux/fs_parser.h>
>> +#include <linux/ve.h>
>>  #include "overlayfs.h"
>>  #include "params.h"
>>  
>> @@ -393,7 +394,7 @@ static int ovl_lower_dir(const char *name, struct path *path,
>>  	 * The inodes index feature and NFS export need to encode and decode
>>  	 * file handles, so they require that all layers support them.
>>  	 */
>> -	fh_type = ovl_can_decode_fh(path->dentry->d_sb);
>> +	fh_type = ovl_can_decode_fh(path->dentry->d_sb, ofs);
>>  	if ((ofs->config.nfs_export ||
>>  	     (ofs->config.index && ofs->config.upperdir)) && !fh_type) {
>>  		ofs->config.index = false;
>> @@ -757,7 +758,7 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs,
>>  	}
>>  
>>  	/* Check if upper/work fs supports file handles */
>> -	fh_type = ovl_can_decode_fh(ofs->workdir->d_sb);
>> +	fh_type = ovl_can_decode_fh(ofs->workdir->d_sb, ofs);
>>  	if (ofs->config.index && !fh_type) {
>>  		ofs->config.index = false;
>>  		pr_warn("upper fs does not support file handles, falling back to index=off.\n");
>> @@ -1328,6 +1329,8 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
>>  
>>  	sb->s_d_op = &ovl_dentry_operations;
>>  
>> +	ofs->creator_ve = get_ve(get_exec_env());
>> +
> 
>   get_ve() pins the ve_struct, but does not keep the VE running: on CT
>   stop ve_drop_context() sets ve->init_cred = NULL, so __ve_capable()
>   falls back to ve0.init_cred, and ns_capable(&init_user_ns, cap) with
>   the in-CT creator creds is false. The check flips after mount again,
>   permanently this time, and the same NULL deref in ovl_create_index()
>   is reachable whenever the sb outlives the VE (host-side bind mount, a
>   held fd, NFS export).
> 
>   After this series both inputs of the check are fixed at mount time
>   (creator_cred, creator_ve), so its result is a mount-time constant.
>   Why not compute ve_capable(CAP_DAC_READ_SEARCH) once in
>   ovl_fill_super() and store a bool in ovl_fs, next to the existing
>   nofh? That closes the stopped-VE window and makes patch 1, creator_ve
>   and the get_ve/put_ve pairing unnecessary.
> 
>   Draft patch instead of both in thise series:

Sounds good.

AI suggests also:

diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
index 444aeeccb6daf..5a6160f8c18ed 100644
--- a/fs/overlayfs/export.c
+++ b/fs/overlayfs/export.c
@@ -802,6 +802,14 @@ static struct dentry *ovl_fh_to_dentry(struct super_block *sb, 
struct fid *fid,
        unsigned int flags = 0;
        int err;
 
+       /*
+        * Unlike lookup, this runs with the caller's creds and not under
+        * creator_cred, so ofs->fh_capable is not enough: the caller has to
+        * be allowed to decode file handles too.
+        */
+       if (!ve_capable(CAP_DAC_READ_SEARCH))
+               return ERR_PTR(-ESTALE);
+
        fh = ovl_fid_to_fh(fid, len, fh_type);
        err = PTR_ERR(fh);
        if (IS_ERR(fh))

This path is not under overlay cred override so it should not fallback to fh_capable.

AI comment:

We didn't need a separate check, because the ve_capable() inside ovl_decode_real_fh() already covered ovl_fh_to_dentry(). Every export decode goes through it:

ovl_fh_to_dentry()
  ovl_upper_fh_to_d() -> ovl_decode_real_fh()   /* NULL -> NULL dentry -> ESTALE */
  ovl_lower_fh_to_d() -> ovl_check_origin_fh()  /* error -> returned at once */
                           -> ovl_decode_real_fh()

That one check meant two different things depending on how it was reached:
- Lookup and index paths (ovl_lookup(), ovl_verify_index(), ovl_index_upper()) run under ovl_override_creds(). There it checks the mounter, which is what we now freeze in fh_capable.
- Export path (ovl_fh_to_dentry()) never overrides credentials. There the same line checks the caller of open_by_handle_at() or nfsd.

> 
>   diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
>   --- a/fs/overlayfs/ovl_entry.h
>   +++ b/fs/overlayfs/ovl_entry.h
>   @@ -77,6 +77,8 @@ struct ovl_fs {
>         struct ovl_config config;
>         /* creds of process who forced instantiation of super block */
>         const struct cred *creator_cred;
>   +     /* ve_capable(CAP_DAC_READ_SEARCH) frozen at mount time */
>   +     bool fh_capable;
>         bool tmpfile;
>         bool noxattr;
>         bool nofh;
>   diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
>   --- a/fs/overlayfs/super.c
>   +++ b/fs/overlayfs/super.c
>   @@ -393,7 +393,7 @@ static int ovl_lower_dir(const char *name, struct path *path,
>          * The inodes index feature and NFS export need to encode and decode
>          * file handles, so they require that all layers support them.
>          */
>   -     fh_type = ovl_can_decode_fh(path->dentry->d_sb);
>   +     fh_type = ovl_can_decode_fh(path->dentry->d_sb, ofs);
>         if ((ofs->config.nfs_export ||
>              (ofs->config.index && ofs->config.upperdir)) && !fh_type) {
>                 ofs->config.index = false;
>   @@ -757,7 +757,7 @@ static int ovl_make_workdir(struct super_block *sb, struct ovl_fs *ofs,
>         }
> 
>         /* Check if upper/work fs supports file handles */
>   -     fh_type = ovl_can_decode_fh(ofs->workdir->d_sb);
>   +     fh_type = ovl_can_decode_fh(ofs->workdir->d_sb, ofs);
>         if (ofs->config.index && !fh_type) {
>                 ofs->config.index = false;
>                 pr_warn("upper fs does not support file handles, falling back to index=off.\n");
>   @@ -1328,6 +1328,8 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
> 
>         sb->s_d_op = &ovl_dentry_operations;
> 
>   +     ofs->fh_capable = ve_capable(CAP_DAC_READ_SEARCH);
>   +
>         err = -ENOMEM;
>         ofs->creator_cred = cred = prepare_creds();
>         if (!cred)
>   diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
>   --- a/fs/overlayfs/util.c
>   +++ b/fs/overlayfs/util.c
>   @@ -78,9 +78,9 @@ const struct cred *ovl_override_creds(struct super_block *sb)
>     * Return 1 (FILEID_INO32_GEN) if fs uses the default 32bit inode encoding.
>     * Return -1 if fs uses a non default encoding with unknown inode size.
>     */
>   -int ovl_can_decode_fh(struct super_block *sb)
>   +int ovl_can_decode_fh(struct super_block *sb, struct ovl_fs *ofs)
>    {
>   -     if (!ve_capable(CAP_DAC_READ_SEARCH))
>   +     if (!ofs->fh_capable)
>                 return 0;
> 
>         if (!exportfs_can_decode_fh(sb->s_export_op))
>   diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
>   --- a/fs/overlayfs/namei.c
>   +++ b/fs/overlayfs/namei.c
>   @@ -166,7 +166,7 @@ struct dentry *ovl_decode_real_fh(struct ovl_fs *ofs, struct ovl_fh *fh,
>         struct dentry *real;
>         int bytes;
> 
>   -     if (!ve_capable(CAP_DAC_READ_SEARCH))
>   +     if (!ofs->fh_capable)
>                 return NULL;
> 
>         /*
>   diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
>   --- a/fs/overlayfs/copy_up.c
>   +++ b/fs/overlayfs/copy_up.c
>   @@ -478,7 +478,7 @@ struct ovl_fh *ovl_get_origin_fh(struct ovl_fs *ofs, struct dentry *origin)
>          * so we can use the overlay.origin xattr to distignuish between a copy
>          * up and a pure upper inode.
>          */
>   -     if (!ovl_can_decode_fh(origin->d_sb))
>   +     if (!ovl_can_decode_fh(origin->d_sb, ofs))
>                 return NULL;
> 
>         return ovl_encode_real_fh(ofs, d_inode(origin), false);
>   diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
>   --- a/fs/overlayfs/overlayfs.h
>   +++ b/fs/overlayfs/overlayfs.h
>   @@ -427,7 +427,7 @@ static inline const struct cred *ovl_creds(struct super_block *sb)
>         return OVL_FS(sb)->creator_cred;
>    }
> 
>   -int ovl_can_decode_fh(struct super_block *sb);
>   +int ovl_can_decode_fh(struct super_block *sb, struct ovl_fs *ofs);
>    struct dentry *ovl_indexdir(struct super_block *sb);
>    bool ovl_index_all(struct super_block *sb);
>    bool ovl_verify_lower(struct super_block *sb);
> 

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

      reply	other threads:[~2026-10-19  9:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-20 17:45 [Devel] [PATCH hci-8.1 0/2] Fix kernel crash in overlayfs Vladimir Riabchun
2026-09-20 17:45 ` [Devel] [PATCH hci-8.1 1/2] capability: Add new helper to check specific VE capabilities Vladimir Riabchun
2026-09-20 17:45 ` [Devel] [PATCH hci-8.1 2/2] ovl: Keep creator VE in overlayfs superblock Vladimir Riabchun
2026-09-25 17:58   ` Konstantin Khorenko
2026-10-19  9:00     ` Pavel Tikhomirov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=244f9a36-6ce0-4002-947b-0c772b80d184@virtuozzo.com \
    --to=ptikhomirov@virtuozzo.com \
    --cc=devel@openvz.org \
    --cc=khorenko@virtuozzo.com \
    --cc=vladimir.riabchun@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox