Re: [RFC][PATCH 1/2] x86: Allow breakpoints to emulate call functions

From: Andy Lutomirski
Date: Fri May 03 2019 - 18:55:48 EST




> On May 3, 2019, at 2:46 PM, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
>> On Fri, May 3, 2019 at 12:24 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>>
>> The problem with this approach is that it would require doing the same
>> for x86_64, as the int3 C code is the same for both. And that may be a
>> bit more difficult on the x86_64 side because it's all done with a
>> simple flag in the idtentry macro to add the gap.
>
> That argument is weakened by the fact that we have to do _other_
> things differently on 32-bit and 64-bit anyway.
>
> So we might as well have a "on 32-bit, the call emulation needs to
> move the pt_regs to make space" special case in the call emulation
> code. It's very easy to explain why.
>
> And then we'd limit the special case to where it matters (with a big
> comment about what's going on), rather than adding random special case
> handling to random _other_ places.

If we do this, it should IMO look like this:

struct pt_regs *change_kernel_stack_pointer(struct pt_regs *, unsigned long new_sp);

And that helper should be used on both variants.

But I think this will end up worse than the version where the entry code fixes it up. This is because, if the C code moves pt_regs, then we need some way to pass the new pointer back to the asm. We will also have a much harder time with runtime sanity checks. In the model where the C code merely updates regs->sp, itâs very easy to compare sp and &regs to check for overlap, but itâs much harder to tell whether memmoveing it is going to overwrite something important. And we have to worry about whether thereâs some other code that assumes that pt_regs stays put.

So my intuition is that the pure asm fixup will result is more maintainable code.