Re: [PATCH v7 5/5] vdso/gettimeofday: Assert that the clockid fits into the u32 bitmask

[Thomas Gleixner]

On Fri, Oct 02 2026 at 23:31, Zhan Xusheng wrote:
> __cvdso_clock_gettime_common() and __cvdso_clock_getres_common() convert
> the clockid into a bitmask and match it against VDSO_HRES, VDSO_COARSE,
> VDSO_RAW and VDSO_AUX:
>
> 	msk = 1U << clock;
>
> vdso_clockid_valid() rejects anything above CLOCK_AUX_LAST beforehand,
> and CLOCK_AUX_LAST is 23, so the shift count is in range.  Nothing
> records that dependency though.  Raising MAX_AUX_CLOCKS beyond 16 moves
> CLOCK_AUX_LAST to 32 and makes the shift undefined.
>
> Add a BUILD_BUG_ON() at both conversion sites.  The condition is on a
> function parameter rather than a constant, so it relies on the compiler
> deriving the range from the vdso_clockid_valid() bail-out above it.  gcc
> 13 and clang 18 both do: the x86 vdso64 and vdso32 builds stay clean, and
> raising MAX_AUX_CLOCKS to 17 trips the assert.

But using 'clock' for the assert is fragile as it relies on the compiler
to defer the maximum 'clock' value from vdso_clockid_valid().

Checking for CLOCK_AUX_LAST >= BITS_PER_TYPE() should be sufficient, no?

> diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
> index f7a591aba59f..ef4dcc614489 100644
> --- a/lib/vdso/gettimeofday.c
> +++ b/lib/vdso/gettimeofday.c
> @@ -285,6 +285,7 @@ __cvdso_clock_gettime_common(const struct vdso_time_data *vd, clockid_t clock,
>  	 * Convert the clockid to a bitmask and use it to check which
>  	 * clocks are handled in the VDSO directly.
>  	 */
> +	BUILD_BUG_ON(clock >= BITS_PER_TYPE(msk));
>  	msk = 1U << clock;
>  	if (likely(msk & VDSO_HRES))
>  		vc = &vc[CS_HRES_COARSE];
> @@ -438,6 +439,7 @@ bool __cvdso_clock_getres_common(const struct vdso_time_data *vd, clockid_t cloc
>  	 * Convert the clockid to a bitmask and use it to check which
>  	 * clocks are handled in the VDSO directly.
>  	 */
> +	BUILD_BUG_ON(clock >= BITS_PER_TYPE(msk));
>  	msk = 1U << clock;
>  	if (msk & (VDSO_HRES | VDSO_RAW)) {
>  		/*

Aside of that why do we need the same BUILD_BUG_ON() in two functions?

Thanks,

        tglx