On 11/9/26 20:43, [email protected] 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 <[email protected]>
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
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
