Re: [Devel] [PATCH VZ10 v9 7/8] ve/printk: Don't use ve log outside task context

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

Sashiko review:

On 9/15/26 19:22, Vladimir Riabchun wrote:
> get_exec_env() in interrupt context will return VE of the process
> that was interrupted.
> We don't want to leak logs to unrelated containers if we use
> ve_printk in a function that may be called in an interrupt.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-135520
> Signed-off-by: Vladimir Riabchun <vladimir.riabchun@virtuozzo.com>
> ---
>   kernel/printk/printk.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 8940b36e790b..90ecd7960800 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -560,7 +560,8 @@ struct log_state *ve_log_state(void)
>   {
>   	struct log_state *log = &init_log_state;
>   #ifdef CONFIG_VE
> -	if (get_exec_env()->log_state)
> +	/* get_exec_env() is not reliable outside task context */
> +	if (get_exec_env()->log_state && in_task())

Sashiko says:
  This is a pre-existing issue, but can the condition in "ve_log_state()" at
  "if (get_exec_env()->log_state && in_task())" lead to incorrectly accessing
  log state outside of task contexts, if callers aren't properly enforcing
  the in_task() guarantee?

This is exactly what the patch is doing: this updated check ensures
that even if somebody forgot to enforce this check or we somehow reached
this from interrupt, we leak nothing to CT.

>   		log = get_exec_env()->log_state;
>   #endif
>   	return log;
> @@ -2538,7 +2539,7 @@ static asmlinkage int ve_vprintk(int dst, const char *fmt, va_list args)
>   	va_copy(args2, args);
>   	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
>   		r = vprintk(fmt, args);
> -	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG))
> +	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG) && in_task())

Sashiko says:
  In "ve_vprintk", is it possible that the "ve_vprintk_emit()" call could cause
  a use-after-free scenario given that it lacks an in_task() check,
  unlike similar calls that were updated in the patch?
  Could accessing "get_exec_env()->log_state" without this check lead to
  accessing unstable memory, causing kernel panics?

ve_vprintk_emit uses ve_log_state helper, so it has in_task() check.

>   		r = ve_vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, fmt, args2);
>   	va_end(args2);
>   
> @@ -2570,7 +2571,7 @@ static asmlinkage int ve_vprintk_deferred(int dst, const char *fmt, va_list args
>   	va_copy(args2, args);
>   	if (ve_is_super(get_exec_env()) || (dst & VE0_LOG))
>   		r = vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args);
> -	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG))
> +	if (!ve_is_super(get_exec_env()) && (dst & VE_LOG) && in_task())
>   		r = ve_vprintk_emit(0, LOGLEVEL_SCHED, NULL, fmt, args2);
>   	va_end(args2);
>   

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

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