Hi all,
This series fixed extable handling for architecture delay slot (MIPS).
Please see previous discussions at [1].
There are some other places in kernel not handling delay slots properly,
such as uprobe and kgdb, I'll sort them later.
Thanks!
[1]: https://lore.kernel.org/lkml/[email protected]
To: Oleg Nesterov <[email protected]>
To: Thomas Bogendoerfer <[email protected]>
To: Andrew Morton <[email protected]>
To: Ben Hutchings <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Signed-off-by: Jiaxun Yang <[email protected]>
---
Changes in v2:
- Reduce diffstat by implemente fallback macro in linux/ptrace.h (linus)
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Jiaxun Yang (3):
ptrace: Introduce exception_ip arch hook
MIPS: Clear Cause.BD in instruction_pointer_set
mm/memory: Use exception ip to search exception tables
arch/mips/include/asm/ptrace.h | 3 +++
arch/mips/kernel/ptrace.c | 7 +++++++
include/linux/ptrace.h | 4 ++++
mm/memory.c | 4 ++--
4 files changed, 16 insertions(+), 2 deletions(-)
---
base-commit: 06f658aadff0e483ee4f807b0b46c9e5cba62bfa
change-id: 20240131-exception_ip-194e4ad0e6ca
Best regards,
--
Jiaxun Yang <[email protected]>
On Fri, 2 Feb 2024 at 04:30, Jiaxun Yang <[email protected]> wrote:
>
> ptrace: Introduce exception_ip arch hook
> MIPS: Clear Cause.BD in instruction_pointer_set
> mm/memory: Use exception ip to search exception tables
Just to clarify: does that second patch fix the problem that
__isa_exception_epc() does a __get_user()?
Because that mm/memory.c use of "exception_ip()" most definitely
cannot take a page fault.
So if MIPS cannot do that whole exception IP thing without potentially
touching user space, I do worry that we might just have to add a
#ifndef __MIPS__
around this all.
Possibly somehow limited to just the microMIPS/MIPS16e case in Kconfig instead?
Linus
在 2024/2/2 18:39, Linus Torvalds 写道:
> On Fri, 2 Feb 2024 at 04:30, Jiaxun Yang <[email protected]> wrote:
>> ptrace: Introduce exception_ip arch hook
>> MIPS: Clear Cause.BD in instruction_pointer_set
>> mm/memory: Use exception ip to search exception tables
> Just to clarify: does that second patch fix the problem that
> __isa_exception_epc() does a __get_user()?
No it only covers an obvious case when I was playing around exception_ip
with kgdb.
There are still potential cases __isa_exception_epc may touch user space.
> Because that mm/memory.c use of "exception_ip()" most definitely
> cannot take a page fault.
>
> So if MIPS cannot do that whole exception IP thing without potentially
> touching user space, I do worry that we might just have to add a
>
> #ifndef __MIPS__
>
> around this all.
It is possible to perform exception_ip() without touching user space by
saving "BadInstr" in pt_regs
at exception entries. For newer hardware it's as simple as saving an
extra CP0 register, on older hardware
we may have to read it from user space.
+ Thomas (MIPS maintainer), what's your opinion?
Thanks
>
> Possibly somehow limited to just the microMIPS/MIPS16e case in Kconfig instead?
>
> Linus
--
---
Jiaxun Yang
在 2024/2/2 18:39, Linus Torvalds 写道:
> On Fri, 2 Feb 2024 at 04:30, Jiaxun Yang <[email protected]> wrote:
>> ptrace: Introduce exception_ip arch hook
>> MIPS: Clear Cause.BD in instruction_pointer_set
>> mm/memory: Use exception ip to search exception tables
> Just to clarify: does that second patch fix the problem that
> __isa_exception_epc() does a __get_user()?
>
> Because that mm/memory.c use of "exception_ip()" most definitely
> cannot take a page fault.
I reconsidered this issue today and I believe that exception_ip() usage
here won't trigger
page_fault anyway.
Given that exception_ip is guarded by !user_mode(regs), EPC must points
to a kernel
text address. There is no way accessing kernel text will generate such
exception..
Thanks
>
> So if MIPS cannot do that whole exception IP thing without potentially
> touching user space, I do worry that we might just have to add a
>
> #ifndef __MIPS__
>
> around this all.
>
> Possibly somehow limited to just the microMIPS/MIPS16e case in Kconfig instead?
>
> Linus
--
---
Jiaxun Yang
On Sat, 3 Feb 2024 at 13:56, Jiaxun Yang <[email protected]> wrote:
>
> Given that exception_ip is guarded by !user_mode(regs), EPC must points
> to a kernel text address. There is no way accessing kernel text will generate such
> exception..
Sadly, that's not actually likely true.
The thing is, the only reason for the code in
get_mmap_lock_carefully() is for kernel bugs. IOW, some kernel bug
with a wild pointer, and we do not want to deadlock on the mm
semaphore, we want to get back to the fault handler and it should
report an oops.
.. and one of the "wild pointers" might in fact be the instruction
pointer, in case we've branched through a NULL function pointer or
similar. IOW, the exception *source* might be the instruction pointer
itself.
So I realy think that MIPS needs to have some kind of "safe kernel
exception pointer" helper. One that is guaranteed not to fault when
evaluating the exception pointer.
Assuming the kernel itself is never built with MIPS16e instructions,
maybe that's a safe assumption thanks to the "get_isa16_mode()" check
of EPC. But all of this makes me nervous.
Linus
在 2024/2/4 07:41, Linus Torvalds 写道:
[...]
> The thing is, the only reason for the code in
> get_mmap_lock_carefully() is for kernel bugs. IOW, some kernel bug
> with a wild pointer, and we do not want to deadlock on the mm
> semaphore, we want to get back to the fault handler and it should
> report an oops.
>
> ... and one of the "wild pointers" might in fact be the instruction
> pointer, in case we've branched through a NULL function pointer or
> similar. IOW, the exception *source* might be the instruction pointer
> itself.
Well this is the tricky part of my assumption.
In `exception_epc()` `__isa_exception_epc()` stuff is only called if we
are in delay slot.
It is impossible for a invalid instruction_pointer to be considered as
"in delay slot"
by hardware.
I'd agree that sounds fragile though.
Thanks
>
> So I realy think that MIPS needs to have some kind of "safe kernel
> exception pointer" helper. One that is guaranteed not to fault when
> evaluating the exception pointer.
>
> Assuming the kernel itself is never built with MIPS16e instructions,
> maybe that's a safe assumption thanks to the "get_isa16_mode()" check
> of EPC. But all of this makes me nervous.
>
> Linus
--
---
Jiaxun Yang
On Sun, 4 Feb 2024 at 11:03, Jiaxun Yang <[email protected]> wrote:
>
> Well this is the tricky part of my assumption.
> In `exception_epc()` `__isa_exception_epc()` stuff is only called if we
> are in delay slot.
> It is impossible for a invalid instruction_pointer to be considered as
> "in delay slot" by hardware.
Ok, I guess I'm convinced this is all safe. Not great, and not exactly
giving me the warm fuzzies, but not buggy.
Linus
在 2024/2/2 12:30, Jiaxun Yang 写道:
> Hi all,
>
> This series fixed extable handling for architecture delay slot (MIPS).
>
> Please see previous discussions at [1].
>
> There are some other places in kernel not handling delay slots properly,
> such as uprobe and kgdb, I'll sort them later.
A gentle ping :-)
This series fixes a regression, perhaps it should go through fixes tree.
Thanks
- Jiaxun
>
> Thanks!
>
> [1]: https://lore.kernel.org/lkml/[email protected]
>
> To: Oleg Nesterov <[email protected]>
>
> To: Thomas Bogendoerfer <[email protected]>
>
> To: Andrew Morton <[email protected]>
> To: Ben Hutchings <[email protected]>
>
> Cc: <[email protected]>
> Cc: <[email protected]>
>
> Cc: <[email protected]>
>
> Cc: <[email protected]>
>
> Signed-off-by: Jiaxun Yang <[email protected]>
> ---
> Changes in v2:
> - Reduce diffstat by implemente fallback macro in linux/ptrace.h (linus)
> - Link to v1: https://lore.kernel.org/r/[email protected]
>
> ---
> Jiaxun Yang (3):
> ptrace: Introduce exception_ip arch hook
> MIPS: Clear Cause.BD in instruction_pointer_set
> mm/memory: Use exception ip to search exception tables
>
> arch/mips/include/asm/ptrace.h | 3 +++
> arch/mips/kernel/ptrace.c | 7 +++++++
> include/linux/ptrace.h | 4 ++++
> mm/memory.c | 4 ++--
> 4 files changed, 16 insertions(+), 2 deletions(-)
> ---
> base-commit: 06f658aadff0e483ee4f807b0b46c9e5cba62bfa
> change-id: 20240131-exception_ip-194e4ad0e6ca
>
> Best regards,
--
---
Jiaxun Yang
On Thu, 2024-02-08 at 09:20 +0000, Jiaxun Yang wrote:
>
>
> 在 2024/2/2 12:30, Jiaxun Yang 写道:
> > Hi all,
> >
> > This series fixed extable handling for architecture delay slot (MIPS).
> >
> > Please see previous discussions at [1].
> >
> > There are some other places in kernel not handling delay slots properly,
> > such as uprobe and kgdb, I'll sort them later.
>
> A gentle ping :-)
>
> This series fixes a regression, perhaps it should go through fixes tree.
If you have Fixes: {sha} and Cc: [email protected] Greg will pick
it up once it's in Linus tree.
--
Xi Ruoyao <[email protected]>
School of Aerospace Science and Technology, Xidian University
在 2024/2/8 09:23, Xi Ruoyao 写道:
> On Thu, 2024-02-08 at 09:20 +0000, Jiaxun Yang wrote:
>>
>> 在 2024/2/2 12:30, Jiaxun Yang 写道:
>>> Hi all,
>>>
>>> This series fixed extable handling for architecture delay slot (MIPS).
>>>
>>> Please see previous discussions at [1].
>>>
>>> There are some other places in kernel not handling delay slots properly,
>>> such as uprobe and kgdb, I'll sort them later.
>> A gentle ping :-)
>>
>> This series fixes a regression, perhaps it should go through fixes tree.
> If you have Fixes: {sha} and Cc: [email protected] Greg will pick
> it up once it's in Linus tree.
In this case I'd like to backport it manually, as there is only one
patch in this series
actually fixing the problem.
Thanks
- Jiaxun
>
--
---
Jiaxun Yang
On Fri, Feb 02, 2024 at 12:30:25PM +0000, Jiaxun Yang wrote:
> Hi all,
>
> This series fixed extable handling for architecture delay slot (MIPS).
>
> Please see previous discussions at [1].
>
> There are some other places in kernel not handling delay slots properly,
> such as uprobe and kgdb, I'll sort them later.
>
> Thanks!
>
> [1]: https://lore.kernel.org/lkml/[email protected]
>
> To: Oleg Nesterov <[email protected]>
>
> To: Thomas Bogendoerfer <[email protected]>
>
> To: Andrew Morton <[email protected]>
> To: Ben Hutchings <[email protected]>
>
> Cc: <[email protected]>
> Cc: <[email protected]>
>
> Cc: <[email protected]>
>
> Cc: <[email protected]>
>
> Signed-off-by: Jiaxun Yang <[email protected]>
> ---
> Changes in v2:
> - Reduce diffstat by implemente fallback macro in linux/ptrace.h (linus)
> - Link to v1: https://lore.kernel.org/r/[email protected]
>
> ---
> Jiaxun Yang (3):
> ptrace: Introduce exception_ip arch hook
> MIPS: Clear Cause.BD in instruction_pointer_set
> mm/memory: Use exception ip to search exception tables
>
> arch/mips/include/asm/ptrace.h | 3 +++
> arch/mips/kernel/ptrace.c | 7 +++++++
> include/linux/ptrace.h | 4 ++++
> mm/memory.c | 4 ++--
> 4 files changed, 16 insertions(+), 2 deletions(-)
series applied to mips-fixes.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]