| Message ID | 20220222155345.138861-1-tsbogend@alpha.franken.de (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | MIPS: Handle address errors for accesses above CPU max virtual user address | expand |
On Tue, Feb 22, 2022 at 4:53 PM Thomas Bogendoerfer <tsbogend@alpha.franken.de> wrote: > > Address errors have always been treated as unaliged accesses and handled > as such. But address errors are also issued for illegal accesses like > user to kernel space or accesses outside of implemented spaces. This > change implements Linux exception handling for accesses to the illegal > space above the CPU implemented maximum virtual user address and the > MIPS 64bit architecture maximum. With this we can now use a fixed value > for the maximum task size on every MIPS CPU and get a more optimized > access_ok(). > > Signed-off-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de> Thank you for addressing this. Should I add this patch to my series ahead of "mips: use simpler access_ok()"? That way I can keep it all in my asm-generic tree as a series for 5.18. > arch/mips/kernel/unaligned.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/arch/mips/kernel/unaligned.c b/arch/mips/kernel/unaligned.c > index df4b708c04a9..7b5aba5df02e 100644 > --- a/arch/mips/kernel/unaligned.c > +++ b/arch/mips/kernel/unaligned.c > @@ -1480,6 +1480,23 @@ asmlinkage void do_ade(struct pt_regs *regs) > prev_state = exception_enter(); > perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, > 1, regs, regs->cp0_badvaddr); > + > +#ifdef CONFIG_64BIT > + /* > + * check, if we are hitting space between CPU implemented maximum > + * virtual user address and 64bit maximum virtual user address > + * and do exception handling to get EFAULTs for get_user/put_user > + */ > + if ((regs->cp0_badvaddr >= (1UL << cpu_vmbits)) && > + (regs->cp0_badvaddr < XKSSEG)) { It might be clearer to use TASK_SIZE_MAX here instead of XKSSEG, to match the check in access_ok(). If you like, I can change that while applying. Arnd
On Tue, Feb 22, 2022 at 06:04:07PM +0100, Arnd Bergmann wrote: > On Tue, Feb 22, 2022 at 4:53 PM Thomas Bogendoerfer > <tsbogend@alpha.franken.de> wrote: > > > > Address errors have always been treated as unaliged accesses and handled > > as such. But address errors are also issued for illegal accesses like > > user to kernel space or accesses outside of implemented spaces. This > > change implements Linux exception handling for accesses to the illegal > > space above the CPU implemented maximum virtual user address and the > > MIPS 64bit architecture maximum. With this we can now use a fixed value > > for the maximum task size on every MIPS CPU and get a more optimized > > access_ok(). > > > > Signed-off-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > > Thank you for addressing this. Should I add this patch to my series > ahead of "mips: use simpler access_ok()"? That way I can keep it all > in my asm-generic tree as a series for 5.18. yes please add it to your series. > > arch/mips/kernel/unaligned.c | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/arch/mips/kernel/unaligned.c b/arch/mips/kernel/unaligned.c > > index df4b708c04a9..7b5aba5df02e 100644 > > --- a/arch/mips/kernel/unaligned.c > > +++ b/arch/mips/kernel/unaligned.c > > @@ -1480,6 +1480,23 @@ asmlinkage void do_ade(struct pt_regs *regs) > > prev_state = exception_enter(); > > perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, > > 1, regs, regs->cp0_badvaddr); > > + > > +#ifdef CONFIG_64BIT > > + /* > > + * check, if we are hitting space between CPU implemented maximum > > + * virtual user address and 64bit maximum virtual user address > > + * and do exception handling to get EFAULTs for get_user/put_user > > + */ > > + if ((regs->cp0_badvaddr >= (1UL << cpu_vmbits)) && > > + (regs->cp0_badvaddr < XKSSEG)) { > > It might be clearer to use TASK_SIZE_MAX here instead of XKSSEG, > to match the check in access_ok(). If you like, I can change that while > applying. I had TASK_SIZE_MAX in an intermediate version, but decided to go with XKSSEG, because it's what this check is about. It's about checking what the MIPS architecture defined. Thomas.
On Tue, Feb 22, 2022 at 8:58 PM Thomas Bogendoerfer <tsbogend@alpha.franken.de> wrote: > > On Tue, Feb 22, 2022 at 06:04:07PM +0100, Arnd Bergmann wrote: > > On Tue, Feb 22, 2022 at 4:53 PM Thomas Bogendoerfer > > <tsbogend@alpha.franken.de> wrote: > > > > > > Address errors have always been treated as unaliged accesses and handled > > > as such. But address errors are also issued for illegal accesses like > > > user to kernel space or accesses outside of implemented spaces. This > > > change implements Linux exception handling for accesses to the illegal > > > space above the CPU implemented maximum virtual user address and the > > > MIPS 64bit architecture maximum. With this we can now use a fixed value > > > for the maximum task size on every MIPS CPU and get a more optimized > > > access_ok(). > > > > > > Signed-off-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > > > > Thank you for addressing this. Should I add this patch to my series > > ahead of "mips: use simpler access_ok()"? That way I can keep it all > > in my asm-generic tree as a series for 5.18. > > yes please add it to your series. Done now. > > > > It might be clearer to use TASK_SIZE_MAX here instead of XKSSEG, > > to match the check in access_ok(). If you like, I can change that while > > applying. > > I had TASK_SIZE_MAX in an intermediate version, but decided to go with XKSSEG, > because it's what this check is about. It's about checking what the MIPS > architecture defined. Right, makes sense. Thanks, Arnd
On Tue, 22 Feb 2022, Thomas Bogendoerfer wrote: > Address errors have always been treated as unaliged accesses and handled > as such. But address errors are also issued for illegal accesses like > user to kernel space or accesses outside of implemented spaces. This > change implements Linux exception handling for accesses to the illegal > space above the CPU implemented maximum virtual user address and the > MIPS 64bit architecture maximum. With this we can now use a fixed value > for the maximum task size on every MIPS CPU and get a more optimized > access_ok(). Decades ago I had a patch to handle this with CPU-specific limits, which in the end I have not submitted for one reason or another. I guess we didn't have what is `cpu_vmbits' nowadays. I'll see if I can dig it out and check if there's anything interesting there we might also require. Overall good work, and I'm rather embarassed we have gone so far without it. Thanks! Maciej
diff --git a/arch/mips/kernel/unaligned.c b/arch/mips/kernel/unaligned.c index df4b708c04a9..7b5aba5df02e 100644 --- a/arch/mips/kernel/unaligned.c +++ b/arch/mips/kernel/unaligned.c @@ -1480,6 +1480,23 @@ asmlinkage void do_ade(struct pt_regs *regs) prev_state = exception_enter(); perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, regs->cp0_badvaddr); + +#ifdef CONFIG_64BIT + /* + * check, if we are hitting space between CPU implemented maximum + * virtual user address and 64bit maximum virtual user address + * and do exception handling to get EFAULTs for get_user/put_user + */ + if ((regs->cp0_badvaddr >= (1UL << cpu_vmbits)) && + (regs->cp0_badvaddr < XKSSEG)) { + if (fixup_exception(regs)) { + current->thread.cp0_baduaddr = regs->cp0_badvaddr; + return; + } + goto sigbus; + } +#endif + /* * Did we catch a fault trying to load an instruction? */
Address errors have always been treated as unaliged accesses and handled as such. But address errors are also issued for illegal accesses like user to kernel space or accesses outside of implemented spaces. This change implements Linux exception handling for accesses to the illegal space above the CPU implemented maximum virtual user address and the MIPS 64bit architecture maximum. With this we can now use a fixed value for the maximum task size on every MIPS CPU and get a more optimized access_ok(). Signed-off-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de> --- arch/mips/kernel/unaligned.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)