* [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code @ 2019-04-04 20:26 Hook, Gary 2019-04-04 20:42 ` Thomas Gleixner 0 siblings, 1 reply; 12+ messages in thread From: Hook, Gary @ 2019-04-04 20:26 UTC (permalink / raw) To: linux-kernel; +Cc: dave.hansen, peterz, x86, mingo, bp, luto, tglx Enablement of AMD's Secure Memory Encryption feature is determined very early in the boot cycle. Part of this procedure involves scanning the command line for the paramater 'mem_encrypt'. To determine intended state, the function sme_enable() uses library functions cmdline_find_option() and strncmp(). Their use occurs early enough such that we can't assume that any instrumentation subsystem is initialized. For example, making calls to a KASAN-instrumented function before KASAN is set up will likely result in the use of uninitialized memory and a boot failure. Avoid instrumenting these dependent functions by: 1) Making a local, static, renamed copy of strncpy() for use solely in mem_encrypt_identity.c. In this file we are able to vet its few uses and avoid exposing the rest of the kernel to a ubiquitously used but un-instrumented function. 2) Disable instrumention of arch/x86/lib/cmdline.c based on the assumption that the needed function (cmdline_find_option()) is vetted through its use to date, and contains no lurking flaws that have not yet been found through instrumentation such as KASAN. Fixes: aca20d546214 ("x86/mm: Add support to make use of Secure Memory Encryption") Reported-by: Li RongQing <lirongqing@baidu.com> Signed-off-by: Gary R Hook <gary.hook@amd.com> --- arch/x86/lib/Makefile | 13 +++++++++++++ arch/x86/mm/mem_encrypt_identity.c | 26 ++++++++++++++++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile index 140e61843a07..38182a64fa81 100644 --- a/arch/x86/lib/Makefile +++ b/arch/x86/lib/Makefile @@ -6,6 +6,19 @@ # Produces uninteresting flaky coverage. KCOV_INSTRUMENT_delay.o := n +# SME early boot code checks the cmdline, so don't instrument +KCOV_INSTRUMENT_cmdline.o := n + +KASAN_SANITIZE_cmdline.o := n + +ifdef CONFIG_FUNCTION_TRACER +CFLAGS_REMOVE_cmdline.o = -pg +endif + +# No stack protector +nostackp := $(call cc-option, -fno-stack-protector) +CFLAGS_cmdline.o := $(nostackp) + inat_tables_script = $(srctree)/arch/x86/tools/gen-insn-attr-x86.awk inat_tables_maps = $(srctree)/arch/x86/lib/x86-opcode-map.txt quiet_cmd_inat_tables = GEN $@ diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c index 4aa9b1480866..0a68d7ccb371 100644 --- a/arch/x86/mm/mem_encrypt_identity.c +++ b/arch/x86/mm/mem_encrypt_identity.c @@ -77,6 +77,28 @@ static char sme_cmdline_arg[] __initdata = "mem_encrypt"; static char sme_cmdline_on[] __initdata = "on"; static char sme_cmdline_off[] __initdata = "off"; +/* + * Local copy to avoid instrumentation + * Copied from lib/string.c and renamed to be unique. + * This file is early boot code, and we assume that instrumentation + * subsystems (e.g. KASAN) are not yet initialized. + */ +static int sme_strncmp(const char *cs, const char *ct, size_t count) +{ + unsigned char c1, c2; + + while (count) { + c1 = *cs++; + c2 = *ct++; + if (c1 != c2) + return c1 < c2 ? -1 : 1; + if (!c1) + break; + count--; + } + return 0; +} + static void __init sme_clear_pgd(struct sme_populate_pgd_data *ppd) { unsigned long pgd_start, pgd_end, pgd_size; @@ -557,9 +579,9 @@ void __init sme_enable(struct boot_params *bp) cmdline_find_option(cmdline_ptr, cmdline_arg, buffer, sizeof(buffer)); - if (!strncmp(buffer, cmdline_on, sizeof(buffer))) + if (!sme_strncmp(buffer, cmdline_on, sizeof(buffer))) sme_me_mask = me_mask; - else if (!strncmp(buffer, cmdline_off, sizeof(buffer))) + else if (!sme_strncmp(buffer, cmdline_off, sizeof(buffer))) sme_me_mask = 0; else sme_me_mask = active_by_default ? me_mask : 0; ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code 2019-04-04 20:26 [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code Hook, Gary @ 2019-04-04 20:42 ` Thomas Gleixner 2019-04-08 16:46 ` Gary R Hook 0 siblings, 1 reply; 12+ messages in thread From: Thomas Gleixner @ 2019-04-04 20:42 UTC (permalink / raw) To: Hook, Gary Cc: linux-kernel, dave.hansen, peterz, x86, mingo, bp, luto, Alexander Potapenko On Thu, 4 Apr 2019, Hook, Gary wrote: > Enablement of AMD's Secure Memory Encryption feature is determined > very early in the boot cycle. Part of this procedure involves scanning > the command line for the paramater 'mem_encrypt'. > > To determine intended state, the function sme_enable() uses library > functions cmdline_find_option() and strncmp(). Their use occurs early > enough such that we can't assume that any instrumentation subsystem is > initialized. For example, making calls to a KASAN-instrumented > function before KASAN is set up will likely result in the use of > uninitialized memory and a boot failure. > > Avoid instrumenting these dependent functions by: > > 1) Making a local, static, renamed copy of strncpy() for use solely in > mem_encrypt_identity.c. In this file we are able to vet its few uses > and avoid exposing the rest of the kernel to a ubiquitously used but > un-instrumented function. > > 2) Disable instrumention of arch/x86/lib/cmdline.c based on the > assumption that the needed function (cmdline_find_option()) is vetted > through its use to date, and contains no lurking flaws that have not > yet been found through instrumentation such as KASAN. Not happy about that :) > +# SME early boot code checks the cmdline, so don't instrument > +KCOV_INSTRUMENT_cmdline.o := n > + > +KASAN_SANITIZE_cmdline.o := n If we can't come up with a better solution then this needs to depend on CONFIG_MEM_ENCRYPT so we still can run KASAN on cmdline.c to catch crap when the code is modified in the future. Thanks, tglx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code 2019-04-04 20:42 ` Thomas Gleixner @ 2019-04-08 16:46 ` Gary R Hook 2019-04-08 16:58 ` Borislav Petkov 0 siblings, 1 reply; 12+ messages in thread From: Gary R Hook @ 2019-04-08 16:46 UTC (permalink / raw) To: Thomas Gleixner, Hook, Gary Cc: linux-kernel, dave.hansen, peterz, x86, mingo, bp, luto, Alexander Potapenko On 4/4/19 3:42 PM, Thomas Gleixner wrote: > On Thu, 4 Apr 2019, Hook, Gary wrote: > >> Enablement of AMD's Secure Memory Encryption feature is determined >> very early in the boot cycle. Part of this procedure involves scanning >> the command line for the paramater 'mem_encrypt'. >> >> To determine intended state, the function sme_enable() uses library >> functions cmdline_find_option() and strncmp(). Their use occurs early >> enough such that we can't assume that any instrumentation subsystem is >> initialized. For example, making calls to a KASAN-instrumented >> function before KASAN is set up will likely result in the use of >> uninitialized memory and a boot failure. >> >> Avoid instrumenting these dependent functions by: >> >> 1) Making a local, static, renamed copy of strncpy() for use solely in >> mem_encrypt_identity.c. In this file we are able to vet its few uses >> and avoid exposing the rest of the kernel to a ubiquitously used but >> un-instrumented function. >> >> 2) Disable instrumention of arch/x86/lib/cmdline.c based on the >> assumption that the needed function (cmdline_find_option()) is vetted >> through its use to date, and contains no lurking flaws that have not >> yet been found through instrumentation such as KASAN. > > Not happy about that :) My reasoning (not arguing): the file has been touched exactly one time in 4 years, by Thomas. Doesn't appear to be a candidate for constant modification, so this approach doesn't seem risky to me. I could be wrong. >> +# SME early boot code checks the cmdline, so don't instrument >> +KCOV_INSTRUMENT_cmdline.o := n >> + >> +KASAN_SANITIZE_cmdline.o := n > > If we can't come up with a better solution then this needs to depend on > CONFIG_MEM_ENCRYPT so we still can run KASAN on cmdline.c to catch crap > when the code is modified in the future. We have considered this problem, and see no alternative solution. But I would love to come up with one. In the meantime, I've already created a v2 patch that fulfills your request. I'll wait a few more days for comments before posting. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code 2019-04-08 16:46 ` Gary R Hook @ 2019-04-08 16:58 ` Borislav Petkov 2019-04-08 18:41 ` Gary R Hook 0 siblings, 1 reply; 12+ messages in thread From: Borislav Petkov @ 2019-04-08 16:58 UTC (permalink / raw) To: Gary R Hook Cc: Thomas Gleixner, Hook, Gary, linux-kernel, dave.hansen, peterz, x86, mingo, luto, Alexander Potapenko On Mon, Apr 08, 2019 at 04:46:31PM +0000, Gary R Hook wrote: > My reasoning (not arguing): the file has been touched exactly one time > in 4 years, by Thomas. Doesn't appear to be a candidate for constant > modification, so this approach doesn't seem risky to me. I could be wrong. The problem, like we discussed it with Tom offlist, is that you simply cannot turn off instrumentation for those generic files just because SME has trouble with them, and that last thing can be any vendor-specific feature. Even if the functions there are "trivial" now (doesn't mean new stuff won't be added there in the future and we forget about the disabled instrumentation here.) We simply cannot constrain generic compilation units like that. So the functions there either need to be copied or ifdeffed. At the time SME was going in, the intention was to reuse code so that there is less duplication. But if there's trouble doing that sharing, then we need to "unshare" it again. Or come up with something else slick and clean. Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code 2019-04-08 16:58 ` Borislav Petkov @ 2019-04-08 18:41 ` Gary R Hook 2019-04-08 19:08 ` Borislav Petkov 0 siblings, 1 reply; 12+ messages in thread From: Gary R Hook @ 2019-04-08 18:41 UTC (permalink / raw) To: Borislav Petkov Cc: Thomas Gleixner, Hook, Gary, linux-kernel, dave.hansen, peterz, x86, mingo, luto, Alexander Potapenko On 4/8/19 11:58 AM, Borislav Petkov wrote: > On Mon, Apr 08, 2019 at 04:46:31PM +0000, Gary R Hook wrote: >> My reasoning (not arguing): the file has been touched exactly one time >> in 4 years, by Thomas. Doesn't appear to be a candidate for constant >> modification, so this approach doesn't seem risky to me. I could be wrong. > > The problem, like we discussed it with Tom offlist, is that you simply > cannot turn off instrumentation for those generic files just because SME > has trouble with them, and that last thing can be any vendor-specific > feature. Again, not arguing. I completely understand. However, to be fair, this isn't about SME having trouble with those facilities, this is about using certain features (e.g. command line option processing) early in the boot. Any complex feature could have had that requirement, don't you think? > Even if the functions there are "trivial" now (doesn't mean new stuff > won't be added there in the future and we forget about the disabled > instrumentation here.) Absolutely agree. Recognizing that things can be forgotten has weighed heavily on me in light of this. I don't like that possibility at all. > We simply cannot constrain generic compilation units like that. So the > functions there either need to be copied or ifdeffed. At the time SME > was going in, the intention was to reuse code so that there is less > duplication. But if there's trouble doing that sharing, then we need to > "unshare" it again. Or come up with something else slick and clean. Right. My goal was to get a conversation started, because folks are running into this problem when KASAN is enabled. N.B. Here's another facet of this problem: cmdline.c doesn't (today) contain anything that would trigger the stack protector. However, it's possible to enable the stack protector globally when building, right? In which case, a boot would fail, so we have the same issue: early boot code has special requirements / restrictions. I'm not sure what the "right" solution is, but I appreciate your time and expertise to discuss. And now I have another idea I'm going to go try out. grh ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code 2019-04-08 18:41 ` Gary R Hook @ 2019-04-08 19:08 ` Borislav Petkov 2019-04-09 13:47 ` Lendacky, Thomas 2019-04-26 15:11 ` Gary R Hook 0 siblings, 2 replies; 12+ messages in thread From: Borislav Petkov @ 2019-04-08 19:08 UTC (permalink / raw) To: Gary R Hook Cc: Thomas Gleixner, Hook, Gary, linux-kernel, dave.hansen, peterz, x86, mingo, luto, Alexander Potapenko On Mon, Apr 08, 2019 at 06:41:30PM +0000, Gary R Hook wrote: > Again, not arguing. I completely understand. However, to be fair, this > isn't about SME having trouble with those facilities, this is about > using certain features (e.g. command line option processing) early in > the boot. Any complex feature could have had that requirement, don't you > think? Sure, but then why do we need that patch at all then? Why do we need to disable instrumentation for SME early code and not for other early code? I mean, if you grep around the tree you can see a bunch of KASAN_SANITIZE but in lib/ we only have lib/Makefile:210:KASAN_SANITIZE_stackdepot.o := n which is special. But the rest of the generic code in lib/ or arch/x86/lib/ isn't. Now, there's this: arch/x86/boot/Makefile:12:KASAN_SANITIZE := n arch/x86/boot/compressed/Makefile:20:KASAN_SANITIZE := n which disables KASAN for all boot code. And this is what you mean - all early boot code should not be sanitized. Which also gives the right solution, IMO: cmdline.o should not be sanitized only when used in the boot code. But that is already the case. So why do you need to disable KASAN for arch/x86/lib/cmdline.c? Because for those two: arch/x86/boot/cmdline.c arch/x86/boot/compressed/cmdline.c that should already be the case due to the Makefile defines above. > Right. My goal was to get a conversation started, because folks are > running into this problem when KASAN is enabled. You say KASAN. Why is there KCOV_INSTRUMENT_cmdline.o too? > N.B. Here's another facet of this problem: cmdline.c doesn't (today) > contain anything that would trigger the stack protector. However, it's > possible to enable the stack protector globally when building, right? In > which case, a boot would fail, so we have the same issue: early boot > code has special requirements / restrictions. How so? This .config boots here in a vm just fine. $ grep STACKPROT .config CONFIG_CC_HAS_SANE_STACKPROTECTOR=y CONFIG_HAVE_STACKPROTECTOR=y CONFIG_CC_HAS_STACKPROTECTOR_NONE=y CONFIG_STACKPROTECTOR=y CONFIG_STACKPROTECTOR_STRONG=y -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code 2019-04-08 19:08 ` Borislav Petkov @ 2019-04-09 13:47 ` Lendacky, Thomas 2019-04-26 15:11 ` Gary R Hook 1 sibling, 0 replies; 12+ messages in thread From: Lendacky, Thomas @ 2019-04-09 13:47 UTC (permalink / raw) To: Borislav Petkov, Hook, Gary Cc: Thomas Gleixner, Hook, Gary, linux-kernel, dave.hansen, peterz, x86, mingo, luto, Alexander Potapenko On 4/8/19 2:08 PM, Borislav Petkov wrote: > On Mon, Apr 08, 2019 at 06:41:30PM +0000, Gary R Hook wrote: >> Again, not arguing. I completely understand. However, to be fair, this >> isn't about SME having trouble with those facilities, this is about >> using certain features (e.g. command line option processing) early in >> the boot. Any complex feature could have had that requirement, don't you >> think? > > Sure, but then why do we need that patch at all then? Why do we need to > disable instrumentation for SME early code and not for other early code? > > I mean, if you grep around the tree you can see a bunch of > KASAN_SANITIZE but in lib/ we only have > > lib/Makefile:210:KASAN_SANITIZE_stackdepot.o := n > > which is special. But the rest of the generic code in lib/ or > arch/x86/lib/ isn't. > > Now, there's this: > > arch/x86/boot/Makefile:12:KASAN_SANITIZE := n > arch/x86/boot/compressed/Makefile:20:KASAN_SANITIZE := n > > which disables KASAN for all boot code. > > And this is what you mean - all early boot code should not be sanitized. > > Which also gives the right solution, IMO: > > cmdline.o should not be sanitized only when used in the boot code. But > that is already the case. > > So why do you need to disable KASAN for arch/x86/lib/cmdline.c? > > Because for those two: > > arch/x86/boot/cmdline.c > arch/x86/boot/compressed/cmdline.c > > that should already be the case due to the Makefile defines above. This isn't the code in arch/x86/boot. This is post decompression phase and part of the actual kernel early boot code where the page tables are being modified with the encryption mask and the actual kernel load address (see the sme_enable() call from __startup_64() in arch/x86/kernel/head64.c). > >> Right. My goal was to get a conversation started, because folks are >> running into this problem when KASAN is enabled. > > You say KASAN. Why is there KCOV_INSTRUMENT_cmdline.o too? > >> N.B. Here's another facet of this problem: cmdline.c doesn't (today) >> contain anything that would trigger the stack protector. However, it's >> possible to enable the stack protector globally when building, right? In >> which case, a boot would fail, so we have the same issue: early boot >> code has special requirements / restrictions. > > How so? > > This .config boots here in a vm just fine. The SME path that this addresses is bypassed when you boot in a VM. The sme_enable() code detects that it is executing under a hypervisor and does not check the command line or do a string compare. Thanks, Tom > > $ grep STACKPROT .config > CONFIG_CC_HAS_SANE_STACKPROTECTOR=y > CONFIG_HAVE_STACKPROTECTOR=y > CONFIG_CC_HAS_STACKPROTECTOR_NONE=y > CONFIG_STACKPROTECTOR=y > CONFIG_STACKPROTECTOR_STRONG=y > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code 2019-04-08 19:08 ` Borislav Petkov 2019-04-09 13:47 ` Lendacky, Thomas @ 2019-04-26 15:11 ` Gary R Hook 2019-04-26 16:24 ` Borislav Petkov 1 sibling, 1 reply; 12+ messages in thread From: Gary R Hook @ 2019-04-26 15:11 UTC (permalink / raw) To: Borislav Petkov Cc: Thomas Gleixner, Hook, Gary, linux-kernel, dave.hansen, peterz, x86, mingo, luto, Alexander Potapenko On 4/8/19 2:08 PM, Borislav Petkov wrote:On 5/8/19 2:08 PM, Borislav Petkov wrote:> On Mon, Apr 08, 2019 at 06:41:30PM +0000, Gary R Hook wrote: >> Again, not arguing. I completely understand. However, to be fair, this >> isn't about SME having trouble with those facilities, this is about >> using certain features (e.g. command line option processing) early in >> the boot. Any complex feature could have had that requirement, don't you >> think? > > Sure, but then why do we need that patch at all then? Why do we need to > disable instrumentation for SME early code and not for other early code? > > I mean, if you grep around the tree you can see a bunch of > KASAN_SANITIZE but in lib/ we only have > > lib/Makefile:210:KASAN_SANITIZE_stackdepot.o := n > > which is special. But the rest of the generic code in lib/ or > arch/x86/lib/ isn't. > > Now, there's this: > > arch/x86/boot/Makefile:12:KASAN_SANITIZE := n > arch/x86/boot/compressed/Makefile:20:KASAN_SANITIZE := n > > which disables KASAN for all boot code. > > And this is what you mean - all early boot code should not be sanitized. > > Which also gives the right solution, IMO: > > cmdline.o should not be sanitized only when used in the boot code. But > that is already the case. > > So why do you need to disable KASAN for arch/x86/lib/cmdline.c? > > Because for those two: > > arch/x86/boot/cmdline.c > arch/x86/boot/compressed/cmdline.c > > that should already be the case due to the Makefile defines above. Except that we're not talking about that code. I probably should have defined terms, so please allow me to back up. When I say "early boot" I meant what happens -after- decompression, when the kernel proper has been laid out in memory and starts to run. This is -after- the boot code has been executed, which means that the cmdline.c to which you refer above is no longer extant in memory. If my usage of the term "early boot" is a misnomer, I can only apologize. And ask what term is in common use to describe what is happening at that point in time. Since, for this discussion, we're already in start_kernel(), the only cmdline.c available is the one in arch/x86/lib. That's that one that is instrumented by KASAN, and the one that is causing problems in this scenario. The strncmp(), too. >> Right. My goal was to get a conversation started, because folks are >> running into this problem when KASAN is enabled. > > You say KASAN. Why is there KCOV_INSTRUMENT_cmdline.o too? I don't care if KCOV_INSTRUMENT is enabled or not, but it's disabled for arch/x86/mm/mem_encrypt_identity.c, so it seems reasonable that it should be disable for this file, too, in the context of resolving this problem. To be more precise, the change is for "instrumentation", in general. >> N.B. Here's another facet of this problem: cmdline.c doesn't (today) >> contain anything that would trigger the stack protector. However, it's >> possible to enable the stack protector globally when building, right? In >> which case, a boot would fail, so we have the same issue: early boot >> code has special requirements / restrictions. > > How so? <sidebar> Makefile contains stackp-flags-$(CONFIG_STACKPROTECTOR) := -fstack-protector stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG) := -fstack-protector-strong This means that (as I understand it) stack protection is decided by the compiler, and is based on certain conditions in the code. This implies that not every function will necessarily be instrumented. However, if you decide to force the issue with something like stackp-flags-$(CONFIG_STACKPROTECTOR) := -fstack-protector-full stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG) := -fstack-protector-all Unless otherwise disabled, I believe this causes everything to be instrumented. Which results in a boot failure. (Actually, in my tests the system restarts after the decompression.) Note that intrumentation such as KASAN isn't involved here. And I figure that doing this is unsupported. It was just an interesting discovery. </sidebar> However, not relevant to the KASAN instrumentation problem. Recap: mem_encrypt_identity.c uses two common functions. The code in mem_encrypt_identity.c runs soon after start_kernel() is invoked. The SME feature command line parameter is searched for, and uses those two common functions. If instrumentation is enabled, it is applied to those to common functions (but not mem_encrypt_identity.c). But if support infrastructure for instrumentation is not initialized before the code in mem_encrypt_identity is invoked, the kernel fails to boot. After discussion over several weeks, we see the following options for a solution: 1) Create a local static copy of strncmp in mem_encrypt_identity.c 2) Turn off instrumentation for lib/cmdline.c. The risk is that any changes to its code would not enjoy the benefits of KASAN/etc testing (if enabled). 3) Make a local static copy of cmdline_find_option() inside of mem_encrypt_identity.c. 4) Use #defines and #include to have cmdline.c included within mem_encrypt_identity.c. This maintains a single source file and continues to allow the function to be instrumented for use everywhere elsewhere in the kernel. We believe (1) is a simple and effective choice, similar to inlining. If a single source file (for cmdline.c) is preferred, option (4) maintains that paradigm but gets the job done fairly cleanly. I understand if there is reticience about include source files, but it's not IMO an abhorrent practice. This also points to a single file as the origin of the required function, so no confusion ensues. I believe TGLX raised the issue of "what happens if we find a bug in cmdline_find_option()?" Despite the fact that the file has been changed only once in 4 years (by Thomas) it's an important consideration. Another reason I lean towards (4), above. I'm -hoping- this is more clear and succinct. How shall we proceed? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code 2019-04-26 15:11 ` Gary R Hook @ 2019-04-26 16:24 ` Borislav Petkov 2019-04-29 20:16 ` Gary R Hook 0 siblings, 1 reply; 12+ messages in thread From: Borislav Petkov @ 2019-04-26 16:24 UTC (permalink / raw) To: Gary R Hook Cc: Thomas Gleixner, Hook, Gary, linux-kernel, dave.hansen, peterz, x86, mingo, luto, Alexander Potapenko On Fri, Apr 26, 2019 at 03:11:17PM +0000, Gary R Hook wrote: > 2) Turn off instrumentation for lib/cmdline.c. The risk is that any > changes to its code would not enjoy the benefits of KASAN/etc testing > (if enabled). What happened to Thomas' suggestion to turn off instrumentation for those files only when CONFIG_AMD_MEM_ENCRYPT=y? Which is a variant of 2) above with ifdeffery. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code 2019-04-26 16:24 ` Borislav Petkov @ 2019-04-29 20:16 ` Gary R Hook 2019-04-29 20:51 ` Borislav Petkov 0 siblings, 1 reply; 12+ messages in thread From: Gary R Hook @ 2019-04-29 20:16 UTC (permalink / raw) To: Borislav Petkov Cc: Thomas Gleixner, Hook, Gary, linux-kernel, dave.hansen, peterz, x86, mingo, luto, Alexander Potapenko On 4/26/19 11:24 AM, Borislav Petkov wrote: > On Fri, Apr 26, 2019 at 03:11:17PM +0000, Gary R Hook wrote: >> 2) Turn off instrumentation for lib/cmdline.c. The risk is that any >> changes to its code would not enjoy the benefits of KASAN/etc testing >> (if enabled). > > What happened to Thomas' suggestion to turn off instrumentation for > those files only when CONFIG_AMD_MEM_ENCRYPT=y? > > Which is a variant of 2) above with ifdeffery. > Ah, very good. That one escaped my list. Yes, option 4 would be a combination of using a local copy of strncmp() and disabling instrumentation (KASAN, KCOV, whatever) for arch/x86/lib/cmdline.c when SME is enabled. I have any/all of these ready to repost as a version 2 offering. What say you? grh ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code 2019-04-29 20:16 ` Gary R Hook @ 2019-04-29 20:51 ` Borislav Petkov 2019-04-29 21:22 ` Gary R Hook 0 siblings, 1 reply; 12+ messages in thread From: Borislav Petkov @ 2019-04-29 20:51 UTC (permalink / raw) To: Gary R Hook Cc: Thomas Gleixner, Hook, Gary, linux-kernel, dave.hansen, peterz, x86, mingo, luto, Alexander Potapenko On Mon, Apr 29, 2019 at 08:16:07PM +0000, Gary R Hook wrote: > Yes, option 4 would be a combination of using a local copy of strncmp() Why the local copy? > and disabling instrumentation (KASAN, KCOV, whatever) for > arch/x86/lib/cmdline.c when SME is enabled. I think this should suffice. You only disable instrumentation when CONFIG_AMD_MEM_ENCRYPT=y and not do any local copies but use the generic functions. Hmm. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code 2019-04-29 20:51 ` Borislav Petkov @ 2019-04-29 21:22 ` Gary R Hook 0 siblings, 0 replies; 12+ messages in thread From: Gary R Hook @ 2019-04-29 21:22 UTC (permalink / raw) To: Borislav Petkov Cc: Thomas Gleixner, Hook, Gary, linux-kernel, dave.hansen, peterz, x86, mingo, luto, Alexander Potapenko On 4/29/19 3:51 PM, Borislav Petkov wrote: > [CAUTION: External Email] > > On Mon, Apr 29, 2019 at 08:16:07PM +0000, Gary R Hook wrote: >> Yes, option 4 would be a combination of using a local copy of strncmp() > > Why the local copy? Seemed suitable, since it's tiny. But I'm not married to the idea. > >> and disabling instrumentation (KASAN, KCOV, whatever) for >> arch/x86/lib/cmdline.c when SME is enabled. > > I think this should suffice. You only disable instrumentation when > CONFIG_AMD_MEM_ENCRYPT=y and not do any local copies but use the generic > functions. Okay, super. I'll post a v2 that does that. Do we want to document the subordinate functions in their respective source files so that a future editor would (hopefully) be made aware of this use? grh ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-04-29 21:22 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-04 20:26 [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code Hook, Gary 2019-04-04 20:42 ` Thomas Gleixner 2019-04-08 16:46 ` Gary R Hook 2019-04-08 16:58 ` Borislav Petkov 2019-04-08 18:41 ` Gary R Hook 2019-04-08 19:08 ` Borislav Petkov 2019-04-09 13:47 ` Lendacky, Thomas 2019-04-26 15:11 ` Gary R Hook 2019-04-26 16:24 ` Borislav Petkov 2019-04-29 20:16 ` Gary R Hook 2019-04-29 20:51 ` Borislav Petkov 2019-04-29 21:22 ` Gary R Hook
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.