Re: [PATCH] vsprintf: protect kernel from panic due to non-canonical pointer dereference

From: Jane Chu <jane.chu@oracle.com>
To: Randy Dunlap <rdunlap@infradead.org>, Petr Mladek <pmladek@suse.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	"rostedt@goodmis.org" <rostedt@goodmis.org>,
	"senozhatsky@chromium.org" <senozhatsky@chromium.org>,
	"linux@rasmusvillemoes.dk" <linux@rasmusvillemoes.dk>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	Haakon Bugge <haakon.bugge@oracle.com>,
	John Haxby <john.haxby@oracle.com>,
	Jane Chu <jane.chu@oracle.com>
Subject: Re: [PATCH] vsprintf: protect kernel from panic due to non-canonical pointer dereference
Date: Tue, 18 Oct 2022 19:58:25 +0000	[thread overview]
Message-ID: <c0c0d440-8a2b-c1b1-83b6-278f4bdaf233@oracle.com> (raw)
In-Reply-To: <46b4bf12-f501-0447-8c3c-8dd0cb91ba92@infradead.org>

On 10/18/2022 12:28 PM, Randy Dunlap wrote:
> Hi--
> 
[..]
>>>> 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.
>>
>> Will do.
>>
>>> 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.
>>
>> On x86 architecture, kern_addr_valid() looks safe to me though, on
>> several other architectures, it's defined (1).
> 
> You might want to compare this patch, which seems to have some support:
> 
> https://lore.kernel.org/lkml/20221018074014.185687-1-wangkefeng.wang@huawei.com/
> 

Thank you for alerting me, appreciated!

The patch comment says "copy_from_kernel_nofault() which could check 
whether the address is a valid kernel address, so no need 
kern_addr_valid()", I'm afraid copy_from_kernel_nofault() is more of a 
heavy hammer, and less appropriate for this patch.  I'll take a closer 
look before responding to the submitter.

thanks!
-jane