From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com> To: Mark Rutland <mark.rutland@arm.com> Cc: linux-arm-kernel@lists.infradead.org, Mark Brown <broonie@kernel.org>, Julien Thierry <jthierry@redhat.com>, jpoimboe@redhat.com, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: Live patching on ARM64 Date: Tue, 26 Jan 2021 12:03:31 -0600 [thread overview] Message-ID: <a3393eb3-03a5-e4dd-f40c-b801cc60778e@linux.microsoft.com> (raw) In-Reply-To: <20210115123347.GB39776@C02TD0UTHF1T.local> Hi all, Mark Rutland had sent me some ideas on what work is pending for ARM64 live patching. I sent some questions to Mark Rutland. I forgot to include everyone in the email. Sorry about that. I have reproduced my questions and his responses below. Please chime in with any comments: Thanks! On Mon, Jan 25, 2021 at 11:58:47AM -0600, Madhavan T. Venkataraman wrote: > Some questions below: I've answered thos below. If possible, I'd prefer to handle future queries on a public list (so that others can chime in, and so that it gets archived), so if you could direct further questions to a thread on LAKML, that would be much appreciated. > On 1/15/21 6:33 AM, Mark Rutland wrote: > [...] >> >> One general thing that I believe we'll need to do is to rework code to >> be patch-safe (which implies being noinstr-safe too). For example, we'll >> need to rework the instruction patching code such that this cannot end >> up patching itself (or anything that has instrumented it) in an unsafe >> way. >> > > OK. I understand that. Are there are other scenarios that make patching > unsafe? I suspect so; these are simply the cases I'm immediately aware of. I suspect there are other cases that we will need to consider that don't immediately spring to mind. > I expect the kernel already handles scenarios such as two CPUs patching > the same location at the same time or a thread executing at a location that is > currently being patched. IIRC that is supposed to be catered for by ftrace (and so I assume for livepatching too); I'm not certain about kprobes. In addition to synchronization in the core ftrace code, arm64's ftrace_modify_code() has a sanity-check with a non-atomic RMW sequence. We might be able to make that more robust wiuth a faultable cmpxchg, and some changes around ftrace_update_ftrace_func() and ftrace_make_nop() to get rid of the unvalidated cases. > Any other scenarios to be considered? I'm not immediately aware of others, but suspect more cases will become apparent as work progresses on the bits we already know about. >> Once we have objtool it should be possible to identify those cases >> automatically. Currently I'm aware that we'll need to do something in at >> least the following places: >> > > OK. AFAIK, objtool checks for the following: > > - returning from noinstr function with instrumentation enabled > > - calling instrumentable functions from noinstr code without: > > instrumentation_begin(); > instrumentation_end(); > > Is that what you mean? That's what I was thinking of, yes -- this should highlight some places that will need attention. > Does objtool check other things as well that is relevant to (un)safe > patching? I'm not entirely familiar with objtool, so I'm not exactly sure what it can do; I expect Josh and Julien can give more detail here. >> * The insn framework (which is used by some patching code), since the >> bulk of it lives in arch/arm64/kernel/insn.c and isn't marked noinstr. >> >> We can probably shift the bulk of the aarch64_insn_gen_*() and >> aarch64_get_*() helpers into a header as __always_inline functions, >> which would allow them to be used in noinstr code. As those are >> typically invoked with a number of constant arguments that the >> compiler can fold, this /might/ work out as an optimization if the >> compiler can elide the error paths. > > OK. I will take a look at the insn code. IIRC Julien's objtool series had some patches had some patches moving the insn code about, so it'd be worth checking whether that's a help or a hindrance. If it's possible to split out a set of preparatory patches that make that ready both for objtool and the kernel, that would make it easier to review that and queue it early. >> * The alternatives code, since we call instrumentable and patchable >> functions between updating instructions and performing all the >> necessary maintenance. There are a number of cases within >> __apply_alternatives(), e.g. >> >> - test_bit() >> - cpus_have_cap() >> - pr_info_once() >> - lm_alias() >> - alt_cb, if the callback is not marked as noinstr, or if it calls >> instrumentable code (e.g. from the insn framework). >> - clean_dcache_range_nopatch(), as read_sanitised_ftr_reg() and >> related code can be instrumented. >> >> This might need some underlying rework elsewhere (e.g. in the >> cpufeature code, or atomics framework). >> >> So on the kernel side, maybe a first step would be to try to headerize >> the insn generation code as __always_inline, and see whether that looks >> ok? With that out of the way it'd be a bit easier to rework patching >> code depending on the insn framework. > > OK. I will study this. Great, thanks! Mark.
WARNING: multiple messages have this Message-ID (diff)
From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com> To: Mark Rutland <mark.rutland@arm.com> Cc: Julien Thierry <jthierry@redhat.com>, linux-kernel@vger.kernel.org, Mark Brown <broonie@kernel.org>, jpoimboe@redhat.com, live-patching@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: Live patching on ARM64 Date: Tue, 26 Jan 2021 12:03:31 -0600 [thread overview] Message-ID: <a3393eb3-03a5-e4dd-f40c-b801cc60778e@linux.microsoft.com> (raw) In-Reply-To: <20210115123347.GB39776@C02TD0UTHF1T.local> Hi all, Mark Rutland had sent me some ideas on what work is pending for ARM64 live patching. I sent some questions to Mark Rutland. I forgot to include everyone in the email. Sorry about that. I have reproduced my questions and his responses below. Please chime in with any comments: Thanks! On Mon, Jan 25, 2021 at 11:58:47AM -0600, Madhavan T. Venkataraman wrote: > Some questions below: I've answered thos below. If possible, I'd prefer to handle future queries on a public list (so that others can chime in, and so that it gets archived), so if you could direct further questions to a thread on LAKML, that would be much appreciated. > On 1/15/21 6:33 AM, Mark Rutland wrote: > [...] >> >> One general thing that I believe we'll need to do is to rework code to >> be patch-safe (which implies being noinstr-safe too). For example, we'll >> need to rework the instruction patching code such that this cannot end >> up patching itself (or anything that has instrumented it) in an unsafe >> way. >> > > OK. I understand that. Are there are other scenarios that make patching > unsafe? I suspect so; these are simply the cases I'm immediately aware of. I suspect there are other cases that we will need to consider that don't immediately spring to mind. > I expect the kernel already handles scenarios such as two CPUs patching > the same location at the same time or a thread executing at a location that is > currently being patched. IIRC that is supposed to be catered for by ftrace (and so I assume for livepatching too); I'm not certain about kprobes. In addition to synchronization in the core ftrace code, arm64's ftrace_modify_code() has a sanity-check with a non-atomic RMW sequence. We might be able to make that more robust wiuth a faultable cmpxchg, and some changes around ftrace_update_ftrace_func() and ftrace_make_nop() to get rid of the unvalidated cases. > Any other scenarios to be considered? I'm not immediately aware of others, but suspect more cases will become apparent as work progresses on the bits we already know about. >> Once we have objtool it should be possible to identify those cases >> automatically. Currently I'm aware that we'll need to do something in at >> least the following places: >> > > OK. AFAIK, objtool checks for the following: > > - returning from noinstr function with instrumentation enabled > > - calling instrumentable functions from noinstr code without: > > instrumentation_begin(); > instrumentation_end(); > > Is that what you mean? That's what I was thinking of, yes -- this should highlight some places that will need attention. > Does objtool check other things as well that is relevant to (un)safe > patching? I'm not entirely familiar with objtool, so I'm not exactly sure what it can do; I expect Josh and Julien can give more detail here. >> * The insn framework (which is used by some patching code), since the >> bulk of it lives in arch/arm64/kernel/insn.c and isn't marked noinstr. >> >> We can probably shift the bulk of the aarch64_insn_gen_*() and >> aarch64_get_*() helpers into a header as __always_inline functions, >> which would allow them to be used in noinstr code. As those are >> typically invoked with a number of constant arguments that the >> compiler can fold, this /might/ work out as an optimization if the >> compiler can elide the error paths. > > OK. I will take a look at the insn code. IIRC Julien's objtool series had some patches had some patches moving the insn code about, so it'd be worth checking whether that's a help or a hindrance. If it's possible to split out a set of preparatory patches that make that ready both for objtool and the kernel, that would make it easier to review that and queue it early. >> * The alternatives code, since we call instrumentable and patchable >> functions between updating instructions and performing all the >> necessary maintenance. There are a number of cases within >> __apply_alternatives(), e.g. >> >> - test_bit() >> - cpus_have_cap() >> - pr_info_once() >> - lm_alias() >> - alt_cb, if the callback is not marked as noinstr, or if it calls >> instrumentable code (e.g. from the insn framework). >> - clean_dcache_range_nopatch(), as read_sanitised_ftr_reg() and >> related code can be instrumented. >> >> This might need some underlying rework elsewhere (e.g. in the >> cpufeature code, or atomics framework). >> >> So on the kernel side, maybe a first step would be to try to headerize >> the insn generation code as __always_inline, and see whether that looks >> ok? With that out of the way it'd be a bit easier to rework patching >> code depending on the insn framework. > > OK. I will study this. Great, thanks! Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-01-27 10:09 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <f3fe6a60-9ac2-591d-1b83-9113c50dc492@linux.microsoft.com> 2021-01-14 23:58 ` Live patching on ARM64 Josh Poimboeuf 2021-01-14 23:58 ` Josh Poimboeuf 2021-01-15 12:33 ` Mark Rutland 2021-01-15 12:33 ` Mark Rutland 2021-01-15 13:44 ` Mark Brown 2021-01-15 13:44 ` Mark Brown 2021-01-17 17:25 ` Madhavan T. Venkataraman 2021-01-17 17:25 ` Madhavan T. Venkataraman 2021-01-19 7:57 ` Julien Thierry 2021-01-19 7:57 ` Julien Thierry 2021-01-19 15:19 ` Madhavan T. Venkataraman 2021-01-19 15:19 ` Madhavan T. Venkataraman 2021-01-20 18:11 ` Julien Thierry 2021-01-20 18:11 ` Julien Thierry 2021-01-26 18:03 ` Madhavan T. Venkataraman [this message] 2021-01-26 18:03 ` Madhavan T. Venkataraman 2021-03-18 22:38 ` Singh, Balbir 2021-03-18 22:38 ` Singh, Balbir
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=a3393eb3-03a5-e4dd-f40c-b801cc60778e@linux.microsoft.com \ --to=madvenka@linux.microsoft.com \ --cc=broonie@kernel.org \ --cc=jpoimboe@redhat.com \ --cc=jthierry@redhat.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=live-patching@vger.kernel.org \ --cc=mark.rutland@arm.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.