Re: [PATCH] x86/mm/mem_encrypt: Disable all instrumentation for SME early boot code

From: Gary R Hook
Date: Mon Apr 08 2019 - 12:46:39 EST


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.