[Devel] [PATCH hci-8.1 v4 0/5] Fix kernel crash in overlayfs

[Devel] [PATCH hci-8.1 v4 0/5] Fix kernel crash in overlayfs

[Vladimir Riabchun]

When testing CRIU with containers with docker images,
we found an easily reproducible kernel crash:

[ 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.895807] Call Trace:
[ 2300.896397]  <TASK>
[ 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.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.922814]  entry_SYSCALL_64_after_hwframe+0x76/0x7e

The cause is that overlayfs does some feature checks on superblock
creation using capable/ve_capable and it stores current credentials.
But ve_capable check result depends on both current creds and
current VE. Later, overlayfs switches credentials only, leading
to unexpected feature disappearance.

This was reworked completely after v2, the approach is different now.
1. Patch 1 removes separate STOPPED state - VE goes directly to DEAD.
2. Patch 2 adds helpers for overriding tasks VE.
3. Patch 3 adds VE owner to overlay superblock.
4. Patch 4 fixes the bug by switching VE together with creds in overlayfs.

v2: http://lore.virtuozzo.com/all/20260920-ovl-use-orig-ve-v1-0-b3e63f6ff80d@virtuozzo.com/
https://virtuozzo.atlassian.net/browse/VSTOR-143259 - non-CRIU repro
https://virtuozzo.atlassian.net/browse/VSTOR-142405

Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
Changes in v4:
- Patch 2: clean non-NULL overrided_ve in fork and report it,
  Report revert_ve(ERR_PTR) usages and reject them.
- Patch 5: changed ovl_override_creds signature so it returns
  error. Added checks for it in all places to avoid.
- Link to v3: http://lore.virtuozzo.com/20261103-ovl-use-orig-ve-v3-0-add042cdd087@virtuozzo.com

Changes in v3:
- Patch 2: added a separate field for ve_override to fix problems Pasha spotted.
- Patch 3: new one, delays init_cred destroy so dead CT can still use it.
- Link to v2: http://lore.virtuozzo.com/20261026-ovl-use-orig-ve-v2-0-eab58bfc29ca@virtuozzo.com

Changes in v2:
- Patch 1: updated description as Sashiko suggested..
- Patch 2: moved stubs outside CONFIG_X86 section,
  added task_lock protection in helpers. Verified via lockdep,
  no splats after vzctl tests, C/R and crash reproducer.
- Patch 4: fixed typo in comment.
- Link to v1: http://lore.virtuozzo.com/20261022-ovl-use-orig-ve-v1-0-da84c00c8065@virtuozzo.com

---
Vladimir Riabchun (5):
      ve: Remove STOPPED state
      ve: Introduce override_ve/revert_ve helpers
      ve: Defer ve context drop until ve is completely destroyed
      ovl: Keep creator VE in overlayfs superblock
      ve/overlayfs: Override VE together with creds

 fs/overlayfs/copy_up.c   |  6 ++--
 fs/overlayfs/dir.c       | 59 ++++++++++++++++++------------
 fs/overlayfs/file.c      | 65 +++++++++++++++++++--------------
 fs/overlayfs/inode.c     | 94 +++++++++++++++++++++++++++++++-----------------
 fs/overlayfs/namei.c     | 39 +++++++++++++-------
 fs/overlayfs/overlayfs.h |  9 ++++-
 fs/overlayfs/ovl_entry.h |  1 +
 fs/overlayfs/params.c    |  3 ++
 fs/overlayfs/readdir.c   | 36 ++++++++++++-------
 fs/overlayfs/super.c     |  3 ++
 fs/overlayfs/util.c      | 41 ++++++++++++++++-----
 fs/overlayfs/xattrs.c    | 35 +++++++++++-------
 include/linux/sched.h    |  1 +
 include/linux/ve.h       | 30 ++++++++++++++--
 kernel/capability.c      |  6 ++--
 kernel/fork.c            |  2 ++
 kernel/ve/ve.c           | 10 +++---
 17 files changed, 300 insertions(+), 140 deletions(-)
---
base-commit: fd34de0d39ec34baee8af703df9c34bc0851e2d3
change-id: 20260918-ovl-use-orig-ve-2dc365976202

Best regards,
--  
Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

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

[Devel] [PATCH hci-8.1 v4 1/5] ve: Remove STOPPED state

[Vladimir Riabchun]

We don't need extra STOPPED state. It makes us feel that CT
could be started again from it, but it is actually not true.

Separate STOPPED stated doesn't bring any value. Drop it and
transition VE to DEAD state directly on the stop.

https://virtuozzo.atlassian.net/browse/VSTOR-142405
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 include/linux/ve.h | 3 +--
 kernel/ve/ve.c     | 6 ++----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/include/linux/ve.h b/include/linux/ve.h
index a400d21593b2..717567775f58 100644
--- a/include/linux/ve.h
+++ b/include/linux/ve.h
@@ -40,8 +40,7 @@ struct vfsmount;
 #define VE_STATE_STARTING	0
 #define VE_STATE_RUNNING	1
 #define VE_STATE_STOPPING	2
-#define VE_STATE_STOPPED	3
-#define VE_STATE_DEAD		4
+#define VE_STATE_DEAD		3
 
 #define VE_IS_RUNNING(ve) ((ve)->state == VE_STATE_RUNNING)
 
diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index 6cdfaaa4e703..b3988a8d590f 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -608,7 +608,7 @@ static int ve_start_container(struct ve_struct *ve)
 err_mark_ve:
 	ve_list_del(ve);
 err_list:
-	ve_set_state(ve, VE_STATE_STOPPED);
+	ve_set_state(ve, VE_STATE_DEAD);
 	ve_drop_context(ve);
 	return err;
 }
@@ -684,7 +684,7 @@ void ve_exit_ns(struct pid_namespace *pid_ns)
 	ve_hook_iterate_fini(VE_SS_CHAIN, ve);
 	ve_list_del(ve);
 	ve_drop_context(ve);
-	ve_set_state(ve, VE_STATE_STOPPED);
+	ve_set_state(ve, VE_STATE_DEAD);
 	up_write(&ve->op_sem);
 
 	printk(KERN_INFO "CT: %s: stopped\n", ve_name(ve));
@@ -939,7 +939,6 @@ static void ve_destroy(struct cgroup_subsys_state *css)
 	ve_free_vdso(ve);
 	mntput(ve->devtmpfs_mnt);
 	free_percpu(ve->sched_lat_ve.cur);
-	ve_set_state(ve, VE_STATE_DEAD);
 	kmem_cache_free(ve_cachep, ve);
 }
 
@@ -959,7 +958,6 @@ static int ve_state_show(struct seq_file *sf, void *v)
 	case VE_STATE_STOPPING:
 		seq_puts(sf, "STOPPING");
 		break;
-	case VE_STATE_STOPPED:
 	case VE_STATE_DEAD:
 		seq_puts(sf, "STOPPED");
 		break;

-- 
2.52.0

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

Re: [Devel] [PATCH hci-8.1 v4 1/5] ve: Remove STOPPED state

[no-reply]

> We don't need extra STOPPED state. It makes us feel that CT
> could be started again from it, but it is actually not true.
> 
> Separate STOPPED stated doesn't bring any value. Drop it and
> transition VE to DEAD state directly on the stop.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-142405
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261109-ovl-use-orig-ve-v4-0-88e65484dc5b@virtuozzo.com?part=1


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

[Devel] [PATCH hci-8.1 v4 2/5] ve: Introduce override_ve/revert_ve helpers

[Vladimir Riabchun]

These helpers mirror override_creds/revert_creds and are supposed to
be used together in cases when VE creds may be stored and later used
by the different VE task.

To keep task_ve consistent for other users, introduce a separate
task->overrided_ve field. Nobody should race on it since its
updates are supposed to go with override_creds, and non-current
processes shouldn't do this.

This field is not supposed to be inherited via fork, so report
a check for this was added as well.

