linux-kernel - Re: [PATCH v2] 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: <Y026l2PZgvt+G6p0@smile.fi.intel.com>
Date:   Mon, 17 Oct 2022 23:27:03 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Jane Chu <jane.chu@...cle.com>
Cc:     pmladek@...e.com, rostedt@...dmis.org, senozhatsky@...omium.org,
        linux@...musvillemoes.dk, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] vsprintf: protect kernel from panic due to
 non-canonical pointer dereference

On Mon, Oct 17, 2022 at 01:44:47PM -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

non-canical?

> 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 run one more round of check via kern_addr_valid(). On architectures
> that provide meaningful implementation, this line of check effectively
> catches non-canonical pointers, etc.

OK, but I don't see how this is useful in the form of returning efault here.
Ideally we should inform user that the pointer is wrong and how it's wrong.
But. It will crash somewhere else at some point, right? I mean that there
is no guarantee that kernel has protection in every single place against
dangling / invalid pointers. One way or another it will crash.

That said, honestly I have no idea how this patch may be considered
anything but band-aid. OTOH, I don't see a harm. Perhaps others will
share their opinions.

-- 
With Best Regards,
Andy Shevchenko