From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: revisit arm64 per-task stack canaries
Date: Tue, 30 Oct 2018 10:35:03 +0000 [thread overview]
Message-ID: <20181030103502.GD19955@arm.com> (raw)
In-Reply-To: <CAKv+Gu_q9mbjHhuU8g_nM6vBS_F3ypAvnuKLAg-Oaf+2wf2ePw@mail.gmail.com>
On Fri, Oct 12, 2018 at 12:37:46PM +0200, Ard Biesheuvel wrote:
> To recap, my proof of concept was based on using the task struct
> pointer directly (which we keep in sp_el0 in the kernel). That way, we
> don't care about per-CPU vars and preemption etc, since the stack
> canary value is tied to the task anyway, regardless on which CPU it
> executes.
>
> So my suggestion was to turn this (the current sequence emitted by the compiler)
>
> adrp x0, __stack__chk_guard
> ldr x0, [x0, :lo12:__stack_chk_guard]
>
> into this
>
> mrs x0, <sysreg>
> ldr x0, [x0, :lo12:<offset>]
>
> where 'sysreg' and 'offset' are the names of 1) a system register and
> 2) an absolute ELF symbol, respectively.
>
> At Ramana's request (and with Arnd's help) I looked into whether we
> can provide offsetof(struct task_struct, stack_canary) as a bare
> number on the GCC command line instead, but this is turning out to be
> rather cumbersome, given that we need to run the compiler to find the
> offset in the first place. (The most promising approach would be to
> generate a file and include it with @filename so its contents are
> parsed as command line options but even that is rather fiddly to add
> into our build system)
>
> My approach of exposing the offset via the linker script works fine,
> and we wouldn't even need a separate GCC command line option in that
> case: we could just settle on __stack_chk_guard_sysreg_offset and
> stipulate that it is exposed as an absolute ELF symbol in the range
> [0..1023] when -mstack-protector-guard=<sysreg> is passed to GCC.
That sounds like a sensible proposal to me. I'm assuming that the sysreg
is specified on the compiler command line as well?
> Ramana, could you elaborate on your preference for a bare number on
> the command line? Do you see any alternatives?
With CONFIG_THREAD_INFO_IN_TASK I suppose we could bodge things so that
this constant is also zero, but it seems to me that all we gain is
fragility when compared to exposing a named symbol.
Ramana -- was this just a cosmetic preference, or are there good technical
reasons to avoid the new symbol (which isn't an awful lot different to what
we currently use).
Will
next prev parent reply other threads:[~2018-10-30 10:35 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-13 12:36 revisit arm64 per-task stack canaries Ard Biesheuvel
2018-02-13 12:52 ` Mark Rutland
2018-02-13 13:56 ` Ard Biesheuvel
2018-02-13 18:04 ` Will Deacon
2018-10-12 10:37 ` Ard Biesheuvel
2018-10-15 21:47 ` Kees Cook
2018-10-30 10:35 ` Will Deacon [this message]
2018-10-30 12:45 ` Ard Biesheuvel
2018-11-19 16:50 ` Ramana Radhakrishnan
2018-11-19 17:04 ` Ard Biesheuvel
2018-11-19 19:47 ` Ard Biesheuvel
2018-12-03 10:16 ` Ramana Radhakrishnan
2018-12-03 16:46 ` Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181030103502.GD19955@arm.com \
--to=will.deacon@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).