[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHmME9rpdCGLQzfsNkX=mLHfWeEWi4TyrOf_2rP_9hsyX9v6ow@mail.gmail.com>
Date: Wed, 30 Nov 2022 16:12:58 +0100
From: "Jason A. Donenfeld" <Jason@...c4.com>
To: Arnd Bergmann <arnd@...db.de>
Cc: Florian Weimer <fweimer@...hat.com>, linux-kernel@...r.kernel.org,
patches@...ts.linux.dev, Thomas Gleixner <tglx@...utronix.de>,
linux-crypto@...r.kernel.org, linux-api@...r.kernel.org,
x86@...nel.org, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Adhemerval Zanella Netto <adhemerval.zanella@...aro.org>,
"Carlos O'Donell" <carlos@...hat.com>,
Christian Brauner <brauner@...nel.org>
Subject: Re: [PATCH v10 3/4] random: introduce generic vDSO getrandom() implementation
Hi Arnd,
On Wed, Nov 30, 2022 at 4:07 PM Arnd Bergmann <arnd@...db.de> wrote:
> > +#ifdef CONFIG_64BIT
> > +typedef u64 vdso_kernel_ulong;
> > +#else
> > +typedef u32 vdso_kernel_ulong;
> > +#endif
>
> This does not address the ABI concern: to allow 32-bit and 64-bit
> tasks to share the same data page, it has to be the same width on
> both, either u32 or 64, but not depending on a configuration
> option.
I think it does address the issue. CONFIG_64BIT is a .config setting,
not a compiler-derived setting. So a 64-bit kernel will get a u64 in
kernel mode, and then it will get a u64 for the 64-bit vdso usermode
compile, and finally it will get a u64 for the 32-bit vdso usermode
compile. So in all three cases, the size is the same.
> > struct vdso_rng_data {
> > vdso_kernel_ulong generation;
> > bool is_ready;
> > };
>
> There is another problem with this: you have implicit padding
> in the structure because the two members have different size
> and alignment requirements. The easiest fix is to make them
> both u64, or you could have a u32 is_ready and an explit u32
> for the padding.
There's padding at the end of the structure, yes. But both
`generation` and `is_ready` will be at the same offset. If the
structure grows, then sure, that'll have to be taken into account. But
that's not a problem because this is a private implementation detail
between the vdso code and the kernel.
Jason
Powered by blists - more mailing lists