Re: [PATCH RFC] x86: KASAN: Sanitize unauthorized irq stack access

[Dave Hansen]

On 02/07/2018 08:14 AM, Kirill Tkhai wrote:
> Sometimes it is possible to meet a situation,
> when irq stack is corrupted, while innocent
> callback function is being executed. This may
> happen because of crappy drivers irq handlers,
> when they access wrong memory on the irq stack.

Can you be more clear about the actual issue?  Which drivers do this?
How do they even find an IRQ stack pointer?

> This patch aims to catch such the situations
> and adds checks of unauthorized stack access.

I think I forgot how KASAN did this.  KASAN has metadata that says which
areas of memory are good or bad to access, right?  So, this just tags
IRQ stacks as bad when we are not _in_ an interrupt?

> +#define KASAN_IRQ_STACK_SIZE \
> +	(sizeof(union irq_stack_union) - \
> +		(offsetof(union irq_stack_union, stack_canary) + 8))

Just curious, but why leave out the canary?  It shouldn't be accessed
either.

> +#ifdef CONFIG_KASAN
> +void __visible x86_poison_irq_stack(void)
> +{
> +	if (this_cpu_read(irq_count) == -1)
> +		kasan_poison_irq_stack();
> +}
> +void __visible x86_unpoison_irq_stack(void)
> +{
> +	if (this_cpu_read(irq_count) == -1)
> +		kasan_unpoison_irq_stack();
> +}
> +#endif

It might be handy to point out here that -1 means "not in an interrupt"
and >=0 means "in an interrupt".

Otherwise, this looks pretty straightforward.  Would it be something to
extend to the other stacks like the NMI or double-fault stacks?  Or are
those just not worth it?