Re: [Devel] [PATCH hci-8.1 3/4] ms/timens: Remove dependency on the vDSO

[no-reply@sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] timens: TIME_NS now selectable on SPARC, whose private vDSO ignores offsets
--

commit 07e6081c6b5ec1169db1364534dcb49acce62383
Author: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

ms/timens: Remove dependency on the vDSO

This backport drops "depends on GENERIC_GETTIMEOFDAY" from TIME_NS. It adds
a TIME_NS_VDSO symbol that gates the vDSO-specific code, and it stubs out
timens_commit() and find_timens_vvar_page() when TIME_NS_VDSO is not set.

Link: https://patch.msgid.link/20260326-vdso-timens-decoupling-v2-2-c82693a7775f@linutronix.de

> diff --git a/init/Kconfig b/init/Kconfig
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1306,12 +1306,14 @@ config UTS_NS
>  
>  config TIME_NS
>  	bool "TIME namespace"
> -	depends on GENERIC_GETTIMEOFDAY
>  	default y
>  	help
>  	  In this namespace boottime and monotonic clocks can be set.
>  	  The time will keep going with the same pace.
>  
> +config TIME_NS_VDSO
> +	def_bool TIME_NS && GENERIC_GETTIMEOFDAY
> +

[Severity: Medium]
Can TIME_NS now be enabled on SPARC with a vDSO that ignores the offsets?

The commit message says "Now that all architectures using a real vDSO are
using the generic library, that dependency is not necessary anymore."
That does not appear to hold for SPARC in this tree. arch/sparc/Kconfig
selects only GENERIC_TIME_VSYSCALL. It does not select
GENERIC_GETTIMEOFDAY or HAVE_GENERIC_VDSO. The tree still contains
arch/sparc/vdso/vclock_gettime.c and
arch/sparc/vdso/vdso32/vclock_gettime.c.

Those files read the host vvar page directly:

arch/sparc/vdso/vclock_gettime.c:__vdso_clock_gettime() {
    ...
    case CLOCK_MONOTONIC:
        if (unlikely(vvd->vclock_mode == VCLOCK_NONE))
            break;
        return do_monotonic(vvd, ts);
    ...
}

They have no time namespace offset handling. Before this patch, the
GENERIC_GETTIMEOFDAY dependency kept TIME_NS (default y) off on SPARC.
With the dependency gone, TIME_NS is on by default there.
TIME_NS_VDSO is then n, so these two stubs do nothing:

include/linux/time_namespace.h:
#else /* !CONFIG_TIME_NS_VDSO */
static inline void timens_commit(struct task_struct *tsk, struct time_namespace *ns)
{
}

static inline struct page *find_timens_vvar_page(struct vm_area_struct *vma)
{
	return NULL;
}

On SPARC, unshare(CLONE_NEWTIME), setns() and writes to
/proc/<pid>/timens_offsets would all succeed. A task in a non-init time
namespace would still get host CLOCK_MONOTONIC and CLOCK_MONOTONIC_COARSE
from the vDSO, with no offset applied. The syscall path, /proc/uptime and
timerfd do apply the offset, so the two paths would disagree, and nothing
reports an error.

Should TIME_NS keep a dependency that excludes architectures whose vDSO is
not the generic one, such as SPARC? Alternatively, should the commit
message be corrected?

> diff --git a/include/linux/time_namespace.h b/include/linux/time_namespace.h
> --- a/include/linux/time_namespace.h
> +++ b/include/linux/time_namespace.h

[ ... ]

> +#ifdef CONFIG_TIME_NS_VDSO
> +extern void timens_commit(struct task_struct *tsk, struct time_namespace *ns);
> +struct page *find_timens_vvar_page(struct vm_area_struct *vma);
> +#else /* !CONFIG_TIME_NS_VDSO */
> +static inline void timens_commit(struct task_struct *tsk, struct time_namespace *ns)
> +{
> +}
> +
> +static inline struct page *find_timens_vvar_page(struct vm_area_struct *vma)
> +{
> +	return NULL;
> +}
> +#endif /* CONFIG_TIME_NS_VDSO */
> +
>  #endif /* _LINUX_TIMENS_H */

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261009135929.3274885-1-eva.kurchatova@virtuozzo.com?part=3

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