Messages in this thread | | | From | Linus Torvalds <> | Date | Fri, 17 Sep 2021 15:23:18 -0700 | Subject | Re: [PATCH] x86/iopl: Fake iopl(3) CLI/STI usage |
| |
On Fri, Sep 17, 2021 at 2:21 AM Peter Zijlstra <peterz@infradead.org> wrote: > > + nr_copied = insn_fetch_from_user(regs, buf);
Ugh. This is the code that does the magic "add CS base" stuff.
Do we really want to do that instead of just doing
unsigned char byte = get_user((char __user *)regs->ip);
when later on the debug code does:
> + pr_err("%s[%d] attempts to use CLI/STI, pretending it's a NOP, ip:%lx", > + current->comm, task_pid_nr(current), regs->ip); > + print_vma_addr(KERN_CONT " in ", regs->ip); > + pr_cont("\n");
and prints out the wrong IP address?
IOW, I'd argue that you should get it right in both places, or not try to get it right in one but not the other.
I think the proper thing to do is perhaps something like
unsigned long cs_base = 0; unsigned long address; unsigned char byte;
if (!user_64bit_mode(regs)) { cs_base = insn_get_seg_base(regs, INAT_SEG_REG_CS); if (cs_base = -1ul) return false; }
// We could check the limit too, but nobody cares address = regs->ip + cs_base; if (get_user(byte, (const char __user *)address)) return false;
// cli/sti? if (byte != 0xfa && byte ! 0xfb) return false;
and now you have the actual linear address in 'address' and can at least print it out correctly.
Hmm? Because it's just sad to get it right in one place, and wrong in another. And we don't actually _want_ any of the instruction fetch/decode stuff.
Linus
| |