linux-kernel - Re: [PATCH RFC v12 2/6] x86: mm: Skip faulting instruction for VM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 12 Dec 2022 14:01:33 -0700
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     linux-kernel@...r.kernel.org, patches@...ts.linux.dev,
        tglx@...utronix.de
Cc:     linux-crypto@...r.kernel.org, linux-api@...r.kernel.org,
        x86@...nel.org, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Adhemerval Zanella Netto <adhemerval.zanella@...aro.org>,
        Carlos O'Donell <carlos@...hat.com>,
        Florian Weimer <fweimer@...hat.com>,
        Arnd Bergmann <arnd@...db.de>, Jann Horn <jannh@...gle.com>,
        Christian Brauner <brauner@...nel.org>, linux-mm@...ck.org
Subject: Re: [PATCH RFC v12 2/6] x86: mm: Skip faulting instruction for
 VM_DROPPABLE faults

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