Re: [PATCH v2 0/9] Add new command to print the shadow stack backtrace

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Hello again,

There's always something more after you hit "send"...

Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes:

> I had a look at the review threads for v1 and v2 of this patch series,
> and I think the discussion below cover the loose ends that were still
> unaddressed.

Of course, thank you for having summarized the loose ends. It was very
helpful.
 
> Please let me know if I am missing anything, or if there
> anything else you want to bring up before posting v3.

It turns out that there was this matter from patch 7 in the v1 thread¹:

----
>> My only comment is that there a also the "set backtrace past-main" and "set
>> backtrace past-entry" options, which "bt shadow" should also respect.  So if
>> the former setting is on, the output above should stop at frame #3.
>
> Ah those settings, ... I tried to add past-main/past-entry but stopped at some point,
> since it turned out to be complicated - but I don't remember the exact reasons.
> Do you think I missed something and adding those settings should be straight-forward?

I don't actually know. I would try to do what inside_main_func and
inside_entry_func in gdb/frame.c do, but if that is what you tried and
it didn't work or was too complicated then I don't have any ideas...

inside_entry_func in particular seems tricky, since it uses
get_frame_func which at least from a first look does appear to be
complicated.

> I then decided to first post the basic functionality of "bt shadow" without support
> for past-main/past-entry. Do you think this is a problem ?

I don't think it's a problem.
----

And there was also this part from your latest reply in the patch 6
thread that I forgot to paste and reply to:

----
> In case the normal backtrace is broken due for instance a programming error
> such as a buffer overflow the user can use "bt -shadow" as alternative.  In that case, 
> implementing frame arguments would probably be helpful, too. But if we do this
> afterwards, we'll change the default behavior of "bt -shadow". On the other hand,
> this series is already big enough...
> And, if we go that direction, I wonder if we shouldn't somehow construct frame #0
> for the shadow stack. But as described above, I've also some concerns for that.
>
> What do you think?

I don't think changing the default behaviour of "bt -shadow" between
printing and not printing arguments would be a problem, because the
output is meant for human consumption.

With the MI output it could be a problem? I'm not very familiar with MI
to have an informed opinion though.

Regarding printing or not frame 0, in the case where the normal
backtrace is broken due to stack corruption, can it happen that GDB
doesn't print frame 0? I think it will because frame 0 is just the
regcache, so doesn't depend on the regular stack. But I haven't checked
how GDB actually behaves in this scenario.

But IMHO this is not a good reason to construct frame 0 in the shadow
stack backtrace. :)
----

-- 
Thiago

¹ https://inbox.sourceware.org/gdb-patches/SN7PR11MB7638197A9C59562378CA63D9F9C9A@SN7PR11MB7638.namprd11.prod.outlook.com/