https://virtuozzo.atlassian.net/browse/VSTOR-142405
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 include/linux/sched.h |  1 +
 include/linux/ve.h    | 27 +++++++++++++++++++++++++++
 kernel/capability.c   |  6 ++++--
 kernel/fork.c         |  2 ++
 4 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 10545e470d3d..7f4e6b1fde38 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1065,6 +1065,7 @@ struct task_struct {
 #ifdef CONFIG_VE
 	struct ve_struct		*task_ve;
 	struct ve_namespace		*ve_ns;
+	struct ve_struct		*overrided_ve; /* NULL when no overrides */
 #endif
 #ifdef CONFIG_PREEMPT_RT
 	struct netdev_xmit		net_xmit;
diff --git a/include/linux/ve.h b/include/linux/ve.h
index 717567775f58..fb8a4cabb951 100644
--- a/include/linux/ve.h
+++ b/include/linux/ve.h
@@ -295,6 +295,26 @@ static inline struct vdso_image *ve_get_vdso_64(struct ve_struct *ve)
 	return ve->vdso_64;
 }
 
+static inline struct ve_struct *override_ve(struct ve_struct *new_ve)
+{
+	struct ve_struct *old_ve = current->overrided_ve;
+
+	/* CT tasks are not supposed to switch VE context, this is a breach */
+	if (WARN_ON(new_ve != get_exec_env() && !ve_is_super(get_exec_env())))
+		return ERR_PTR(-EPERM);
+
+	current->overrided_ve = new_ve;
+	return old_ve;
+}
+
+static inline void revert_ve(struct ve_struct *orig_ve)
+{
+	/* Somebody forgot to check override_ve return value? */
+	if (WARN_ON(IS_ERR(orig_ve)))
+		return;
+	current->overrided_ve = orig_ve;
+}
+
 #else	/* CONFIG_VE */
 #include <linux/init_task.h>
 #include <net/net_namespace.h>
@@ -405,6 +425,13 @@ static inline struct vdso_image *ve_get_vdso_64(struct ve_struct *ve)
 }
 #endif /* CONFIG_X86 */
 
+static inline struct ve_struct *override_ve(struct ve_struct *new_ve)
+{
+	return NULL;
+}
+
+static inline void revert_ve(struct ve_struct *orig_ve) {}
+
 #endif	/* CONFIG_VE */
 
 struct seq_file;
diff --git a/kernel/capability.c b/kernel/capability.c
index 34716847c6b8..6de4dcba2283 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -389,10 +389,11 @@ EXPORT_SYMBOL(ns_capable);
 bool ve_capable(int cap)
 {
 	struct cred *cred;
+	struct ve_struct *ve = current->overrided_ve ?: get_exec_env();
 	bool ret;
 
 	rcu_read_lock();
-	cred = get_exec_env()->init_cred;
+	cred = ve->init_cred;
 
 	if (cred == NULL) /* ve isn't running */
 		cred = ve0.init_cred;
@@ -406,10 +407,11 @@ bool ve_capable(int cap)
 bool ve_capable_noaudit(int cap)
 {
 	struct cred *cred;
+	struct ve_struct *ve = current->overrided_ve ?: get_exec_env();
 	bool ret;
 
 	rcu_read_lock();
-	cred = get_exec_env()->init_cred;
+	cred = ve->init_cred;
 
 	if (cred == NULL) /* ve isn't running */
 		cred = ve0.init_cred;
diff --git a/kernel/fork.c b/kernel/fork.c
index 25e87cdbeb07..0cb8ccb2f4f3 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1161,6 +1161,8 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
 
 #ifdef CONFIG_VE
 	memset(tsk->alloc_lat, 0, sizeof(tsk->alloc_lat));
+	WARN_ON(orig->overrided_ve);
+	tsk->overrided_ve = NULL;
 #endif
 
 	/*

-- 
2.52.0

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

Re: [Devel] [PATCH hci-8.1 v4 2/5] ve: Introduce override_ve/revert_ve helpers

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] ve: override_ve() uses a non-ratelimited WARN_ON as its permission check
--

Commit dab91ee458061d579a92288ba48a09c48c12df09
Author: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

ve: Introduce override_ve/revert_ve helpers

This adds override_ve() and revert_ve() helpers that mirror
override_creds() and revert_creds(). It also adds a task->overrided_ve
field, which ve_capable() and ve_capable_noaudit() now consult, and a
check in dup_task_struct() that the field is not inherited on fork.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-142405

> diff --git a/include/linux/ve.h b/include/linux/ve.h
> --- a/include/linux/ve.h
> +++ b/include/linux/ve.h

[ ... ]

> @@ -295,6 +295,26 @@ static inline struct vdso_image *ve_get_vdso_64(struct ve_struct *ve)
>  	return ve->vdso_64;
>  }
>  
> +static inline struct ve_struct *override_ve(struct ve_struct *new_ve)
> +{
> +	struct ve_struct *old_ve = current->overrided_ve;
> +
> +	/* CT tasks are not supposed to switch VE context, this is a breach */
> +	if (WARN_ON(new_ve != get_exec_env() && !ve_is_super(get_exec_env())))
> +		return ERR_PTR(-EPERM);

[Severity: Low]
Can this WARN_ON fire on a legitimate path? override_ve() is called from
ovl_override_creds() with ofs->creator_ve, which is fixed at mount time
in ovl_fill_super() as get_ve(get_exec_env()).

If a container task reaches an overlay whose creator VE is not its own VE
(for example an overlay mounted by the host and exposed to the container,
perhaps through a bind mount), the condition
new_ve != get_exec_env() && !ve_is_super(get_exec_env())
is true on every lookup, permission check, getattr, readdir and so on.

Each of those operations would then log a warning splat and fail with
-EPERM, where they previously succeeded. I could not confirm from the
repository that such exposure actually occurs, so this depends on whether
it is reachable.

The WARN_ON is neither _ONCE nor ratelimited, so repeated failures can
flood the log, and with panic_on_warn set the system would panic. Would
a ratelimited or one-time warning, or no warning and just the -EPERM
return, be more appropriate for a permission check that callers already
handle?

> +
> +	current->overrided_ve = new_ve;
> +	return old_ve;
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261109-ovl-use-orig-ve-v4-0-88e65484dc5b@virtuozzo.com?part=2

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

Re: [Devel] [PATCH hci-8.1 v4 2/5] ve: Introduce override_ve/revert_ve helpers

[Vladimir Riabchun]

On 11/9/26 20:43, no-reply@sashiko.virtuozzo.com wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] ve: override_ve() uses a non-ratelimited WARN_ON as its permission check
> --
> 
> Commit dab91ee458061d579a92288ba48a09c48c12df09
> Author: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> 
> ve: Introduce override_ve/revert_ve helpers
> 
> This adds override_ve() and revert_ve() helpers that mirror
> override_creds() and revert_creds(). It also adds a task->overrided_ve
> field, which ve_capable() and ve_capable_noaudit() now consult, and a
> check in dup_task_struct() that the field is not inherited on fork.
> 
> Link: https://virtuozzo.atlassian.net/browse/VSTOR-142405
> 
>> diff --git a/include/linux/ve.h b/include/linux/ve.h
>> --- a/include/linux/ve.h
>> +++ b/include/linux/ve.h
> 
> [ ... ]
> 
>> @@ -295,6 +295,26 @@ static inline struct vdso_image *ve_get_vdso_64(struct ve_struct *ve)
>>   	return ve->vdso_64;
>>   }
>>   
>> +static inline struct ve_struct *override_ve(struct ve_struct *new_ve)
>> +{
>> +	struct ve_struct *old_ve = current->overrided_ve;
>> +
>> +	/* CT tasks are not supposed to switch VE context, this is a breach */
>> +	if (WARN_ON(new_ve != get_exec_env() && !ve_is_super(get_exec_env())))
>> +		return ERR_PTR(-EPERM);
> 
> [Severity: Low]
> Can this WARN_ON fire on a legitimate path? override_ve() is called from
> ovl_override_creds() with ofs->creator_ve, which is fixed at mount time
> in ovl_fill_super() as get_ve(get_exec_env()).

This is a security issue and we really don't want to
allow this. Just don't make an overlayfs in a CT over an overlayfs
from another, this is weird.

> 
> If a container task reaches an overlay whose creator VE is not its own VE
> (for example an overlay mounted by the host and exposed to the container,
> perhaps through a bind mount), the condition
> new_ve != get_exec_env() && !ve_is_super(get_exec_env())
> is true on every lookup, permission check, getattr, readdir and so on.
> 
> Each of those operations would then log a warning splat and fail with
> -EPERM, where they previously succeeded. I could not confirm from the
> repository that such exposure actually occurs, so this depends on whether
> it is reachable.
> 
> The WARN_ON is neither _ONCE nor ratelimited, so repeated failures can
> flood the log, and with panic_on_warn set the system would panic. Would
> a ratelimited or one-time warning, or no warning and just the -EPERM
> return, be more appropriate for a permission check that callers already
> handle?

We want to see a call trace for debugging purposes in such case.
I think WARN_ON is fine. User should see a bunch of warnings that
something is going terribly wrong and run to us and report it.

> 
>> +
>> +	current->overrided_ve = new_ve;
>> +	return old_ve;
>> +}
> 
> [ ... ]
> 

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

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

