Page MenuHomeFreeBSD

arm64: Disable per-thread stack-smashing protection in data_abort()
ClosedPublic

Authored by markj on Nov 3 2022, 7:00 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 7, 1:58 PM
Unknown Object (File)
Mon, Nov 25, 4:01 PM
Unknown Object (File)
Fri, Nov 15, 9:20 PM
Unknown Object (File)
Nov 7 2024, 4:16 PM
Unknown Object (File)
Oct 13 2024, 1:23 PM
Unknown Object (File)
Oct 7 2024, 5:59 PM
Unknown Object (File)
Oct 4 2024, 4:14 PM
Unknown Object (File)
Oct 4 2024, 3:01 PM
Subscribers

Details

Summary

With PERTHREAD_SSP configured, SSP uses a per-thread canary value
instead of a global value. The value is stored in td->td_md.md_canary;
the sp_el0 register always contains a pointer to that value, and certain
functions selected by the compiler will store the canary value on the
stack as a part of the function prologue (and will verify the copy as
part of the epilogue). In particular, the thread structure may be
accessed.

This happens to occur in data_abort(), which leads to the same problem
addressed by commit 2c10be9e06d4 ("arm64: Handle translation faults for
thread structures"). This patch fixes that directly, by disabling SSP
in data_abort() and a couple of related functions by using a function
attribute. It also moves the update of sp_el0 out of C code in case
the compiler decides to start checking the canary in pmap_switch()
someday.

A different solution might be to move the canary value to the PCB, which
currently lives on the kernel stack and isn't subject to the same
problem as thread structures. However, there isn't any particular
reason the PCB has to live on the stack today; on amd64 it is embedded
in struct thread, reintroducing the same problem. Keeping the reference
canary value at the top of the stack also seems a bit dubious since it
could be clobbered by a sufficiently large stack overflow.

A third solution could be to go back to the approach of commit
5aa5420ff2e8, and modify UMA to use the direct map for thread structures
even if KASAN is enabled. But transient promotions and demotions in the
direct map are possible too.

Since all of these options have shortcomings, I went with the most
straightforward one.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 48152
Build 45039: arc lint + arc unit

Event Timeline

markj requested review of this revision.Nov 3 2022, 7:00 PM
sys/sys/cdefs.h
902

Why does this need _KERNEL protection?

markj marked an inline comment as done.Nov 4 2022, 4:12 PM
markj added inline comments.
sys/sys/cdefs.h
902

No real reason.

Note, gcc <= 10 does not have this attribute.

markj marked an inline comment as done.

Define __nostackprotector for !_KERNEL.

sys/arm64/include/param.h
113

This is missing the #else case.

sys/sys/cdefs.h
902

It looks like the gcc <= 10 spelling is __attribute__ ((__optimize__ ("-fno-stack-protector")))

Define NO_PERTHREAD_SSP in all cases.

Provide a fallback definition for __nostackprotect which works with older gcc.

This revision is now accepted and ready to land.Nov 4 2022, 5:22 PM