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