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: <860872bd-127f-36ee-f803-6553a6f03826@oracle.com>
Date:   Wed, 19 Oct 2022 20:02:36 +0000
From:   Jane Chu <jane.chu@...cle.com>
To:     Petr Mladek <pmladek@...e.com>
CC:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>,
        "senozhatsky@...omium.org" <senozhatsky@...omium.org>,
        "linux@...musvillemoes.dk" <linux@...musvillemoes.dk>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        Haakon Bugge <haakon.bugge@...cle.com>,
        John Haxby <john.haxby@...cle.com>,
        Jane Chu <jane.chu@...cle.com>
Subject: Re: [PATCH v2] vsprintf: protect kernel from panic due to
 non-canonical pointer dereference

Hi, Petr,

Sorry, I didn't catch this email prior to sending out v3.

[..]
>>
>> Yes, kern_addr_valid() is used by read_kcore() which is architecturally
>> independent and applies everywhere, so does that imply that it is
>> defined in all architectures?
> 
> It is more complicated. fs/proc/kcore.c is built when
> CONFIG_PROC_KCORE is set. It is defined in fs/proc/Kconfig as:
> 
> config PROC_KCORE
> 	bool "/proc/kcore support" if !ARM
> 	depends on PROC_FS && MMU
> 
> So, it is not built on ARM.

Indeed, it's defined on ARM though.

> 
> More importantly, kern_addr_valid() seems to be implemented only for x86_64.
> It is always true (1) on all other architectures, see
> 
> $> git grep kern_addr_valid
> arch/alpha/include/asm/pgtable.h:#define kern_addr_valid(addr)  (1)
> arch/arc/include/asm/pgtable-bits-arcv2.h:#define kern_addr_valid(addr) (1)
> arch/arm/include/asm/pgtable-nommu.h:#define kern_addr_valid(addr)      (1)
> arch/arm/include/asm/pgtable.h:#define kern_addr_valid(addr)    (1)
> [...]
> 
> Wait, it is actually always false (0) on x86 when SPARSEMEM is used,
> see arch/x86/include/asm/pgtable_32.h:
> 
> #ifdef CONFIG_FLATMEM
> #define kern_addr_valid(addr)	(1)
> #else
> #define kern_addr_valid(kaddr)	(0)
> #endif
> 

Thanks for pointing this out.  Let me do some digging ...

> 
>> I guess the early boot scenario is different in that, potentially unkind
>> users aren't involved, hence a broken kernel is broken and need a fix.
> 
> The important thing is that kern_addr_valid() must return valid
> results even during early boot. Otherwise, vsprintf() would not work
> during the early boot which is not expected.

Yes, agreed.

> 
>> The scenario concerned here is with users could potentially exploit a
>> kernel issue with DOS attack.  Then we have the scenario that the kernel
>> bug itself is confined, in that, had the sysfs not been accessed, the
>> OOB pointer won't be produced.  So in this case, "(efault)" is a lot
>> more desirable than panic.
> 
> Please, provide more details about the bug when invalid pointer was
> passed. As Andy wrote, even if we catch the bad pointer in vsprintf(),
> the kernel would most likely kernel crash anyway.

Hopefully the comment in v3 clarifies the bug, please let me know.

thanks!
-jane


> 
> Best Regards,
> Petr