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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y1FxS30zVENd/1Ap@smile.fi.intel.com>
Date:   Thu, 20 Oct 2022 19:03:23 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Cc:     Jane Chu <jane.chu@...cle.com>, pmladek@...e.com,
        rostedt@...dmis.org, senozhatsky@...omium.org,
        linux@...musvillemoes.dk, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, wangkefeng.wang@...wei.com,
        haakon.bugge@...cle.com, john.haxby@...cle.com
Subject: Re: [PATCH v3 1/1] vsprintf: protect kernel from panic due to
 non-canonical pointer dereference

On Thu, Oct 20, 2022 at 10:52:03AM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Oct 19, 2022 at 11:33:47PM +0300, Andy Shevchenko wrote:
> > On Wed, Oct 19, 2022 at 01:41:59PM -0600, Jane Chu wrote:
> > > Having stepped on a local kernel bug where reading sysfs has led to
> > > out-of-bound pointer dereference by vsprintf() which led to GPF panic.
> > > And the reason for GPF is that the OOB pointer was turned to a
> > > non-canonical address such as 0x7665645f63616465.
> > > 
> > > vsprintf() already has this line of defense
> > > 	if ((unsigned long)ptr < PAGE_SIZE || IS_ERR_VALUE(ptr))
> > >                 return "(efault)";
> > > Since a non-canonical pointer can be detected by kern_addr_valid()
> > > on architectures that present VM holes as well as meaningful
> > > implementation of kern_addr_valid() that detects the non-canonical
> > > addresses, this patch adds a check on non-canonical string pointer by
> > > kern_addr_valid() and "(efault)" to alert user that something
> > > is wrong instead of unecessarily panic the server.
> > > 
> > > On the other hand, if the non-canonical string pointer is dereferenced
> > > else where in the kernel, by virtue of being non-canonical, a crash
> > > is expected to be immediate.
> > 
> > What if there is no other dereference except the one happened in printf()?
> > 
> > Just to point out here, that I formally NAKed this on the basis that NULL
> > and error pointers are special, for the bogus pointers we need crash ASAP,
> > no matter what the code issues it. I.o.w. printf() is not special for that
> > kind of pointers (i.e. bogus pointers, but not special).
> 
> Hey Andy,
> 
> Do we want to have user space programs crash the kernel?
> 
> This patch leads to making the kernel more harden so that we do
> not crash when there are bugs but continue on.

Fine, how to push a user to report a bug in the kernel if for them
there is no bug?

OK, let's assume user recognizes this as a bug, what should they do in order
to provide a better description of the bug, so developer can easily debug
and fix it?

> Would we not want that experience for users ?

Yes, if it is a bug in the kernel we want to know it with all possible details.
Hiding bugs is a way to nowhere.

-- 
With Best Regards,
Andy Shevchenko