Re: [PATCH 2/2] Fixup for 3279be36b671 ("powerpc/vdso: Wire up getrandom() vDSO implementation on VDSO32")

From: Christophe Leroy
Date: Fri Sep 06 2024 - 11:14:55 EST




Le 06/09/2024 à 16:46, Jason A. Donenfeld a écrit :
On Fri, Sep 06, 2024 at 04:26:32PM +0200, Christophe Leroy wrote:

On the long run I wonder if we should try to find a more generic
solution for getrandom instead of requiring each architecture to handle
it. On gettimeofday the selection of the right page is embeded in the
generic part, see for instance :

static __maybe_unused __kernel_old_time_t
__cvdso_time_data(const struct vdso_data *vd, __kernel_old_time_t *time)
{
__kernel_old_time_t t;

if (IS_ENABLED(CONFIG_TIME_NS) &&
vd->clock_mode == VDSO_CLOCKMODE_TIMENS)
vd = __arch_get_timens_vdso_data(vd);

t = READ_ONCE(vd[CS_HRES_COARSE].basetime[CLOCK_REALTIME].sec);

if (time)
*time = t;

return t;
}

and powerpc just provides:

static __always_inline
const struct vdso_data *__arch_get_timens_vdso_data(const struct
vdso_data *vd)
{
return (void *)vd + (1U << CONFIG_PAGE_SHIFT);
}

It's tempting, but maybe a bit tricky. LoongArch, for example, doesn't
have this problem at all, because the layout of their vvars doesn't
require it. So the vd->clock_mode access is unnecessary.

Or another solution could be to put random data in a third page that is
always at the same place regardless of timens ?

Maybe that's the easier way, yea. Potentially wasteful, though.


Indeed I just looked at Loongarch and that's exactly what they do: they have a third page after the two pages dedicated to TIME for arch specific data, and they have added getrandom data there.

The third page is common to every process so it won't waste more than a few bytes. It doesn't worry me even on the older boards that only have 32 Mbytes of RAM.

So yes, I may have a look at that in the future, what we have at the moment is good enough to move forward.

Christophe