Re: [PATCH] x86/iopl: Fake iopl(3) CLI/STI usage

[Linus Torvalds]

On Fri, Sep 17, 2021 at 2:21 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> 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