netdev - Re: [PATCH 12/13] x86/jitalloc: prepare to allocate exectuatble memory as ROX

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]

Message-ID: <fc80e5bcec4f1fd06d9d81a2ccae3f489166d503.camel@intel.com>
Date: Mon, 5 Jun 2023 21:01:14 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "rppt@...nel.org" <rppt@...nel.org>
CC: "tglx@...utronix.de" <tglx@...utronix.de>, "deller@....de"
	<deller@....de>, "mcgrof@...nel.org" <mcgrof@...nel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "nadav.amit@...il.com"
	<nadav.amit@...il.com>, "linux@...linux.org.uk" <linux@...linux.org.uk>,
	"davem@...emloft.net" <davem@...emloft.net>, "linux-mips@...r.kernel.org"
	<linux-mips@...r.kernel.org>, "linux-riscv@...ts.infradead.org"
	<linux-riscv@...ts.infradead.org>, "hca@...ux.ibm.com" <hca@...ux.ibm.com>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"catalin.marinas@....com" <catalin.marinas@....com>,
	"kent.overstreet@...ux.dev" <kent.overstreet@...ux.dev>,
	"linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
	"palmer@...belt.com" <palmer@...belt.com>, "chenhuacai@...nel.org"
	<chenhuacai@...nel.org>, "tsbogend@...ha.franken.de"
	<tsbogend@...ha.franken.de>, "linux-trace-kernel@...r.kernel.org"
	<linux-trace-kernel@...r.kernel.org>, "linux-parisc@...r.kernel.org"
	<linux-parisc@...r.kernel.org>, "christophe.leroy@...roup.eu"
	<christophe.leroy@...roup.eu>, "x86@...nel.org" <x86@...nel.org>,
	"mpe@...erman.id.au" <mpe@...erman.id.au>, "rostedt@...dmis.org"
	<rostedt@...dmis.org>, "will@...nel.org" <will@...nel.org>,
	"dinguyen@...nel.org" <dinguyen@...nel.org>, "naveen.n.rao@...ux.ibm.com"
	<naveen.n.rao@...ux.ibm.com>, "sparclinux@...r.kernel.org"
	<sparclinux@...r.kernel.org>, "linux-modules@...r.kernel.org"
	<linux-modules@...r.kernel.org>, "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "song@...nel.org" <song@...nel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>, "loongarch@...ts.linux.dev"
	<loongarch@...ts.linux.dev>, "akpm@...ux-foundation.org"
	<akpm@...ux-foundation.org>
Subject: Re: [PATCH 12/13] x86/jitalloc: prepare to allocate exectuatble
 memory as ROX

On Mon, 2023-06-05 at 23:42 +0300, Mike Rapoport wrote:
> > I tried this technique previously [0], and I thought it was not too
> > bad. In most of the callers it looks similar to what you have in
> > do_text_poke(). Sometimes less, sometimes more. It might need
> > enlightening of some of the stuff currently using text_poke()
> > during
> > module loading, like jump labels. So that bit is more intrusive,
> > yea.
> > But it sounds so much cleaner and well controlled. Did you have a
> > particular trouble spot in mind?
> 
> Nothing in particular, except the intrusive part. Except the changes
> in
> modules.c we'd need to teach alternatives to deal with a writable
> copy.

I didn't think alternatives piece looked too bad on the caller side (if
that's what you meant):
https://lore.kernel.org/lkml/20201120202426.18009-7-rick.p.edgecombe@intel.com/

The ugly part was in the (poorly named) module_adjust_writable_addr():

+static inline void *module_adjust_writable_addr(void *addr)
+{
+	unsigned long laddr = (unsigned long)addr;
+	struct module *mod;
+
+	mutex_lock(&module_mutex);
+	mod = __module_address(laddr);
+	if (!mod) {
+		mutex_unlock(&module_mutex);
+		return addr;
+	}
+	mutex_unlock(&module_mutex);
+	/* The module shouldn't be going away if someone is trying to
write to it */
+
+	return (void *)perm_writable_addr(module_get_allocation(mod,
laddr), laddr);
+}
+

It took module_mutex and looked up the module in order to find the
writable buffer from just the executable address. Basically all the
loading code external to modules had to go through that interface. But
now I'm wondering what I was thinking, it seems this could just be an
RCU read lock. That doesn't seem to bad...