linux-kernel - Re: [PATCH] ARM: mm: Facilitate debugging CONFIG_KUSER

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]

Date:   Fri, 26 Oct 2018 00:36:27 +0530
From:   Souptick Joarder <jrdr.linux@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     linux-arm-kernel@...ts.infradead.org, Andrew Lunn <andrew@...n.ch>,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        ebiederm@...ssion.com, Tony Lindgren <tony@...mide.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ARM: mm: Facilitate debugging CONFIG_KUSER_HELPERS disabled

On Fri, Oct 26, 2018 at 12:18 AM Florian Fainelli <f.fainelli@...il.com> wrote:
>
> On 10/25/18 11:30 AM, Souptick Joarder wrote:
> > On Thu, Oct 25, 2018 at 11:40 PM Florian Fainelli <f.fainelli@...il.com> wrote:
> >>
> >> Some software such as perf makes unconditional use of the special
> >> [vectors] page which is only provided when CONFIG_KUSER_HELPERS is
> >> enabled in the kernel.
> >>
> >> Facilitate the debugging of such situations by printing a debug message
> >> to the kernel log showing the task name and the faulting address.
> >>
> >> Suggested-by: Russell King <rmk+kernel@...linux.org.uk>
> >> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
> >> ---
> >>  arch/arm/mm/fault.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> >> index f4ea4c62c613..f17471fbc1c4 100644
> >> --- a/arch/arm/mm/fault.c
> >> +++ b/arch/arm/mm/fault.c
> >> @@ -173,6 +173,11 @@ __do_user_fault(struct task_struct *tsk, unsigned long addr,
> >>                 show_regs(regs);
> >>         }
> >>  #endif
> >> +#ifndef CONFIG_KUSER_HELPERS
> >
> > Just have one doubt, if the condition is "#ifdef CONFIG_KUSER_HELPER"
> > as commit message suggests the scenario is valid when CONFIG_KUSER_HELPER
> > is enabled ? No ?
>
> #ifndef CONFIG_KUSER_HELPERS is what is intended here, when that option
> is not enabled, there is no [vectors] page provided in a program's
> virtual address space, so accesses to that virtual address will cause a
> fault which we are catching here.
>
> When CONFIG_KUSER_HELPERS is enabled, every program gets a valid vectors
> page in its virtual address space, and accessing that address would not
> cause a fault, since the page is there.
>
> Does this clarify the intent of this commit?

Yes, It's clear. It might be good to add little more description on
commit message here.
But it's your choice on taking this input :-)
>
> >
> >> +       if ((sig == SIGSEGV) && ((addr & PAGE_MASK) == 0xffff0000))
> >> +               printk(KERN_DEBUG "%s: CONFIG_KUSER_HELPERS disabled at 0x%08lx\n",
> >> +                      tsk->comm, addr);
> >> +#endif
> >>
> >>         tsk->thread.address = addr;
> >>         tsk->thread.error_code = fsr;
> >> --
> >> 2.17.1
> >>
>
>
> --
> Florian