Page MenuHomeFreeBSD

udpate stack guard bytes inside __guard_setup()
ClosedPublic

Authored by lffpires_ruabrasil.org on Apr 23 2018, 5:40 PM.

Details

Summary

This is necessary to make sure that functions that can have stack protection are not used to update the stack guard.
In that case, the stack guard check would fail when it shouldn't.

guard_setup() calls elf_aux_info(), which, in turn, calls memcpy() to update stack_chk_guard. If either elf_aux_info() or memcpy() have stack protection enabled, __stack_chk_guard will be modified before returning from them, causing the stack protection check to fail.

This change uses a temporary buffer to delay changing __stack_chk_guard until elf_aux_info() returns.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

lffpires_ruabrasil.org retitled this revision from udpate stack guard bytes inside __guard_setup This is necessary to make sure that functions that can have stack protection are not used to update the stack guard. In that case, the stack guard check would fail when it shouldn't. to udpate stack guard bytes inside __guard_setup().Apr 23 2018, 5:50 PM
lffpires_ruabrasil.org edited the summary of this revision. (Show Details)
lffpires_ruabrasil.org added inline comments.
lib/libc/secure/stack_protector.c
68 ↗(On Diff #41773)

I guess I should probably zero out tmp_stack_chk_guard[idx] after copying to __stack_chk_guard, to avoid leaving it on the stack longer than necessary.

lib/libc/secure/stack_protector.c
59 ↗(On Diff #41773)

You can spell it as nitems(__stack_chk_guard)).

66 ↗(On Diff #41773)

Put the variable declaration into the locals block at the start of the function body.

68 ↗(On Diff #41773)

I believe modern compilers can detect the copy and optimize the loop into memcpy() call. To fight this, you can use e.g. volatile specifier for tmp_stack_chk_guard.

Why did you added PPC guys to the review ? Does this problem appeared on Power ? Do nathanw/jhibbits want to commit this ?

lib/libc/secure/stack_protector.c
60 ↗(On Diff #41796)

Please move idx into the same declaration as error.

This revision is now accepted and ready to land.Apr 24 2018, 1:02 PM

@kib, I added them just because this showed up while working on an optimization for powerpc64, and at the time I didn't know who I should add as reviewer. Please feel free to commit this.

This revision was automatically updated to reflect the committed changes.