Re: [RFC PATCH v2 2/2] target/ppc: make gdb able to translate priviledged addresses

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3115 bytes --]

On Mon, Jun 14, 2021 at 09:37:54PM +0200, Philippe Mathieu-Daudé wrote:
> On 6/14/21 9:16 PM, Bruno Larsen (billionai) wrote:
> > This patch changes ppc_cpu_get_phys_page_debug so that it is now
> > able to translate both, priviledged and real mode addresses
> > independently of whether the CPU executing it has those permissions
> > 
> > This was mentioned by Fabiano as something that would be very useful to
> > help with debugging, but could possibly constitute a security issue if
> > that debug function can be called in some way by prodution code. the
> > solution was implemented such that it would be trivial to wrap it around
> > ifdefs for building only with --enable-debug, for instance, but we are
> > not sure this is the best approach, hence why it is an RFC.
> > 
> > Suggested-by: Fabiano Rosas <farosas@linux.ibm.com>
> > Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
> > ---
> >  target/ppc/mmu_helper.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> > index 9dcdf88597..41c727c690 100644
> > --- a/target/ppc/mmu_helper.c
> > +++ b/target/ppc/mmu_helper.c
> > @@ -2947,6 +2947,29 @@ hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> >                    cpu_mmu_index(&cpu->env, true), false)) {
> >          return raddr & TARGET_PAGE_MASK;
> >      }
> > +
> > +    /*
> > +     * This is a fallback, in case we're asking for priviledged memory to
> > +     * be printed, but the PCU is not executing in a priviledged
> > manner.

s/PCU/CPU/

> > +     *
> > +     * The code could be considered a security vulnerability if
> > +     * this function can be called in a scenario that does not involve
> > +     * debugging.
> > +     * Given the name and how useful using real addresses may be for
> > +     * actually debugging, however, we decided to include it anyway and
> > +     * discuss how to best avoid the possible security concerns.
> > +     * The current plan is that, if there is a chance this code is called in
> > +     * a production environment, we can surround it with ifdefs so that it
> > +     * is only compiled with --enable-debug
> 
> Nothing forbid us to use --enable-debug in a production environment.
> 
> > +     */
> > +        /* attempt to translate first with virtual addresses */
> > +    if (ppc_xlate(cpu, addr, MMU_DATA_LOAD, &raddr, &s, &p, 1, false) ||
> > +        ppc_xlate(cpu, addr, MMU_INST_FETCH, &raddr, &s, &p, 1, false) ||
> > +        /* if didn't work, attempt to translate with real addresses */
> > +        ppc_xlate(cpu, addr, MMU_DATA_LOAD, &raddr, &s, &p, 3, false) ||
> > +        ppc_xlate(cpu, addr, MMU_INST_FETCH, &raddr, &s, &p, 3, false)) {
> > +        return raddr & TARGET_PAGE_MASK;
> > +    }
> >      return -1;
> >  }
> >  
> > 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]