linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ARM: mm: Facilitate debugging CONFIG_KUSER_HELPERS disabled
@ 2018-10-25 18:09 Florian Fainelli
  2018-10-25 18:25 ` Eric W. Biederman
  2018-10-25 18:30 ` Souptick Joarder
  0 siblings, 2 replies; 7+ messages in thread
From: Florian Fainelli @ 2018-10-25 18:09 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: andrew, Florian Fainelli, Russell King, Eric W. Biederman,
	Tony Lindgren, Souptick Joarder, open list

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
+	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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ARM: mm: Facilitate debugging CONFIG_KUSER_HELPERS disabled
  2018-10-25 18:09 [PATCH] ARM: mm: Facilitate debugging CONFIG_KUSER_HELPERS disabled Florian Fainelli
@ 2018-10-25 18:25 ` Eric W. Biederman
  2018-10-25 18:50   ` Florian Fainelli
  2018-10-25 18:30 ` Souptick Joarder
  1 sibling, 1 reply; 7+ messages in thread
From: Eric W. Biederman @ 2018-10-25 18:25 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-arm-kernel, andrew, Russell King, Tony Lindgren,
	Souptick Joarder, open list

Florian Fainelli <f.fainelli@gmail.com> writes:

> 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.

Can't someone trigger this segv deliberately and spam the kerne log?
Doesn't this need something like printk_ratelimit or something to ensure
this message only gets printed once?

Eric

> 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
> +	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;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ARM: mm: Facilitate debugging CONFIG_KUSER_HELPERS disabled
  2018-10-25 18:09 [PATCH] ARM: mm: Facilitate debugging CONFIG_KUSER_HELPERS disabled Florian Fainelli
  2018-10-25 18:25 ` Eric W. Biederman
@ 2018-10-25 18:30 ` Souptick Joarder
  2018-10-25 18:48   ` Florian Fainelli
  2018-10-25 21:20   ` Russell King - ARM Linux
  1 sibling, 2 replies; 7+ messages in thread
From: Souptick Joarder @ 2018-10-25 18:30 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-arm-kernel, Andrew Lunn, Russell King - ARM Linux,
	ebiederm, Tony Lindgren, linux-kernel

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 ?

> +       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
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ARM: mm: Facilitate debugging CONFIG_KUSER_HELPERS disabled
  2018-10-25 18:30 ` Souptick Joarder
@ 2018-10-25 18:48   ` Florian Fainelli
  2018-10-25 19:06     ` Souptick Joarder
  2018-10-25 21:20   ` Russell King - ARM Linux
  1 sibling, 1 reply; 7+ messages in thread
From: Florian Fainelli @ 2018-10-25 18:48 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: linux-arm-kernel, Andrew Lunn, Russell King - ARM Linux,
	ebiederm, Tony Lindgren, linux-kernel

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?

> 
>> +       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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ARM: mm: Facilitate debugging CONFIG_KUSER_HELPERS disabled
  2018-10-25 18:25 ` Eric W. Biederman
@ 2018-10-25 18:50   ` Florian Fainelli
  0 siblings, 0 replies; 7+ messages in thread
From: Florian Fainelli @ 2018-10-25 18:50 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: linux-arm-kernel, andrew, Russell King, Tony Lindgren,
	Souptick Joarder, open list

On 10/25/18 11:25 AM, Eric W. Biederman wrote:
> Florian Fainelli <f.fainelli@gmail.com> writes:
> 
>> 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.
> 
> Can't someone trigger this segv deliberately and spam the kerne log?
> Doesn't this need something like printk_ratelimit or something to ensure
> this message only gets printed once?

Yes, good point, thanks.
-- 
Florian

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ARM: mm: Facilitate debugging CONFIG_KUSER_HELPERS disabled
  2018-10-25 18:48   ` Florian Fainelli
@ 2018-10-25 19:06     ` Souptick Joarder
  0 siblings, 0 replies; 7+ messages in thread
From: Souptick Joarder @ 2018-10-25 19:06 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: linux-arm-kernel, Andrew Lunn, Russell King - ARM Linux,
	ebiederm, Tony Lindgren, linux-kernel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] ARM: mm: Facilitate debugging CONFIG_KUSER_HELPERS disabled
  2018-10-25 18:30 ` Souptick Joarder
  2018-10-25 18:48   ` Florian Fainelli
@ 2018-10-25 21:20   ` Russell King - ARM Linux
  1 sibling, 0 replies; 7+ messages in thread
From: Russell King - ARM Linux @ 2018-10-25 21:20 UTC (permalink / raw)
  To: Souptick Joarder
  Cc: Florian Fainelli, linux-arm-kernel, Andrew Lunn, ebiederm,
	Tony Lindgren, linux-kernel

On Fri, Oct 26, 2018 at 12:00:09AM +0530, 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 ?
> 
> > +       if ((sig == SIGSEGV) && ((addr & PAGE_MASK) == 0xffff0000))
> > +               printk(KERN_DEBUG "%s: CONFIG_KUSER_HELPERS disabled at 0x%08lx\n",
> > +                      tsk->comm, addr);
> > +#endif

The idea is to print a message when we get a SEGV _and_ the faulting
address is in the vectors page _and_ CONFIG_KUSER_HELPERS is _not_ set
(which makes the page inaccessible.)  The message points users to
enable it and/or why the application has failed.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-10-25 21:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-25 18:09 [PATCH] ARM: mm: Facilitate debugging CONFIG_KUSER_HELPERS disabled Florian Fainelli
2018-10-25 18:25 ` Eric W. Biederman
2018-10-25 18:50   ` Florian Fainelli
2018-10-25 18:30 ` Souptick Joarder
2018-10-25 18:48   ` Florian Fainelli
2018-10-25 19:06     ` Souptick Joarder
2018-10-25 21:20   ` Russell King - ARM Linux

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).