From: alex.popov@linux•com (Alexander Popov)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 2/2] arm64: Clear the stack
Date: Fri, 11 May 2018 18:50:09 +0300 [thread overview]
Message-ID: <71199506-b46b-5f91-e489-e6450b6d1067@linux.com> (raw)
In-Reply-To: <4badae50-be9b-2c6d-854b-57ab48664800@linux.com>
Hello everyone,
On 06.05.2018 11:22, Alexander Popov wrote:
> On 04.05.2018 14:09, Mark Rutland wrote:
>>>>> + stack_left = sp & (THREAD_SIZE - 1);
>>>>> + BUG_ON(stack_left < 256 || size >= stack_left - 256);
>>>>
>>>> Is this arbitrary, or is there something special about 256?
>>>>
>>>> Even if this is arbitrary, can we give it some mnemonic?
>>>
>>> It's just a reasonable number. We can introduce a macro for it.
>>
>> I'm just not sure I see the point in the offset, given things like
>> VMAP_STACK exist. BUG_ON() handling will likely require *more* than 256
>> bytes of stack, so it seems superfluous, as we'd be relying on stack
>> overflow detection at that point.
>>
>> I can see that we should take the CONFIG_SCHED_STACK_END_CHECK offset
>> into account, though.
>
> Mark, thank you for such an important remark!
>
> In Kconfig STACKLEAK implies but doesn't depend on VMAP_STACK. In fact x86_32
> doesn't have VMAP_STACK at all but can have STACKLEAK.
>
> [Adding Andy Lutomirski]
>
> I've made some additional experiments: I exhaust the thread stack to have only
> (MIN_STACK_LEFT - 1) bytes left and then force alloca. If VMAP_STACK is
> disabled, BUG_ON() handling causes stack depth overflow, which is detected by
> SCHED_STACK_END_CHECK. If VMAP_STACK is enabled, the kernel hangs on BUG_ON()
> handling! Enabling CONFIG_PROVE_LOCKING gives the needed report from VMAP_STACK:
[...]
> I can't say why VMAP_STACK report hangs during BUG_ON() handling on defconfig.
> Andy, can you give a clue?
>
> I see that MIN_STACK_LEFT = 2048 is enough for BUG_ON() handling on both x86_64
> and x86_32. So I'm going to:
> - set MIN_STACK_LEFT to 2048;
> - improve the lkdtm test to cover this case.
>
> Mark, Kees, Laura, does it sound good?
Could you have a look at the following changes in check_alloca() before I send
the next version?
If VMAP_STACK is enabled and alloca causes stack depth overflow, I write to
guard page below the thread stack to cause double fault and VMAP_STACK report.
If VMAP_STACK is disabled, I use MIN_STACK_LEFT = 2048, which seems to be enough
for BUG_ON() handling both on x86_32 and x86_64. Unfortunately, I can't
guarantee that it is always enough.
#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
-#define MIN_STACK_LEFT 256
+#define MIN_STACK_LEFT 2048
void __used check_alloca(unsigned long size)
{
unsigned long sp = (unsigned long)&sp;
struct stack_info stack_info = {0};
unsigned long visit_mask = 0;
unsigned long stack_left;
BUG_ON(get_stack_info(&sp, current, &stack_info, &visit_mask));
stack_left = sp - (unsigned long)stack_info.begin;
+
+#ifdef CONFIG_VMAP_STACK
+ /*
+ * If alloca oversteps the thread stack boundary, we touch the guard
+ * page provided by VMAP_STACK to trigger handle_stack_overflow().
+ */
+ if (size >= stack_left)
+ *(stack_info.begin - 1) = 42;
+#else
BUG_ON(stack_left < MIN_STACK_LEFT ||
size >= stack_left - MIN_STACK_LEFT);
+#endif
}
EXPORT_SYMBOL(check_alloca);
#endif
Looking forward to your feedback.
Best regards,
Alexander
next prev parent reply other threads:[~2018-05-11 15:50 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1523024546-6150-1-git-send-email-alex.popov@linux.com>
2018-05-02 20:33 ` [PATCH 0/2] Stackleak for arm64 Laura Abbott
2018-05-02 20:33 ` [PATCH 1/2] stackleak: Update " Laura Abbott
2018-05-02 20:33 ` [PATCH 2/2] arm64: Clear the stack Laura Abbott
2018-05-02 21:31 ` Kees Cook
2018-05-02 23:07 ` Laura Abbott
2018-05-02 23:37 ` Kees Cook
2018-05-03 16:05 ` Alexander Popov
2018-05-03 16:45 ` Kees Cook
2018-05-03 7:19 ` Mark Rutland
2018-05-03 11:37 ` Ard Biesheuvel
2018-05-03 17:33 ` Alexander Popov
2018-05-03 19:09 ` Laura Abbott
2018-05-04 8:30 ` Alexander Popov
2018-05-04 11:09 ` Mark Rutland
2018-05-06 8:22 ` Alexander Popov
2018-05-11 15:50 ` Alexander Popov [this message]
2018-05-11 16:13 ` Mark Rutland
2018-05-13 8:40 ` Alexander Popov
2018-05-14 5:15 ` Mark Rutland
2018-05-14 9:35 ` Alexander Popov
2018-05-14 10:06 ` Mark Rutland
2018-05-14 13:53 ` Alexander Popov
2018-05-14 14:07 ` Mark Rutland
2018-05-03 19:00 ` Laura Abbott
2018-05-04 11:16 ` Mark Rutland
2018-07-18 21:10 [PATCH 0/2] Stackleak for arm64 Laura Abbott
2018-07-18 21:10 ` [PATCH 2/2] arm64: Clear the stack Laura Abbott
2018-07-19 2:20 ` Kees Cook
2018-07-19 10:41 ` Alexander Popov
2018-07-19 11:41 ` Mark Rutland
-- strict thread matches above, loose matches on Subject: below --
2018-02-21 1:13 [PATCH 0/2] Stackleak for arm64 Laura Abbott
2018-02-21 1:13 ` [PATCH 2/2] arm64: Clear the stack Laura Abbott
2018-02-21 15:38 ` Mark Rutland
2018-02-21 23:53 ` Laura Abbott
2018-02-22 1:35 ` Laura Abbott
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=71199506-b46b-5f91-e489-e6450b6d1067@linux.com \
--to=alex.popov@linux$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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