[Devel] [PATCH hci-8.1 v4 3/5] ve: Defer ve context drop until ve is completely destroyed

[Vladimir Riabchun]

We may have a case, when VE is dead, but there are some users of it,
overlayfs superblock for example.

This user may want to use VE creds, so we need to keep ve->init_cred alive.
Drop creds in ve_destroy. We need to drop ve_nsproxy early because
ploop waits for refcount going to zero, dropping ve_nsproxy in destroy
causes CT suspend/resume hangs.

https://virtuozzo.atlassian.net/browse/VSTOR-142405
Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 kernel/ve/ve.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index b3988a8d590f..1e0c092f080e 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -378,7 +378,10 @@ static void ve_drop_context(struct ve_struct *ve)
 	rcu_assign_pointer(ve->ve_nsproxy, NULL);
 	synchronize_rcu();
 	put_nsproxy(ve_nsproxy);
+}
 
+static void ve_drop_cred(struct ve_struct *ve)
+{
 	put_cred(ve->init_cred);
 	ve->init_cred = NULL;
 }
@@ -929,6 +932,7 @@ static void ve_destroy(struct cgroup_subsys_state *css)
 {
 	struct ve_struct *ve = css_to_ve(css);
 
+	ve_drop_cred(ve);
 	free_ve_devmnts(ve);
 
 	kmapset_unlink(&ve->sysfs_perms_key, &sysfs_ve_perms_set);

-- 
2.52.0

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

Re: [Devel] [PATCH hci-8.1 v4 3/5] ve: Defer ve context drop until ve is completely destroyed

[no-reply]

> We may have a case, when VE is dead, but there are some users of it,
> overlayfs superblock for example.
> 
> This user may want to use VE creds, so we need to keep ve->init_cred alive.
> Drop creds in ve_destroy. We need to drop ve_nsproxy early because
> ploop waits for refcount going to zero, dropping ve_nsproxy in destroy
> causes CT suspend/resume hangs.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-142405
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261109-ovl-use-orig-ve-v4-0-88e65484dc5b@virtuozzo.com?part=3


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

[Devel] [PATCH hci-8.1 v4 4/5] ovl: Keep creator VE in overlayfs superblock

[Vladimir Riabchun]

Overlayfs superblock stores creator creds in it, but these creds
should go together with the VE they were taken from.

Store the VE in the superblock. We can't use mount->owner_ve,
because several mounts can reference one superblock.

https://virtuozzo.atlassian.net/browse/VSTOR-142405

Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 fs/overlayfs/ovl_entry.h | 1 +
 fs/overlayfs/params.c    | 3 +++
 fs/overlayfs/super.c     | 3 +++
 3 files changed, 7 insertions(+)

diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index ab78001635e7..a6ad8b8de48c 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 32c58fcfe52f..c047f1265b8e 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 fb47711c8fb9..8ab39a35a873 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"
 
@@ -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());
+
 	err = -ENOMEM;
 	ofs->creator_cred = cred = prepare_creds();
 	if (!cred)

-- 
2.52.0

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

Re: [Devel] [PATCH hci-8.1 v4 4/5] ovl: Keep creator VE in overlayfs superblock

[no-reply]

> Overlayfs superblock stores creator creds in it, but these creds
> should go together with the VE they were taken from.
> 
> Store the VE in the superblock. We can't use mount->owner_ve,
> because several mounts can reference one superblock.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-142405
> 
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261109-ovl-use-orig-ve-v4-0-88e65484dc5b@virtuozzo.com?part=4


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

[Devel] [PATCH hci-8.1 v4 5/5] ve/overlayfs: Override VE together with creds

[Vladimir Riabchun]

Host users might want to access overlayfs owned by CT. In such case
ve_capable checks will fail - they will perform VE cred checks against
host user_ns.

Modify ovl_override_creds so that it returns both old cred and old VE.
This will also protect us from future rebase issues - all new users of
old_cred and ovl_override_creds will fail with compilation error.

This fixes an observed crash in overlayfs when checkpointing
a container with a 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: ...
[ 2300.895807] Call Trace:
[ 2300.896397]
[ 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.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.922814] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 2300.923823] RIP: 0033:0x7f5a2f8de3eb

Overlayfs was created inside CT, so ovl_fill_super stored
in-container credentials to ofs->creator_cred.

Those creds were 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.

Stroing ve_capable check result on creation is a possible fix,
but it just masks the problem. We have other paths from overlay to
ve_capable checks:

ovl_fileattr_set
  ovl_override_creds
  ovl_real_fileattr_set
    vfs_fileattr_set
      fileattr_set_prepare
        ve_capable

And storing capability check result will make rebase dangerous - we
would have to track new added code - not funny.

https://virtuozzo.atlassian.net/browse/VSTOR-142405

Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
---
 fs/overlayfs/copy_up.c   |  6 ++--
 fs/overlayfs/dir.c       | 59 ++++++++++++++++++------------
 fs/overlayfs/file.c      | 65 +++++++++++++++++++--------------
 fs/overlayfs/inode.c     | 94 +++++++++++++++++++++++++++++++-----------------
 fs/overlayfs/namei.c     | 39 +++++++++++++-------
 fs/overlayfs/overlayfs.h |  9 ++++-
 fs/overlayfs/readdir.c   | 36 ++++++++++++-------
 fs/overlayfs/util.c      | 41 ++++++++++++++++-----
 fs/overlayfs/xattrs.c    | 35 +++++++++++-------
 9 files changed, 252 insertions(+), 132 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 4388004a319d..f478d1b51af9 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -1215,7 +1215,7 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
 static int ovl_copy_up_flags(struct dentry *dentry, int flags)
 {
 	int err = 0;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	bool disconnected = (dentry->d_flags & DCACHE_DISCONNECTED);
 
 	/*
@@ -1235,7 +1235,7 @@ static int ovl_copy_up_flags(struct dentry *dentry, int flags)
 	if (err)
 		return err;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	err = ovl_override_creds(dentry->d_sb, &old_cred);
 	while (!err) {
 		struct dentry *next;
 		struct dentry *parent = NULL;
@@ -1260,7 +1260,7 @@ static int ovl_copy_up_flags(struct dentry *dentry, int flags)
 		dput(parent);
 		dput(next);
 	}
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 
 	return err;
 }
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 1c8009bf194b..23897fabe9d5 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -554,7 +554,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode,
 }
 
 static int ovl_setup_cred_for_create(struct dentry *dentry, struct inode *inode,
-				     umode_t mode, const struct cred *old_cred)
+				     umode_t mode, const struct ovl_creds_desc *old_cred)
 {
 	int err;
 	struct cred *override_cred;
@@ -566,7 +566,7 @@ static int ovl_setup_cred_for_create(struct dentry *dentry, struct inode *inode,
 	override_cred->fsuid = inode->i_uid;
 	override_cred->fsgid = inode->i_gid;
 	err = security_dentry_create_files_as(dentry, mode, &dentry->d_name,
-					      old_cred, override_cred);
+					      old_cred->cred, override_cred);
 	if (err) {
 		put_cred(override_cred);
 		return err;
@@ -581,10 +581,12 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
 			      struct ovl_cattr *attr, bool origin)
 {
 	int err;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	struct dentry *parent = dentry->d_parent;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	err = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (err)
+		return err;
 
 	/*
 	 * When linking a file with copy up origin into a new parent, mark the
@@ -610,7 +612,7 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
 		 * create a new inode, so just use the ovl mounter's
 		 * fs{u,g}id.
 		 */
-		err = ovl_setup_cred_for_create(dentry, inode, attr->mode, old_cred);
+		err = ovl_setup_cred_for_create(dentry, inode, attr->mode, &old_cred);
 		if (err)
 			goto out_revert_creds;
 	}
