Re: [PATCH 1/2] powerpc/vdso: Fix VDSO data access when running in a non-root time namespace

[Michael Ellerman <mpe@ellerman.id.au>]

"Jason A. Donenfeld" <Jason@zx2c4.com> writes:
> On Fri, Sep 06, 2024 at 03:43:17PM +0200, Jason A. Donenfeld wrote:
>> On Fri, Sep 06, 2024 at 10:23:08PM +1000, Michael Ellerman wrote:
>> > Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> > > When running in a non-root time namespace, the global VDSO data page
>> > > is replaced by a dedicated namespace data page and the global data
>> > > page is mapped next to it. Detailed explanations can be found at
>> > > commit 660fd04f9317 ("lib/vdso: Prepare for time namespace support").
>> > >
>> > > When it happens, __kernel_get_syscall_map and __kernel_get_tbfreq
>> > > and __kernel_sync_dicache don't work anymore because they read 0
>> > > instead of the data they need.
>> > >
>> > > To address that, clock_mode has to be read. When it is set to
>> > > VDSO_CLOCKMODE_TIMENS, it means it is a dedicated namespace data page
>> > > and the global data is located on the following page.
>> > >
>> > > Add a macro called get_realdatapage which reads clock_mode and add
>> > > PAGE_SIZE to the pointer provided by get_datapage macro when
>> > > clock_mode is equal to VDSO_CLOCKMODE_TIMENS. Use this new macro
>> > > instead of get_datapage macro except for time functions as they handle
>> > > it internally.
>> > >
>> > > Fixes: 74205b3fc2ef ("powerpc/vdso: Add support for time namespaces")
>> > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> >  
>> > Oops.
>> > 
>> > I guess it should also have:
>> > 
>> >   Cc: stable@vger.kernel.org # v5.13+
>> >   Reported-by: Jason A. Donenfeld <Jason@zx2c4.com>
>> >   Closes: https://lore.kernel.org/all/ZtnYqZI-nrsNslwy@zx2c4.com/
>> > 
>> > Jason how do you want to handle this?
>> > 
>> > I can put patch 1 in a topic branch that we both merge? Then you can
>> > apply patch 2 on top of that merge in your tree.
>> > 
>> > Or we could both apply patch 1 to our trees, it might lead to a conflict
>> > but it wouldn't be anything drastic.
>> 
>> The merge window for 6.12 is pretty soon. Why don't I just take this in
>> my random.git tree (with your ack) as a prereq to the ppc vDSO work.
>> It'll slide in _before_ Christophe's other commits, and then the
>> separate vgetrandom fixup will be squashed in the right place there.
>> 
>> And then it'll hit stable when that's submitted for 6.12. It's an old
>> bug that nobody noticed, and time namespaces are kind of obscure, so I
>> think waiting a week and a half for the merge window to open is probably
>> fine.
>
> So I've just done this (preliminarily, pending Michael's approval), and
> it comes out decently clean and everything works fine.

Sorry, didn't check email over the weekend.

Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)

> The commit
> sequence becomes:
>
> ...
> c206cd11e7f2 selftests: vDSO: ensure vgetrandom works in a time namespace
> ...
> e59cc170924c powerpc/vdso: Fix VDSO data access when running in a non-root time namespace
> 887e7a77dc99 mm: Define VM_DROPPABLE for powerpc/32
> f2ee39ec52c2 powerpc/vdso32: Add crtsavres
> 994148e87080 powerpc/vdso: Refactor CFLAGS for CVDSO build
> c49ec121a6dd powerpc/vdso: Wire up getrandom() vDSO implementation on VDSO32 <-- fixed up
> a8a5e16cde32 powerpc/vdso: Wire up getrandom() vDSO implementation on VDSO64
>
> So I'm happy with this.

Thanks.

cheers