Messages in this thread | | | From | Jane Chu <> | Subject | Re: [PATCH v2] vsprintf: protect kernel from panic due to non-canonical pointer dereference | Date | Wed, 19 Oct 2022 20:02:36 +0000 |
| |
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
| |