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()? [ ... ] -- Sashiko AI review ยท https://sashiko.vzint.dev/#/patchset/[email protected]?part=5 _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
