lkml.org 
[lkml]   [2022]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2] vsprintf: protect kernel from panic due to non-canonical pointer dereference
Date
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

\
 
 \ /
  Last update: 2022-10-19 22:03    [W:0.063 / U:1.716 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site