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

[Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>]

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