Re: [RFC PATCH] target/i386: allow access_ptr to force slow path on failed probe

[Richard Henderson]

On 8/13/24 23:12, Alex Bennée wrote:
> Richard Henderson <richard.henderson@linaro.org> writes:
>
> > On 8/8/24 02:02, Alex Bennée wrote:
> > > When we are using TCG plugin memory callbacks probe_access_internal
> > > will return TLB_MMIO to force the slow path for memory access. This
> > > results in probe_access returning NULL but the x86 access_ptr function
> > > happily accepts an empty haddr resulting in segfault hilarity.
> > > Check for an empty haddr to prevent the segfault and enable plugins
> > > to
> > > track all the memory operations for the x86 save/restore helpers.
> > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2489
> > > Fixes: 6d03226b42 (plugins: force slow path when plugins instrument memory ops)
> > > ---
> > >    target/i386/tcg/access.c | 4 ++++
> > >    1 file changed, 4 insertions(+)
> > > diff --git a/target/i386/tcg/access.c b/target/i386/tcg/access.c
> > > index 56a1181ea5..8ea5c453a0 100644
> > > --- a/target/i386/tcg/access.c
> > > +++ b/target/i386/tcg/access.c
> > > @@ -58,6 +58,10 @@ static void *access_ptr(X86Access *ac, vaddr addr, unsigned 
> > > len)
> > >          assert(addr >= ac->vaddr);
> > >    +    if (!ac->haddr1) {
> > > +        return NULL;
> > > +    }
> > > +
> > >    #ifdef CONFIG_USER_ONLY
> > >        assert(offset <= ac->size1 - len);
> > >        return ac->haddr1 + offset;
> >
> > You need to remove the test_ptr macro below as well.
>
> So we fall-back to the slow path for linux-user as well?

So we don't ignore the null, and dereference it anyway.

r~