Re: [PATCH] x86/ftrace: Don't bother preserving/restoring R10/R11

[Ard Biesheuvel]

On Wed, 9 Oct 2024 at 18:19, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> On Wed, 9 Oct 2024 18:13:54 +0200
> Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
>
> > > > @@ -256,7 +254,6 @@ SYM_INNER_LABEL(ftrace_regs_call, SYM_L_GLOBAL)
> > > >       movq R14(%rsp), %r14
> > > >       movq R13(%rsp), %r13
> > > >       movq R12(%rsp), %r12
> > > > -     movq R10(%rsp), %r10
> > >
> > > This part of the patch I think is perfectly fine. We haven't been restoring
> > > R11 for 12 years I'm sure nobody will notice if we don't restore R10.
> > >
> >
> > Not sure I follow this reasoning tbh. R10/R11 are guaranteed to be
> > dead, so I don't see any point in preserving them. But if you do
> > capture them, shouldn't you at least ensure that the captured values
> > match the values that the callee will observe? (even though they are
> > dead and we know it will never look at them)
>
> Say we have code of:
>
>         pushq   r10
>         pushq   r11
>         call    foo
>         popq    r11
>         popq    r10
>
> Where we add a kprobe to the start of foo, the callback should be able to
> see what r10 and r11 were.

Why exactly is that? The contents of R10 and R11 have no purpose going
forward, so is it just to see what some previous code may have left in
them?

> But the restore part is for the function foo to
> see. It shouldn't care about r10 or r11 and if a kprobe updates them, it
> should not have any effect.
>
> What does restoring r10 and r11 give us?
>

Nothing. Which is why I don't understand why you would need to record
them in the first place.