Re: [PATCH v5 06/12] gdb, gdbserver: Add support of Intel shadow stack pointer register.

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

Hello Christina,

"Schimpe, Christina" <christina.schimpe@intel.com> writes:

>> > >> > +    # Read PL3_SSP register.
>> > >> > +    set ssp_main [get_hexadecimal_valueof "\$pl3_ssp" "read
>> > >> > + pl3_ssp value"]
>> > >> > +
>> > >> > +    # Write PL3_SSP register.
>> > >> > +    gdb_test "print /x \$pl3_ssp = 0x12345678" "= 0x12345678"
>> > >> > + "set pl3_ssp
>> > >> value"
>> > >> > +    gdb_test "print /x \$pl3_ssp" "= 0x12345678" "read pl3_ssp
>> > >> > + value after
>> > >> setting"
>> > >> > +
>> > >> > +    # Restore original value.
>> > >> > +    gdb_test "print /x \$pl3_ssp = $ssp_main" "= $ssp_main"
>> > >> > + "restore
>> > >> original pl3_ssp"
>> > >> > +
>> > >> > +    # Potential CET violations often only occur after resuming
>> > >> > + normal
>> > >> execution.
>> > >> > +    # Therefore, it is important to test normal program
>> > >> > + continuation
>> > after
>> > >> > +    # configuring the shadow stack pointer.
>> > >> > +    gdb_continue_to_end
>> > >>
>> > >> I assume that if we continue with the bogus value in place the
>> > >> inferior would either give an error or terminate.  Is it worth
>> > >> trying this and checking that the inferior behaves as expected?
>> > >
>> > > If we don't reset the shadow stack pointer to it's original value we
>> > > will see
>> > a SEGV.
>> > > Dependent on the address of the wrong shadow stack pointer it's
>> > > either a SEGV with si code that points to a control flow protection
>> > > fault or a
>> > different si code.
>> > >
>> > > So if I stay in a valid address range for configuring pl3_ssp but
>> > > don't restore the original value I'll see a control flow protection
>> exception:
>> > >
>> > > [...]
>> > > breakpoint 1, 0x0000555555555148 in main ()^M
>> > > (gdb) print /x $pl3_ssp^M
>> > > $1 = 0x7ffff7bfffe8^M
>> > > (gdb) PASS: gdb.arch/amd64-ssp.exp: get hexadecimal valueof
>> "$pl3_ssp"
>> > > print /x $pl3_ssp = 0x7ffff7bfffe0^M
>> > > $2 = 0x7ffff7bfffe0^M
>> > > (gdb) PASS: gdb.arch/amd64-ssp.exp: set pl3_ssp value print /x
>> > > $pl3_ssp^M
>> > > $3 = 0x7ffff7bfffe0^M
>> > > (gdb) PASS: gdb.arch/amd64-ssp.exp: read pl3_ssp value after setting
>> > > continue^M Continuing.^M ^M Program received signal SIGSEGV,
>> > > Segmentation fault.^M
>> > > 0x0000555555555158 in main ()^M
>> > > (gdb) FAIL: gdb.arch/amd64-ssp.exp: continue until exit
>> > >
>> > > Siginfo shows si_code = 10, which indicates a control protection fault.
>> > >
>> > > p $_siginfo^M
>> > > $4 = {si_signo = 11, si_errno = 0, si_code = 10, [...]
>> > >
>> > > If I set the value of pl3_ssp as in the current test (0x12345678)
>> > > I'll see a different SEGV actually
>> > >
>> > > p $_siginfo
>> > > $4 = {si_signo = 11, si_errno = 0, si_code = 1, [...]
>> > >
>> > >>
>> > >> What if, say, the $pl3_ssp value only ever made it as far as the
>> > >> register cache, and was never actually written back to the inferior?
>> > >> I don't think the above test would actually spot this bug, right?
>> > >
>> > > Hm, if I understand you correctly here and you mean the scenario as
>> > > shown above the above test would spot this bug I think (as we saw a
>> fail).
>> > >
>> > > Does my example above show what you described or do you mean a
>> > > different scenario?
>> >
>> > Yes, something like the above would check that the register is
>> > actually being written back to the hardware, and is written to the expected
>> location.
>> >
>> > The current test, as written in the patch, writes a bad value to the
>> > shadow stack, then restores the correct value.  What if the bad value
>> > never actually got written back to the hardware at all, and was just
>> > being held in the register cache?
>> >
>> > Having a test that writes a bad value, then does 'continue', and
>> > expects to see something like 'Program received signal ...' would be a
>> > reasonable indication that the write to the shadow stack actually made it
>> to the h/w.
>> >
>> > Thanks,
>> > Andrew
>> 
>> 
>> Yes, I agree, I'll add:
>> 
>> ~~~
>>     with_test_prefix "invalid ssp" {
>> 	write_invalid_ssp
>> 
>> 	# Continue until SIGSEV to test that the value is written back to HW.
>> 	gdb_test "continue" \
>> 	    [multi_line \
>> 		"Continuing\\." \
>> 		"" \
>> 		"Program received signal SIGSEGV, Segmentation fault\\." \
>> 		"$hex in main \\(\\)"] \
>> 	    "continue to SIGSEGV"
>>     }
>> 
>>     clean_restart ${binfile}
>>     if { ![runto_main] } {
>> 	return -1
>>     }
>> 
>>     with_test_prefix "restore original ssp" {
>> 	# Read PL3_SSP register.
>> 	set ssp_main [get_hexadecimal_valueof "\$pl3_ssp" "read pl3_ssp
>> value"]
>> 
>> 	write_invalid_ssp
>> 
>> 	# Restore original value.
>> 	gdb_test "print /x \$pl3_ssp = $ssp_main" "= $ssp_main" "restore
>> original value"
>> 
>> 	# Now we should not see a SIGSEV, since the original value is
>> restored.
>> 	gdb_continue_to_end
>>     }
>> 
>> ~~~
>> 
>> Regards,
>> Christina
>
> Do you have a test for actual write back to HW (as above). If not, it
> might make sense to add it also for GCS?

I don't have a test for it, but I agree it's worth adding one. Will
do. Thanks for pointing out.

-- 
Thiago