Messages in this thread | | | Date | Tue, 18 Oct 2022 14:45:20 +0200 | From | Petr Mladek <> | Subject | Re: [PATCH] vsprintf: protect kernel from panic due to non-canonical pointer dereference |
| |
On Mon 2022-10-17 19:31:53, Jane Chu wrote: > On 10/17/2022 12:25 PM, Andy Shevchenko wrote: > > On Mon, Oct 17, 2022 at 01:16:11PM -0600, Jane Chu wrote: > >> While debugging a separate issue, it was found that an invalid string > >> pointer could very well contain a non-canical address, such as > >> 0x7665645f63616465. In that case, this line of defense isn't enough > >> to protect the kernel from crashing due to general protection fault > >> > >> if ((unsigned long)ptr < PAGE_SIZE || IS_ERR_VALUE(ptr)) > >> return "(efault)"; > >> > >> So instead, use kern_addr_valid() to validate the string pointer. > > > > How did you check that value of the (invalid string) pointer? > > > > In the bug scenario, the invalid string pointer was an out-of-bound > string pointer. While the OOB referencing is fixed,
Could you please provide more details about the fixed OOB? What exact vsprintf()/printk() call was broken and eventually how it was fixed, please?
> the lingering issue > is that the kernel ought to be able to protect itself, as the pointer > contains a non-canonical address.
Was the pointer used only by the vsprintf()? Or was it accessed also by another code, please?
I wonder if this patch would prevent the crash or if the broken kernel would crash later anyway.
> That said, I realized that not all > architecture implement meaningful kern_addr_valid(), so this line > if ((unsigned long)ptr < PAGE_SIZE || IS_ERR_VALUE(ptr)) > is still need. I'll send v2.
Please, add linux-mm@kvack.org into CC. I wonder if kern_addr_valid() is safe to use anywhere, especially during early boot. I wonder if it would make sense to implement it on all architectures.
Best Regards, Petr
| |