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

["Jason A. Donenfeld" <Jason@zx2c4.com>]

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. 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.

Jason