On Mon, Dec 12, 2022 at 11:53:43AM -0700, Jason A. Donenfeld wrote:
> + if (fault & VM_FAULT_SKIP_INSN) {
> + u8 insn_buf[MAX_INSN_SIZE];
> + struct insn insn;
> + size_t len;
> +
> + len = sizeof(insn_buf) - copy_from_user(insn_buf, (void *)regs->ip, sizeof(insn_buf));
> + if (!len)
> + return;
> +
> + if (insn_decode(&insn, insn_buf, len, in_32bit_syscall() ? INSN_MODE_32 : INSN_MODE_64) < 0)
> + return;
> +
> + regs->ip += insn.length;
> + return;
> + }
I just found umip.c, which does basically the same thing, but does it
correctly. For v+1, the above snippet will instead do this:
if (fault & VM_FAULT_SKIP_INSN) {
u8 buf[MAX_INSN_SIZE];
struct insn insn;
int nr_copied;
nr_copied = insn_fetch_from_user(regs, buf);
if (nr_copied <= 0)
return;
if (!insn_decode_from_regs(&insn, regs, buf, nr_copied))
return;
regs->ip += insn.length;
return;
}
Same thing, but those helpers do correct inspection of the environment
and registers.
Also, seeing this already being done in umip.c is heartening that the
approach here isn't overly insane.
Jason