On Sun, Jan 31, 2021 at 09:24:32AM -0800, Andy Lutomirski wrote: > The recent rework of probe_kernel_read() and its conversion to Judging by 25f12ae45fc1 ("maccess: rename probe_kernel_address to get_kernel_nofault") I think you mean probe_kernel_address() above and below. > get_kernel_nofault() inadvertently broke is_prefetch(). We were using Let's drop the "we" pls and switch to passive voice. > probe_kernel_read() as a sloppy "read user or kernel memory" helper, but it > doens't do that any more. The new get_kernel_nofault() reads *kernel* > memory only, which completely broke is_prefetch() for user access. > > Adjust the code to the the correct accessor based on access mode. The s/the // > manual address bounds check is no longer necessary, since the accessor > helpers (get_user() / get_kernel_nofault()) do the right thing all by > themselves. As a bonus, by using the correct accessor, we don't need the > open-coded address bounds check. > > While we're at it, disable the workaround on all CPUs except AMD Family > 0xF. By my reading of the Revision Guide for AMD Athlon™ 64 and AMD > Opteron™ Processors, only family 0xF is affected. Yah, actually, only !NPT K8s have the erratum listed, i.e., CPU models < 0x40, AFAICT. I.e., your test should be: struct cpuinfo_x86 *c = &boot_cpu_data; ... /* Erratum #91 on AMD K8, pre-NPT CPUs */ if (likely(c->x86_vendor != X86_VENDOR_AMD || c->x86 != 0xf || c->x86_model >= 0x40)) return 0; I can try to dig out such a machine to test this on if you wanna. We might still have one collecting dust somewhere in a corner... > Fixes: eab0c6089b68 ("maccess: unify the probe kernel arch hooks") > Cc: stable@xxxxxxxxxxxxxxx @stable because theoretically without that fix, kernel should explode on those machines when it #PFs on a prefetch insn in user mode? Hmm, yap, probably... > Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx> > Cc: Christoph Hellwig <hch@xxxxxx> > Cc: Alexei Starovoitov <ast@xxxxxxxxxx> > Cc: Daniel Borkmann <daniel@xxxxxxxxxxxxx> > Cc: Masami Hiramatsu <mhiramat@xxxxxxxxxx> > Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx> > --- > arch/x86/mm/fault.c | 31 +++++++++++++++++++++---------- > 1 file changed, 21 insertions(+), 10 deletions(-) > > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c > index 106b22d1d189..50dfdc71761e 100644 > --- a/arch/x86/mm/fault.c > +++ b/arch/x86/mm/fault.c > @@ -54,7 +54,7 @@ kmmio_fault(struct pt_regs *regs, unsigned long addr) > * 32-bit mode: > * > * Sometimes AMD Athlon/Opteron CPUs report invalid exceptions on prefetch. > - * Check that here and ignore it. > + * Check that here and ignore it. This is AMD erratum #91. > * > * 64-bit mode: > * > @@ -83,11 +83,7 @@ check_prefetch_opcode(struct pt_regs *regs, unsigned char *instr, > #ifdef CONFIG_X86_64 > case 0x40: > /* > - * In AMD64 long mode 0x40..0x4F are valid REX prefixes > - * Need to figure out under what instruction mode the > - * instruction was issued. Could check the LDT for lm, > - * but for now it's good enough to assume that long > - * mode only uses well known segments or kernel. > + * In 64-bit mode 0x40..0x4F are valid REX prefixes > */ > return (!user_mode(regs) || user_64bit_mode(regs)); > #endif Yah, no need to convert that to the insn decoder - that can die together with the hardware it is supposed to query... -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette