From: Matthias Reichl <hias@horus.com> To: Steven Rostedt <rostedt@goodmis.org> Cc: Kees Cook <keescook@chromium.org>, LKML <linux-kernel@vger.kernel.org>, Russell King <linux@armlinux.org.uk>, Eric Anholt <eric@anholt.net>, Stefan Wahren <stefan.wahren@i2se.com>, Phil Elwell <phil@raspberrypi.org>, linux-rpi-kernel@lists.infradead.org, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH] Arm: mm: ftrace: Only set text back to ro after kernel has been marked ro Date: Tue, 5 Dec 2017 12:47:09 +0100 [thread overview] Message-ID: <20171205114709.f6aj6i426keq2cn5@camel2.lan> (raw) In-Reply-To: <20170823150351.606ba09f@gandalf.local.home> On Wed, Aug 23, 2017 at 03:03:51PM -0400, Steven Rostedt wrote: > On Wed, 23 Aug 2017 11:48:13 -0700 > Kees Cook <keescook@chromium.org> wrote: > > > > diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c > > > index ad80548..fd75f38 100644 > > > --- a/arch/arm/mm/init.c > > > +++ b/arch/arm/mm/init.c > > > @@ -745,19 +745,29 @@ static int __mark_rodata_ro(void *unused) > > > return 0; > > > } > > > > > > +static int kernel_set_to_readonly; > > > > Adding a comment here might be a good idea, something like: > > > > /* Has system boot-up reached mark_rodata_ro() yet? */ > > I don't mind adding a comment, but the above is rather self explanatory > (one can easily see that it is set in mark_rodata_ro() with a simple > search). > > If a comment is to be added, something a bit more descriptive of the > functionality of the variable would be appropriate: > > /* > * Ignore modifying kernel text permissions until the kernel core calls > * make_rodata_ro() at system start up. > */ > > I can resend with the comment, or whoever takes this could add it > themselves. Gentle ping: this patch doesn't seem to have landed in upstream trees yet. Is any more work required? It would be nice to have this fix added. Just tested next-20171205 on RPi B+, it oopses when the function tracer is enabled during boot. next-20171205 plus this patch boots up fine. so long, Hias > > -- Steve > > > > > > Otherwise: > > > > Acked-by: Kees Cook <keescook@chromium.org> > > > > > + > > > void mark_rodata_ro(void) > > > { > > > + kernel_set_to_readonly = 1; > > > + > > > stop_machine(__mark_rodata_ro, NULL, NULL); > > > } > > > > > > void set_kernel_text_rw(void) > > > { > > > + if (!kernel_set_to_readonly) > > > + return; > > > + > > > set_section_perms(ro_perms, ARRAY_SIZE(ro_perms), false, > > > current->active_mm); > > > } > > > > > > void set_kernel_text_ro(void) > > > { > > > + if (!kernel_set_to_readonly) > > > + return; > > > + > > > set_section_perms(ro_perms, ARRAY_SIZE(ro_perms), true, > > > current->active_mm); > > > } > > > > Does arm64 suffer from a similar condition? (It looks like no, as text > > patching is done with a fixmap poke.) > > > > -Kees > > >
WARNING: multiple messages have this Message-ID (diff)
From: hias@horus.com (Matthias Reichl) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH] Arm: mm: ftrace: Only set text back to ro after kernel has been marked ro Date: Tue, 5 Dec 2017 12:47:09 +0100 [thread overview] Message-ID: <20171205114709.f6aj6i426keq2cn5@camel2.lan> (raw) In-Reply-To: <20170823150351.606ba09f@gandalf.local.home> On Wed, Aug 23, 2017 at 03:03:51PM -0400, Steven Rostedt wrote: > On Wed, 23 Aug 2017 11:48:13 -0700 > Kees Cook <keescook@chromium.org> wrote: > > > > diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c > > > index ad80548..fd75f38 100644 > > > --- a/arch/arm/mm/init.c > > > +++ b/arch/arm/mm/init.c > > > @@ -745,19 +745,29 @@ static int __mark_rodata_ro(void *unused) > > > return 0; > > > } > > > > > > +static int kernel_set_to_readonly; > > > > Adding a comment here might be a good idea, something like: > > > > /* Has system boot-up reached mark_rodata_ro() yet? */ > > I don't mind adding a comment, but the above is rather self explanatory > (one can easily see that it is set in mark_rodata_ro() with a simple > search). > > If a comment is to be added, something a bit more descriptive of the > functionality of the variable would be appropriate: > > /* > * Ignore modifying kernel text permissions until the kernel core calls > * make_rodata_ro() at system start up. > */ > > I can resend with the comment, or whoever takes this could add it > themselves. Gentle ping: this patch doesn't seem to have landed in upstream trees yet. Is any more work required? It would be nice to have this fix added. Just tested next-20171205 on RPi B+, it oopses when the function tracer is enabled during boot. next-20171205 plus this patch boots up fine. so long, Hias > > -- Steve > > > > > > Otherwise: > > > > Acked-by: Kees Cook <keescook@chromium.org> > > > > > + > > > void mark_rodata_ro(void) > > > { > > > + kernel_set_to_readonly = 1; > > > + > > > stop_machine(__mark_rodata_ro, NULL, NULL); > > > } > > > > > > void set_kernel_text_rw(void) > > > { > > > + if (!kernel_set_to_readonly) > > > + return; > > > + > > > set_section_perms(ro_perms, ARRAY_SIZE(ro_perms), false, > > > current->active_mm); > > > } > > > > > > void set_kernel_text_ro(void) > > > { > > > + if (!kernel_set_to_readonly) > > > + return; > > > + > > > set_section_perms(ro_perms, ARRAY_SIZE(ro_perms), true, > > > current->active_mm); > > > } > > > > Does arm64 suffer from a similar condition? (It looks like no, as text > > patching is done with a fixmap poke.) > > > > -Kees > > >
next prev parent reply other threads:[~2017-12-05 11:47 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-08-23 17:58 [PATCH] Arm: mm: ftrace: Only set text back to ro after kernel has been marked ro Steven Rostedt 2017-08-23 17:58 ` Steven Rostedt 2017-08-23 18:20 ` Matthias Reichl 2017-08-23 18:20 ` Matthias Reichl 2017-08-23 18:48 ` Kees Cook 2017-08-23 18:48 ` Kees Cook 2017-08-23 19:03 ` Steven Rostedt 2017-08-23 19:03 ` Steven Rostedt 2017-12-05 11:47 ` Matthias Reichl [this message] 2017-12-05 11:47 ` Matthias Reichl 2017-12-05 13:14 ` Russell King - ARM Linux 2017-12-05 13:14 ` Russell King - ARM Linux 2017-12-05 13:23 ` Matthias Reichl 2017-12-05 13:23 ` Matthias Reichl 2017-12-05 13:30 ` Phil Elwell 2017-12-05 13:30 ` Phil Elwell 2017-12-05 13:36 ` Russell King - ARM Linux 2017-12-05 13:36 ` Russell King - ARM Linux 2017-12-05 19:35 ` Kees Cook 2017-12-05 19:35 ` Kees Cook 2017-12-05 20:09 ` Russell King - ARM Linux 2017-12-05 20:09 ` Russell King - ARM Linux 2017-12-05 20:14 ` Kees Cook 2017-12-05 20:14 ` Kees Cook 2018-06-29 14:47 ` Matthias Reichl 2018-06-29 14:47 ` Matthias Reichl 2018-06-29 15:16 ` Steven Rostedt 2018-06-29 15:16 ` Steven Rostedt 2018-06-29 17:29 ` Matthias Reichl 2018-06-29 17:29 ` Matthias Reichl
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20171205114709.f6aj6i426keq2cn5@camel2.lan \ --to=hias@horus.com \ --cc=eric@anholt.net \ --cc=keescook@chromium.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-rpi-kernel@lists.infradead.org \ --cc=linux@armlinux.org.uk \ --cc=phil@raspberrypi.org \ --cc=rostedt@goodmis.org \ --cc=stefan.wahren@i2se.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.