Message ID | 20190820144227.25380-2-kleber.souza@canonical.com |
---|---|
State | New |
Headers | show |
Series | fix for kprobes regression (LP: #1840750) | expand |
On 20/08/2019 15:42, Kleber Sacilotto de Souza wrote: > From: Nadav Amit <namit@vmware.com> > > BugLink: https://bugs.launchpad.net/bugs/1840750 > > Set the page as executable after allocation. This patch is a > preparatory patch for a following patch that makes module allocated > pages non-executable. > > While at it, do some small cleanup of what appears to be unnecessary > masking. > > Signed-off-by: Nadav Amit <namit@vmware.com> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Cc: <akpm@linux-foundation.org> > Cc: <ard.biesheuvel@linaro.org> > Cc: <deneen.t.dock@intel.com> > Cc: <kernel-hardening@lists.openwall.com> > Cc: <kristen@linux.intel.com> > Cc: <linux_dti@icloud.com> > Cc: <will.deacon@arm.com> > Cc: Andy Lutomirski <luto@kernel.org> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: H. Peter Anvin <hpa@zytor.com> > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Cc: Rik van Riel <riel@surriel.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Link: https://lkml.kernel.org/r/20190426001143.4983-11-namit@vmware.com > Signed-off-by: Ingo Molnar <mingo@kernel.org> > (cherry picked from commit 7298e24f904224fa79eb8fd7e0fbd78950ccf2db) > Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > arch/x86/kernel/kprobes/core.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c > index f4b954ff5b89..3bc4cc70f1e5 100644 > --- a/arch/x86/kernel/kprobes/core.c > +++ b/arch/x86/kernel/kprobes/core.c > @@ -431,8 +431,20 @@ void *alloc_insn_page(void) > void *page; > > page = module_alloc(PAGE_SIZE); > - if (page) > - set_memory_ro((unsigned long)page & PAGE_MASK, 1); > + if (!page) > + return NULL; > + > + /* > + * First make the page read-only, and only then make it executable to > + * prevent it from being W+X in between. > + */ > + set_memory_ro((unsigned long)page, 1); > + > + /* > + * TODO: Once additional kernel code protection mechanisms are set, ensure > + * that the page was not maliciously altered and it is still zeroed. > + */ > + set_memory_x((unsigned long)page, 1); > > return page; > } > @@ -440,8 +452,12 @@ void *alloc_insn_page(void) > /* Recover page to RW mode before releasing it */ > void free_insn_page(void *page) > { > - set_memory_nx((unsigned long)page & PAGE_MASK, 1); > - set_memory_rw((unsigned long)page & PAGE_MASK, 1); > + /* > + * First make the page non-executable, and only then make it writable to > + * prevent it from being W+X in between. > + */ > + set_memory_nx((unsigned long)page, 1); > + set_memory_rw((unsigned long)page, 1); > module_memfree(page); > } > > Clean upstream cherry pick, addresses the issue. Acked-by: Colin Ian King <colin.king@canonical.com>
On 20.08.19 16:42, Kleber Sacilotto de Souza wrote: > From: Nadav Amit <namit@vmware.com> > > BugLink: https://bugs.launchpad.net/bugs/1840750 > > Set the page as executable after allocation. This patch is a > preparatory patch for a following patch that makes module allocated > pages non-executable. > > While at it, do some small cleanup of what appears to be unnecessary > masking. > > Signed-off-by: Nadav Amit <namit@vmware.com> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Cc: <akpm@linux-foundation.org> > Cc: <ard.biesheuvel@linaro.org> > Cc: <deneen.t.dock@intel.com> > Cc: <kernel-hardening@lists.openwall.com> > Cc: <kristen@linux.intel.com> > Cc: <linux_dti@icloud.com> > Cc: <will.deacon@arm.com> > Cc: Andy Lutomirski <luto@kernel.org> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: H. Peter Anvin <hpa@zytor.com> > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Cc: Rik van Riel <riel@surriel.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Link: https://lkml.kernel.org/r/20190426001143.4983-11-namit@vmware.com > Signed-off-by: Ingo Molnar <mingo@kernel.org> > (cherry picked from commit 7298e24f904224fa79eb8fd7e0fbd78950ccf2db) > Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- The small cleanup of not masking is probably only clear to some people... > arch/x86/kernel/kprobes/core.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c > index f4b954ff5b89..3bc4cc70f1e5 100644 > --- a/arch/x86/kernel/kprobes/core.c > +++ b/arch/x86/kernel/kprobes/core.c > @@ -431,8 +431,20 @@ void *alloc_insn_page(void) > void *page; > > page = module_alloc(PAGE_SIZE); > - if (page) > - set_memory_ro((unsigned long)page & PAGE_MASK, 1); > + if (!page) > + return NULL; > + > + /* > + * First make the page read-only, and only then make it executable to > + * prevent it from being W+X in between. > + */ > + set_memory_ro((unsigned long)page, 1); > + > + /* > + * TODO: Once additional kernel code protection mechanisms are set, ensure > + * that the page was not maliciously altered and it is still zeroed. > + */ > + set_memory_x((unsigned long)page, 1); > > return page; > } > @@ -440,8 +452,12 @@ void *alloc_insn_page(void) > /* Recover page to RW mode before releasing it */ > void free_insn_page(void *page) > { > - set_memory_nx((unsigned long)page & PAGE_MASK, 1); > - set_memory_rw((unsigned long)page & PAGE_MASK, 1); > + /* > + * First make the page non-executable, and only then make it writable to > + * prevent it from being W+X in between. > + */ > + set_memory_nx((unsigned long)page, 1); > + set_memory_rw((unsigned long)page, 1); > module_memfree(page); > } > >
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c index f4b954ff5b89..3bc4cc70f1e5 100644 --- a/arch/x86/kernel/kprobes/core.c +++ b/arch/x86/kernel/kprobes/core.c @@ -431,8 +431,20 @@ void *alloc_insn_page(void) void *page; page = module_alloc(PAGE_SIZE); - if (page) - set_memory_ro((unsigned long)page & PAGE_MASK, 1); + if (!page) + return NULL; + + /* + * First make the page read-only, and only then make it executable to + * prevent it from being W+X in between. + */ + set_memory_ro((unsigned long)page, 1); + + /* + * TODO: Once additional kernel code protection mechanisms are set, ensure + * that the page was not maliciously altered and it is still zeroed. + */ + set_memory_x((unsigned long)page, 1); return page; } @@ -440,8 +452,12 @@ void *alloc_insn_page(void) /* Recover page to RW mode before releasing it */ void free_insn_page(void *page) { - set_memory_nx((unsigned long)page & PAGE_MASK, 1); - set_memory_rw((unsigned long)page & PAGE_MASK, 1); + /* + * First make the page non-executable, and only then make it writable to + * prevent it from being W+X in between. + */ + set_memory_nx((unsigned long)page, 1); + set_memory_rw((unsigned long)page, 1); module_memfree(page); }