Re: [PATCH] ARM: mm: Facilitate debugging CONFIG_KUSER_HELPERS disabled

[Souptick Joarder <jrdr.linux@gmail.com>]

On Fri, Oct 26, 2018 at 12:18 AM Florian Fainelli <f.fainelli@gmail.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@gmail.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@armlinux.org.uk>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.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