@@ -621,7 +623,7 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
 		err = ovl_create_over_whiteout(dentry, inode, attr);
 
 out_revert_creds:
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 	return err;
 }
 
@@ -697,12 +699,15 @@ static int ovl_symlink(struct mnt_idmap *idmap, struct inode *dir,
 
 static int ovl_set_link_redirect(struct dentry *dentry)
 {
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	int err;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	err = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (err)
+		return err;
+
 	err = ovl_set_redirect(dentry, false);
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 
 	return err;
 }
@@ -888,7 +893,7 @@ static void ovl_drop_nlink(struct dentry *dentry)
 static int ovl_do_remove(struct dentry *dentry, bool is_dir)
 {
 	int err;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	bool lower_positive = ovl_lower_positive(dentry);
 	LIST_HEAD(list);
 
@@ -907,12 +912,15 @@ static int ovl_do_remove(struct dentry *dentry, bool is_dir)
 	if (err)
 		goto out;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
-	if (!lower_positive)
-		err = ovl_remove_upper(dentry, is_dir, &list);
-	else
-		err = ovl_remove_and_whiteout(dentry, &list);
-	revert_creds(old_cred);
+	err = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (!err) {
+		if (!lower_positive)
+			err = ovl_remove_upper(dentry, is_dir, &list);
+		else
+			err = ovl_remove_and_whiteout(dentry, &list);
+		ovl_revert_creds(&old_cred);
+	}
+
 	if (!err) {
 		if (is_dir)
 			clear_nlink(dentry->d_inode);
@@ -1095,7 +1103,7 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 	bool new_is_dir = d_is_dir(new);
 	bool samedir = olddir == newdir;
 	struct dentry *opaquedir = NULL;
-	const struct cred *old_cred = NULL;
+	struct ovl_creds_desc old_cred = {};
 	struct ovl_fs *ofs = OVL_FS(old->d_sb);
 	LIST_HEAD(list);
 
@@ -1159,7 +1167,9 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 			goto out;
 	}
 
-	old_cred = ovl_override_creds(old->d_sb);
+	err = ovl_override_creds(old->d_sb, &old_cred);
+	if (err)
+		goto out_revert_creds;
 
 	if (!list_empty(&list)) {
 		opaquedir = ovl_clear_empty(new, &list);
@@ -1292,7 +1302,7 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 out_unlock:
 	unlock_rename(new_upperdir, old_upperdir);
 out_revert_creds:
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 	if (update_nlink)
 		ovl_nlink_end(new);
 	else
@@ -1306,7 +1316,7 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir,
 static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
 			      struct inode *inode, umode_t mode)
 {
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	struct path realparentpath;
 	struct file *realfile;
 	struct dentry *newdentry;
@@ -1314,8 +1324,11 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
 	int flags = file->f_flags | OVL_OPEN_FLAGS;
 	int err;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
-	err = ovl_setup_cred_for_create(dentry, inode, mode, old_cred);
+	err = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (err)
+		return err;
+
+	err = ovl_setup_cred_for_create(dentry, inode, mode, &old_cred);
 	if (err)
 		goto out_revert_creds;
 
@@ -1337,7 +1350,7 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
 		fput(realfile);
 	}
 out_revert_creds:
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 	return err;
 }
 
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 84aad9d19bdd..3762998be706 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -31,7 +31,7 @@ static struct file *ovl_open_realfile(const struct file *file,
 	struct inode *inode = file_inode(file);
 	struct mnt_idmap *real_idmap;
 	struct file *realfile;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	int flags = file->f_flags | OVL_OPEN_FLAGS;
 	int acc_mode = ACC_MODE(flags);
 	int err;
@@ -39,7 +39,10 @@ static struct file *ovl_open_realfile(const struct file *file,
 	if (flags & O_APPEND)
 		acc_mode |= MAY_APPEND;
 
-	old_cred = ovl_override_creds(inode->i_sb);
+	err = ovl_override_creds(inode->i_sb, &old_cred);
+	if (err)
+		return ERR_PTR(err);
+
 	real_idmap = mnt_idmap(realpath->mnt);
 	err = inode_permission(real_idmap, realinode, MAY_OPEN | acc_mode);
 	if (err) {
@@ -51,7 +54,7 @@ static struct file *ovl_open_realfile(const struct file *file,
 		realfile = backing_file_open(file, flags, realpath,
 					     current_cred());
 	}
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 
 	pr_debug("open(%p[%pD2/%c], 0%o) -> (%p, 0%o)\n",
 		 file, file, ovl_whatisit(inode, realinode), file->f_flags,
@@ -184,7 +187,7 @@ static loff_t ovl_llseek(struct file *file, loff_t offset, int whence)
 {
 	struct inode *inode = file_inode(file);
 	struct fd real;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	loff_t ret;
 
 	/*
@@ -213,9 +216,10 @@ static loff_t ovl_llseek(struct file *file, loff_t offset, int whence)
 	ovl_inode_lock(inode);
 	fd_file(real)->f_pos = file->f_pos;
 
-	old_cred = ovl_override_creds(inode->i_sb);
-	ret = vfs_llseek(fd_file(real), offset, whence);
-	revert_creds(old_cred);
+	ret = ovl_override_creds(inode->i_sb, &old_cred);
+	if (!ret)
+		ret = vfs_llseek(fd_file(real), offset, whence);
+	ovl_revert_creds(&old_cred);
 
 	file->f_pos = fd_file(real)->f_pos;
 	ovl_inode_unlock(inode);
@@ -392,7 +396,7 @@ static ssize_t ovl_splice_write(struct pipe_inode_info *pipe, struct file *out,
 static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync)
 {
 	struct fd real;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	int ret;
 
 	ret = ovl_sync_status(OVL_FS(file_inode(file)->i_sb));
@@ -405,9 +409,10 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync)
 
 	/* Don't sync lower file for fear of receiving EROFS error */
 	if (file_inode(fd_file(real)) == ovl_inode_upper(file_inode(file))) {
-		old_cred = ovl_override_creds(file_inode(file)->i_sb);
-		ret = vfs_fsync_range(fd_file(real), start, end, datasync);
-		revert_creds(old_cred);
+		ret = ovl_override_creds(file_inode(file)->i_sb, &old_cred);
+		if (!ret)
+			ret = vfs_fsync_range(fd_file(real), start, end, datasync);
+		ovl_revert_creds(&old_cred);
 	}
 
 	fdput(real);
@@ -431,7 +436,7 @@ static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len
 {
 	struct inode *inode = file_inode(file);
 	struct fd real;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	int ret;
 
 	inode_lock(inode);
@@ -445,9 +450,10 @@ static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len
 	if (ret)
 		goto out_unlock;
 
-	old_cred = ovl_override_creds(file_inode(file)->i_sb);
-	ret = vfs_fallocate(fd_file(real), mode, offset, len);
-	revert_creds(old_cred);
+	ret = ovl_override_creds(file_inode(file)->i_sb, &old_cred);
+	if (!ret)
+		ret = vfs_fallocate(fd_file(real), mode, offset, len);
+	ovl_revert_creds(&old_cred);
 
 	/* Update size */
 	ovl_file_modified(file);
@@ -463,16 +469,17 @@ static long ovl_fallocate(struct file *file, int mode, loff_t offset, loff_t len
 static int ovl_fadvise(struct file *file, loff_t offset, loff_t len, int advice)
 {
 	struct fd real;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	int ret;
 
 	ret = ovl_real_fdget(file, &real);
 	if (ret)
 		return ret;
 
-	old_cred = ovl_override_creds(file_inode(file)->i_sb);
-	ret = vfs_fadvise(fd_file(real), offset, len, advice);
-	revert_creds(old_cred);
+	ret = ovl_override_creds(file_inode(file)->i_sb, &old_cred);
+	if (!ret)
+		ret = vfs_fadvise(fd_file(real), offset, len, advice);
+	ovl_revert_creds(&old_cred);
 
 	fdput(real);
 
@@ -491,7 +498,7 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in,
 {
 	struct inode *inode_out = file_inode(file_out);
 	struct fd real_in, real_out;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	loff_t ret;
 
 	inode_lock(inode_out);
@@ -513,7 +520,10 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in,
 		goto out_unlock;
 	}
 
-	old_cred = ovl_override_creds(file_inode(file_out)->i_sb);
+	ret = ovl_override_creds(file_inode(file_out)->i_sb, &old_cred);
+	if (ret)
+		goto skip_switch;
+
 	switch (op) {
 	case OVL_COPY:
 		ret = vfs_copy_file_range(fd_file(real_in), pos_in,
@@ -531,7 +541,9 @@ static loff_t ovl_copyfile(struct file *file_in, loff_t pos_in,
 						flags);
 		break;
 	}
-	revert_creds(old_cred);
+
+skip_switch:
+	ovl_revert_creds(&old_cred);
 
 	/* Update size */
 	ovl_file_modified(file_out);
@@ -583,7 +595,7 @@ static loff_t ovl_remap_file_range(struct file *file_in, loff_t pos_in,
 static int ovl_flush(struct file *file, fl_owner_t id)
 {
 	struct fd real;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	int err;
 
 	err = ovl_real_fdget(file, &real);
@@ -591,9 +603,10 @@ static int ovl_flush(struct file *file, fl_owner_t id)
 		return err;
 
 	if (fd_file(real)->f_op->flush) {
-		old_cred = ovl_override_creds(file_inode(file)->i_sb);
-		err = fd_file(real)->f_op->flush(fd_file(real), id);
-		revert_creds(old_cred);
+		err = ovl_override_creds(file_inode(file)->i_sb, &old_cred);
+		if (!err)
+			err = fd_file(real)->f_op->flush(fd_file(real), id);
+		ovl_revert_creds(&old_cred);
 	}
 	fdput(real);
 
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 8b31f44c12cd..2d27bafad986 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -25,7 +25,7 @@ int ovl_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
 	bool full_copy_up = false;
 	struct dentry *upperdentry;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 
 	err = setattr_prepare(&nop_mnt_idmap, dentry, attr);
 	if (err)
@@ -78,9 +78,10 @@ int ovl_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 			goto out_put_write;
 
 		inode_lock(upperdentry->d_inode);
-		old_cred = ovl_override_creds(dentry->d_sb);
-		err = ovl_do_notify_change(ofs, upperdentry, attr);
-		revert_creds(old_cred);
+		err = ovl_override_creds(dentry->d_sb, &old_cred);
+		if (!err)
+			err = ovl_do_notify_change(ofs, upperdentry, attr);
+		ovl_revert_creds(&old_cred);
 		if (!err)
 			ovl_copyattr(dentry->d_inode);
 		inode_unlock(upperdentry->d_inode);
@@ -159,7 +160,7 @@ int ovl_getattr(struct mnt_idmap *idmap, const struct path *path,
 	struct dentry *dentry = path->dentry;
 	enum ovl_path_type type;
 	struct path realpath;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	struct inode *inode = d_inode(dentry);
 	bool is_dir = S_ISDIR(inode->i_mode);
 	int fsid = 0;
@@ -169,8 +170,10 @@ int ovl_getattr(struct mnt_idmap *idmap, const struct path *path,
 	metacopy_blocks = ovl_is_metacopy_dentry(dentry);
 
 	type = ovl_path_real(dentry, &realpath);
-	old_cred = ovl_override_creds(dentry->d_sb);
-	err = vfs_getattr_nosec(&realpath, stat, request_mask, flags);
+
+	err = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (!err)
+		err = vfs_getattr_nosec(&realpath, stat, request_mask, flags);
 	if (err)
 		goto out;
 
@@ -280,7 +283,7 @@ int ovl_getattr(struct mnt_idmap *idmap, const struct path *path,
 		stat->nlink = dentry->d_inode->i_nlink;
 
 out:
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 
 	return err;
 }
@@ -291,7 +294,7 @@ int ovl_permission(struct mnt_idmap *idmap,
 	struct inode *upperinode = ovl_inode_upper(inode);
 	struct inode *realinode;
 	struct path realpath;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	int err;
 
 	/* Careful in RCU walk mode */
@@ -309,7 +312,10 @@ int ovl_permission(struct mnt_idmap *idmap,
 	if (err)
 		return err;
 
-	old_cred = ovl_override_creds(inode->i_sb);
+	err = ovl_override_creds(inode->i_sb, &old_cred);
+	if (err)
+		return err;
+
 	if (!upperinode &&
 	    !special_file(realinode->i_mode) && mask & MAY_WRITE) {
 		mask &= ~(MAY_WRITE | MAY_APPEND);
@@ -317,7 +323,7 @@ int ovl_permission(struct mnt_idmap *idmap,
 		mask |= MAY_READ;
 	}
 	err = inode_permission(mnt_idmap(realpath.mnt), realinode, mask);
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 
 	return err;
 }
@@ -326,15 +332,19 @@ static const char *ovl_get_link(struct dentry *dentry,
 				struct inode *inode,
 				struct delayed_call *done)
 {
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	const char *p;
+	int err;
 
 	if (!dentry)
 		return ERR_PTR(-ECHILD);
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	err = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (err)
+		return ERR_PTR(err);
+
 	p = vfs_get_link(ovl_dentry_real(dentry), done);
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 	return p;
 }
 
@@ -465,11 +475,15 @@ struct posix_acl *do_ovl_get_acl(struct mnt_idmap *idmap,
 
 		acl = get_cached_acl_rcu(realinode, type);
 	} else {
-		const struct cred *old_cred;
+		struct ovl_creds_desc old_cred;
+		int err;
+
+		err = ovl_override_creds(inode->i_sb, &old_cred);
+		if (err)
+			return ERR_PTR(err);
 
-		old_cred = ovl_override_creds(inode->i_sb);
 		acl = ovl_get_acl_path(&realpath, posix_acl_xattr_name(type), noperm);
-		revert_creds(old_cred);
+		ovl_revert_creds(&old_cred);
 	}
 
 	return acl;
@@ -481,7 +495,7 @@ static int ovl_set_or_remove_acl(struct dentry *dentry, struct inode *inode,
 	int err;
 	struct path realpath;
 	const char *acl_name;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
 	struct dentry *upperdentry = ovl_dentry_upper(dentry);
 	struct dentry *realdentry = upperdentry ?: ovl_dentry_lower(dentry);
@@ -495,10 +509,14 @@ static int ovl_set_or_remove_acl(struct dentry *dentry, struct inode *inode,
 		struct posix_acl *real_acl;
 
 		ovl_path_lower(dentry, &realpath);
-		old_cred = ovl_override_creds(dentry->d_sb);
+
+		err = ovl_override_creds(dentry->d_sb, &old_cred);
+		if (err)
+			return err;
+
 		real_acl = vfs_get_acl(mnt_idmap(realpath.mnt), realdentry,
 				       acl_name);
-		revert_creds(old_cred);
+		ovl_revert_creds(&old_cred);
 		if (IS_ERR(real_acl)) {
 			err = PTR_ERR(real_acl);
 			goto out;
@@ -518,12 +536,17 @@ static int ovl_set_or_remove_acl(struct dentry *dentry, struct inode *inode,
 	if (err)
 		goto out;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	err = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (err)
+		goto out_revert;
+
 	if (acl)
 		err = ovl_do_set_acl(ofs, realdentry, acl_name, acl);
 	else
 		err = ovl_do_remove_acl(ofs, realdentry, acl_name);
-	revert_creds(old_cred);
+
+out_revert:
+	ovl_revert_creds(&old_cred);
 	ovl_drop_write(dentry);
 
 	/* copy c/mtime */
@@ -590,7 +613,7 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 {
 	int err;
 	struct inode *realinode = ovl_inode_realdata(inode);
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 
 	if (!realinode)
 		return -EIO;
@@ -598,9 +621,10 @@ static int ovl_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 	if (!realinode->i_op->fiemap)
 		return -EOPNOTSUPP;
 
-	old_cred = ovl_override_creds(inode->i_sb);
-	err = realinode->i_op->fiemap(realinode, fieinfo, start, len);
-	revert_creds(old_cred);
+	err = ovl_override_creds(inode->i_sb, &old_cred);
+	if (!err)
+		err = realinode->i_op->fiemap(realinode, fieinfo, start, len);
+	ovl_revert_creds(&old_cred);
 
 	return err;
 }
@@ -648,7 +672,7 @@ int ovl_fileattr_set(struct mnt_idmap *idmap,
 {
 	struct inode *inode = d_inode(dentry);
 	struct path upperpath;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	unsigned int flags;
 	int err;
 
@@ -660,7 +684,7 @@ int ovl_fileattr_set(struct mnt_idmap *idmap,
 		if (err)
 			goto out;
 
-		old_cred = ovl_override_creds(inode->i_sb);
+		err = ovl_override_creds(inode->i_sb, &old_cred);
 		/*
 		 * Store immutable/append-only flags in xattr and clear them
 		 * in upper fileattr (in case they were set by older kernel)
@@ -668,10 +692,11 @@ int ovl_fileattr_set(struct mnt_idmap *idmap,
 		 * "ovl-immutable" hardlinks could be copied up.
 		 * Clear xattr when flags are cleared.
 		 */
-		err = ovl_set_protattr(inode, upperpath.dentry, fa);
+		if (!err)
+			err = ovl_set_protattr(inode, upperpath.dentry, fa);
 		if (!err)
 			err = ovl_real_fileattr_set(&upperpath, fa);
-		revert_creds(old_cred);
+		ovl_revert_creds(&old_cred);
 		ovl_drop_write(dentry);
 
 		/*
@@ -725,15 +750,18 @@ int ovl_fileattr_get(struct dentry *dentry, struct fileattr *fa)
 {
 	struct inode *inode = d_inode(dentry);
 	struct path realpath;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	int err;
 
 	ovl_path_real(dentry, &realpath);
 
-	old_cred = ovl_override_creds(inode->i_sb);
+	err = ovl_override_creds(inode->i_sb, &old_cred);
+	if (err)
+		return err;
+
 	err = ovl_real_fileattr_get(&realpath, fa);
 	ovl_fileattr_prot_flags(inode, fa);
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 
 	return err;
 }
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index efa2a072aa69..edc11a7b27ee 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -969,15 +969,19 @@ static int ovl_maybe_validate_verity(struct dentry *dentry)
 		return err;
 
 	if (!ovl_test_flag(OVL_VERIFIED_DIGEST, inode)) {
-		const struct cred *old_cred;
+		struct ovl_creds_desc old_cred;
 
-		old_cred = ovl_override_creds(dentry->d_sb);
+		err = ovl_override_creds(dentry->d_sb, &old_cred);
+		if (err) {
+			ovl_inode_unlock(inode);
+			return err;
+		}
 
 		err = ovl_validate_verity(ofs, &metapath, &datapath);
 		if (err == 0)
 			ovl_set_flag(OVL_VERIFIED_DIGEST, inode);
 
-		revert_creds(old_cred);
+		ovl_revert_creds(&old_cred);
 	}
 
 	ovl_inode_unlock(inode);
@@ -991,7 +995,7 @@ static int ovl_maybe_lookup_lowerdata(struct dentry *dentry)
 	struct inode *inode = d_inode(dentry);
 	const char *redirect = ovl_lowerdata_redirect(inode);
 	struct ovl_path datapath = {};
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	int err;
 
 	if (!redirect || ovl_dentry_lowerdata(dentry))
@@ -1009,9 +1013,12 @@ static int ovl_maybe_lookup_lowerdata(struct dentry *dentry)
 	if (ovl_dentry_lowerdata(dentry))
 		goto out;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	err = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (err)
+		goto out_err;
+
 	err = ovl_lookup_data_layers(dentry, redirect, &datapath);
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 	if (err)
 		goto out_err;
 
@@ -1071,7 +1078,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			  unsigned int flags)
 {
 	struct ovl_entry *oe = NULL;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
 	struct ovl_entry *poe = OVL_E(dentry->d_parent);
 	struct ovl_entry *roe = OVL_E(dentry->d_sb->s_root);
@@ -1104,7 +1111,10 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 	if (dentry->d_name.len > ofs->namelen)
 		return ERR_PTR(-ENAMETOOLONG);
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	err = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (err)
+		return ERR_PTR(err);
+
 	upperdir = ovl_dentry_upper(dentry->d_parent);
 	if (upperdir) {
 		d.layer = &ofs->layers[0];
@@ -1378,7 +1388,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 
 	ovl_dentry_init_reval(dentry, upperdentry, OVL_I_E(inode));
 
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 	if (origin_path) {
 		dput(origin_path->dentry);
 		kfree(origin_path);
@@ -1402,7 +1412,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 	kfree(d.upperredirect);
 out:
 	kfree(d.redirect);
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 	return ERR_PTR(err);
 }
 
@@ -1410,7 +1420,7 @@ bool ovl_lower_positive(struct dentry *dentry)
 {
 	struct ovl_entry *poe = OVL_E(dentry->d_parent);
 	const struct qstr *name = &dentry->d_name;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	unsigned int i;
 	bool positive = false;
 	bool done = false;
@@ -1426,7 +1436,10 @@ bool ovl_lower_positive(struct dentry *dentry)
 	if (!ovl_dentry_upper(dentry))
 		return true;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	/* Ugh, hard - we can't pass an error from here. */
+	if (ovl_override_creds(dentry->d_sb, &old_cred))
+		return false;
+
 	/* Positive upper -> have to look up lower to see whether it exists */
 	for (i = 0; !done && !positive && i < ovl_numlower(poe); i++) {
 		struct dentry *this;
@@ -1459,7 +1472,7 @@ bool ovl_lower_positive(struct dentry *dentry)
 			dput(this);
 		}
 	}
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 
 	return positive;
 }
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 8577c7884fb9..4297912b0cc4 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -164,6 +164,12 @@ struct ovl_metacopy {
 	u8 digest[FS_VERITY_MAX_DIGEST_SIZE];  /* Only the used part on disk */
 } __packed;
 
+/* Credential information used by overlay */
+struct ovl_creds_desc {
+	const struct cred *cred;
+	struct ve_struct *ve;
+};
+
 #define OVL_METACOPY_MAX_SIZE (sizeof(struct ovl_metacopy))
 #define OVL_METACOPY_MIN_SIZE (OVL_METACOPY_MAX_SIZE - FS_VERITY_MAX_DIGEST_SIZE)
 #define OVL_METACOPY_INIT { 0, OVL_METACOPY_MIN_SIZE }
@@ -420,7 +426,8 @@ void ovl_end_write(struct dentry *dentry);
 int ovl_want_write(struct dentry *dentry);
 void ovl_drop_write(struct dentry *dentry);
 struct dentry *ovl_workdir(struct dentry *dentry);
-const struct cred *ovl_override_creds(struct super_block *sb);
+int ovl_override_creds(struct super_block *sb, struct ovl_creds_desc *desc);
+void ovl_revert_creds(struct ovl_creds_desc *old_creds);
 
 static inline const struct cred *ovl_creds(struct super_block *sb)
 {
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 0ca8af060b0c..478b0a32ff64 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -273,9 +273,11 @@ static int ovl_check_whiteouts(const struct path *path, struct ovl_readdir_data
 	int err;
 	struct ovl_cache_entry *p;
 	struct dentry *dentry, *dir = path->dentry;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 
-	old_cred = ovl_override_creds(rdd->dentry->d_sb);
+	err = ovl_override_creds(rdd->dentry->d_sb, &old_cred);
+	if (err)
+		return err;
 
 	err = down_write_killable(&dir->d_inode->i_rwsem);
 	if (!err) {
@@ -290,7 +292,7 @@ static int ovl_check_whiteouts(const struct path *path, struct ovl_readdir_data
 		}
 		inode_unlock(dir->d_inode);
 	}
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 
 	return err;
 }
@@ -753,10 +755,13 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx)
 	struct dentry *dentry = file->f_path.dentry;
 	struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
 	struct ovl_cache_entry *p;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	int err;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	err = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (err)
+		return err;
+
 	if (!ctx->pos)
 		ovl_dir_reset(file);
 
@@ -808,7 +813,7 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx)
 	}
 	err = 0;
 out:
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 	return err;
 }
 
@@ -856,11 +861,15 @@ static struct file *ovl_dir_open_realfile(const struct file *file,
 					  const struct path *realpath)
 {
 	struct file *res;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
+	int err;
+
+	err = ovl_override_creds(file_inode(file)->i_sb, &old_cred);
+	if (err)
+		return ERR_PTR(err);
 
-	old_cred = ovl_override_creds(file_inode(file)->i_sb);
 	res = ovl_path_open(realpath, O_RDONLY | (file->f_flags & O_LARGEFILE));
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 
 	return res;
 }
@@ -983,11 +992,14 @@ int ovl_check_empty_dir(struct dentry *dentry, struct list_head *list)
 	int err;
 	struct ovl_cache_entry *p, *n;
 	struct rb_root root = RB_ROOT;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
+
+	err = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (err)
+		return err;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
 	err = ovl_dir_read_merged(dentry, list, &root);
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 	if (err)
 		return err;
 
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index f95090f0d29c..d28d572d6099 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -63,11 +63,27 @@ struct dentry *ovl_workdir(struct dentry *dentry)
 	return ofs->workdir;
 }
 
-const struct cred *ovl_override_creds(struct super_block *sb)
+/* On error no revert is needed. */
+int ovl_override_creds(struct super_block *sb, struct ovl_creds_desc *desc)
 {
 	struct ovl_fs *ofs = OVL_FS(sb);
 
-	return override_creds(ofs->creator_cred);
+	desc->ve = override_ve(ofs->creator_ve);
+	if (IS_ERR(desc->ve))
+		return PTR_ERR(desc->ve);
+
+	desc->cred = override_creds(ofs->creator_cred);
+	return 0;
+}
+
+/* Accepts desc from a failed ovl_override_creds and does nothing. */
+void ovl_revert_creds(struct ovl_creds_desc *old_creds)
+{
+	if (IS_ERR(old_creds->ve))
+		return;
+
+	revert_creds(old_creds->cred);
+	revert_ve(old_creds->ve);
 }
 
 /*
@@ -1150,7 +1166,7 @@ static void ovl_cleanup_index(struct dentry *dentry)
 int ovl_nlink_start(struct dentry *dentry)
 {
 	struct inode *inode = d_inode(dentry);
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	int err;
 
 	if (WARN_ON(!inode))
@@ -1187,7 +1203,10 @@ int ovl_nlink_start(struct dentry *dentry)
 	if (d_is_dir(dentry) || !ovl_test_flag(OVL_INDEX, inode))
 		return 0;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	err = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (err)
+		goto out_drop_write;
+
 	/*
 	 * The overlay inode nlink should be incremented/decremented IFF the
 	 * upper operation succeeds, along with nlink change of upper inode.
@@ -1195,7 +1214,7 @@ int ovl_nlink_start(struct dentry *dentry)
 	 * value relative to the upper inode nlink in an upper inode xattr.
 	 */
 	err = ovl_set_nlink_upper(dentry);
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 	if (err)
 		goto out_drop_write;
 
@@ -1216,11 +1235,15 @@ void ovl_nlink_end(struct dentry *dentry)
 	ovl_drop_write(dentry);
 
 	if (ovl_test_flag(OVL_INDEX, inode) && inode->i_nlink == 0) {
-		const struct cred *old_cred;
+		struct ovl_creds_desc old_cred;
+		int err;
 
-		old_cred = ovl_override_creds(dentry->d_sb);
-		ovl_cleanup_index(dentry);
-		revert_creds(old_cred);
+		err = ovl_override_creds(dentry->d_sb, &old_cred);
+		if (err)
+			pr_err("%s: skipping ovl_cleanup_index.\n", __func__);
+		else
+			ovl_cleanup_index(dentry);
+		ovl_revert_creds(&old_cred);
 	}
 
 	ovl_inode_unlock(inode);
diff --git a/fs/overlayfs/xattrs.c b/fs/overlayfs/xattrs.c
index 0ed2b049056d..7528ec76868d 100644
--- a/fs/overlayfs/xattrs.c
+++ b/fs/overlayfs/xattrs.c
@@ -41,13 +41,16 @@ static int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char
 	struct dentry *upperdentry = ovl_i_dentry_upper(inode);
 	struct dentry *realdentry = upperdentry ?: ovl_dentry_lower(dentry);
 	struct path realpath;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 
 	if (!value && !upperdentry) {
 		ovl_path_lower(dentry, &realpath);
-		old_cred = ovl_override_creds(dentry->d_sb);
+		err = ovl_override_creds(dentry->d_sb, &old_cred);
+		if (err)
+			return err;
+
 		err = vfs_getxattr(mnt_idmap(realpath.mnt), realdentry, name, NULL, 0);
-		revert_creds(old_cred);
+		ovl_revert_creds(&old_cred);
 		if (err < 0)
 			goto out;
 	}
@@ -64,7 +67,12 @@ static int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char
 	if (err)
 		goto out;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	err = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (err) {
+		ovl_drop_write(dentry);
+		return err;
+	}
+
 	if (value) {
 		err = ovl_do_setxattr(ofs, realdentry, name, value, size,
 				      flags);
@@ -72,7 +80,7 @@ static int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char
 		WARN_ON(flags != XATTR_REPLACE);
 		err = ovl_do_removexattr(ofs, realdentry, name);
 	}
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 	ovl_drop_write(dentry);
 
 	/* copy c/mtime */
@@ -85,13 +93,14 @@ static int ovl_xattr_get(struct dentry *dentry, struct inode *inode, const char
 			 void *value, size_t size)
 {
 	ssize_t res;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	struct path realpath;
 
 	ovl_i_path_real(inode, &realpath);
-	old_cred = ovl_override_creds(dentry->d_sb);
-	res = vfs_getxattr(mnt_idmap(realpath.mnt), realpath.dentry, name, value, size);
-	revert_creds(old_cred);
+	res = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (!res)
+		res = vfs_getxattr(mnt_idmap(realpath.mnt), realpath.dentry, name, value, size);
+	ovl_revert_creds(&old_cred);
 	return res;
 }
 
@@ -116,12 +125,14 @@ ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
 	ssize_t res;
 	size_t len;
 	char *s;
-	const struct cred *old_cred;
+	struct ovl_creds_desc old_cred;
 	size_t prefix_len, name_len;
 
-	old_cred = ovl_override_creds(dentry->d_sb);
+	res = ovl_override_creds(dentry->d_sb, &old_cred);
+	if (res)
+		return res;
 	res = vfs_listxattr(realdentry, list, size);
-	revert_creds(old_cred);
+	ovl_revert_creds(&old_cred);
 	if (res <= 0 || size == 0)
 		return res;
 

-- 
2.52.0

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

Re: [Devel] [PATCH hci-8.1 v4 5/5] ve/overlayfs: Override VE together with creds

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ovl: ovl_override_creds() fails with WARN and -EPERM for CT callers of a foreign-VE overlay
--

commit 9e04559c3517d9690eb025bf97e30b602d11f4ca
Author: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>

ve/overlayfs: Override VE together with creds

This patch changes ovl_override_creds() to also override the VE
(through override_ve()) using ofs->creator_ve. It returns both the old
cred and the old VE in a new struct ovl_creds_desc, and converts all
overlayfs callers to the new ovl_override_creds()/ovl_revert_creds()
pair. The stated goal is to let a host user access an overlayfs created
inside a CT without ve_capable() checks failing.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-142405

> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index f95090f0d29c..d28d572d6099 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c

[ ... ]

> @@ -63,11 +63,27 @@ struct dentry *ovl_workdir(struct dentry *dentry)
>  	return ofs->workdir;
>  }
>  
> -const struct cred *ovl_override_creds(struct super_block *sb)
> +/* On error no revert is needed. */
> +int ovl_override_creds(struct super_block *sb, struct ovl_creds_desc *desc)
>  {
>  	struct ovl_fs *ofs = OVL_FS(sb);
>  
> -	return override_creds(ofs->creator_cred);
> +	desc->ve = override_ve(ofs->creator_ve);
> +	if (IS_ERR(desc->ve))
> +		return PTR_ERR(desc->ve);
> +
> +	desc->cred = override_creds(ofs->creator_cred);
> +	return 0;
> +}

[Severity: Medium]
Can this turn a previously working access into a WARN plus -EPERM for
tasks running in a container?

ovl_fill_super() stores ofs->creator_ve = get_ve(get_exec_env()), so
creator_ve is the VE of the mounting task. Every converted entry point
(ovl_lookup(), ovl_permission(), ovl_getattr(), ovl_iterate(),
ovl_open_realfile(), the xattr and copy-up paths) now calls
ovl_override_creds(), which calls override_ve(ofs->creator_ve).

With CONFIG_VE, override_ve() in include/linux/ve.h does:

    if (WARN_ON(new_ve != get_exec_env() && !ve_is_super(get_exec_env())))
        return ERR_PTR(-EPERM);

That condition is true for any non-host task whose VE differs from
creator_ve. One example is a CT task reaching a host-mounted overlay
through a bind mount or mount propagation. Another is a CT task reaching
another CT's overlay. In both cases, ovl_override_creds() returns -EPERM
and the operation fails.

Before this patch only the creds were swapped, so that access worked.
Each failure now also emits a WARN backtrace, which is a panic with
panic_on_warn. Is this reachable from within a container?

The commit message covers only the host-accessing-CT direction and does
not mention this one. Is it intended that a CT task accessing an overlay
created in a different VE fails with -EPERM?

If it is intended, would it be better to avoid the WARN_ON backtrace on
this path, for example by checking the VE relationship in
ovl_override_creds() before calling override_ve()?

[ ... ]

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261109-ovl-use-orig-ve-v4-0-88e65484dc5b@virtuozzo.com?part=5

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

Re: [Devel] [PATCH hci-8.1 v4 5/5] ve/overlayfs: Override VE together with creds

[Vladimir Riabchun]

On 11/9/26 20:49, no-reply@sashiko.virtuozzo.com wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] ovl: ovl_override_creds() fails with WARN and -EPERM for CT callers of a foreign-VE overlay
> --
> 
> commit 9e04559c3517d9690eb025bf97e30b602d11f4ca
> Author: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> 
> ve/overlayfs: Override VE together with creds
> 
> This patch changes ovl_override_creds() to also override the VE
> (through override_ve()) using ofs->creator_ve. It returns both the old
> cred and the old VE in a new struct ovl_creds_desc, and converts all
> overlayfs callers to the new ovl_override_creds()/ovl_revert_creds()
> pair. The stated goal is to let a host user access an overlayfs created
> inside a CT without ve_capable() checks failing.
> 
> Link: https://virtuozzo.atlassian.net/browse/VSTOR-142405
> 
>> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
>> index f95090f0d29c..d28d572d6099 100644
>> --- a/fs/overlayfs/util.c
>> +++ b/fs/overlayfs/util.c
> 
> [ ... ]
> 
>> @@ -63,11 +63,27 @@ struct dentry *ovl_workdir(struct dentry *dentry)
>>   	return ofs->workdir;
>>   }
>>   
>> -const struct cred *ovl_override_creds(struct super_block *sb)
>> +/* On error no revert is needed. */
>> +int ovl_override_creds(struct super_block *sb, struct ovl_creds_desc *desc)
>>   {
>>   	struct ovl_fs *ofs = OVL_FS(sb);
>>   
>> -	return override_creds(ofs->creator_cred);
>> +	desc->ve = override_ve(ofs->creator_ve);
>> +	if (IS_ERR(desc->ve))
>> +		return PTR_ERR(desc->ve);
>> +
>> +	desc->cred = override_creds(ofs->creator_cred);
>> +	return 0;
>> +}
> 
> [Severity: Medium]
> Can this turn a previously working access into a WARN plus -EPERM for
> tasks running in a container?
> 
> ovl_fill_super() stores ofs->creator_ve = get_ve(get_exec_env()), so
> creator_ve is the VE of the mounting task. Every converted entry point
> (ovl_lookup(), ovl_permission(), ovl_getattr(), ovl_iterate(),
> ovl_open_realfile(), the xattr and copy-up paths) now calls
> ovl_override_creds(), which calls override_ve(ofs->creator_ve).
> 
> With CONFIG_VE, override_ve() in include/linux/ve.h does:
> 
>      if (WARN_ON(new_ve != get_exec_env() && !ve_is_super(get_exec_env())))
>          return ERR_PTR(-EPERM);
> 
> That condition is true for any non-host task whose VE differs from
> creator_ve. One example is a CT task reaching a host-mounted overlay
> through a bind mount or mount propagation. Another is a CT task reaching
> another CT's overlay. In both cases, ovl_override_creds() returns -EPERM
> and the operation fails.
> 
> Before this patch only the creds were swapped, so that access worked.
> Each failure now also emits a WARN backtrace, which is a panic with
> panic_on_warn. Is this reachable from within a container?
> 
> The commit message covers only the host-accessing-CT direction and does
> not mention this one. Is it intended that a CT task accessing an overlay
> created in a different VE fails with -EPERM?
> 
> If it is intended, would it be better to avoid the WARN_ON backtrace on
> this path, for example by checking the VE relationship in
> ovl_override_creds() before calling override_ve()?

This is the whole idea. We never want to upgrade our privileges,
so this should be restricted.

If this happens - it is either someone trying to use host overlay
mount inside CT, or a CT/kernel is doing something nasty.
Both cases should be reported loudly.

> 
> [ ... ]
> 

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

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