Message-ID: <14506678-918f-81e1-2c26-2b347ff50701@intel.com>
Date:   Wed, 25 Jan 2023 07:29:39 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH] x86: enable Data Operand Independent Timing Mode
Content-Language: en-US
To:     Eric Biggers <ebiggers@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "H . Peter Anvin" <hpa@zytor.com>, x86@kernel.org
Cc:     linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-hardening@vger.kernel.org,
        Peter Zijlstra <peterz@infradead.org>,
        Roxana Bradescu <roxabee@chromium.org>,
        Adam Langley <agl@google.com>,
        Ard Biesheuvel <ardb@kernel.org>,
        "Jason A . Donenfeld" <Jason@zx2c4.com>
References: <20230125012801.362496-1-ebiggers@kernel.org>
From:   Dave Hansen <dave.hansen@intel.com>
In-Reply-To: <20230125012801.362496-1-ebiggers@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 1/24/23 17:28, Eric Biggers wrote:
> To mitigate this CPU vulnerability, it's possible to enable "Data
> Operand Independent Timing Mode" (DOITM) by setting a bit in a MSR.
> While Intel's documentation suggests that this bit should only be set
> where "necessary", that is highly impractical, given the fact that
> cryptography can happen nearly anywhere in the kernel and userspace, and
> the fact that the entire kernel likely needs to be protected anyway.

I think this misses a key point from the documentation:

	This functionality is intended for use by software which has
	already applied other techniques to mitigate software timing
	side channels, such as those documented in Intel's Guidelines
	for Mitigating Timing Side Channels Against Cryptographic
	Implementations.

Translating from Intel-speak: Intel thinks that DOITM purely a way to
make the CPU run slower if you haven't already written code specifically
to mitigate timing side channels.  All pain, no gain.

The kernel as a whole is not written that way.  I'm sure the crypto
folks that are cc'd can tell us specifically if the kernel crypto code
is written following those recommendations.

So, let's call this patch what it is: a potential global slowdown which
protects a very small amount of crypto code, probably just in userspace.
 That is probably the code that's generating your RSA keys, so it's
quite important, but it's also a _very_ small total amount of code.

There's another part here which I think was recently added to the
documentation:

	Intel expects the performance impact of this mode may be
	significantly higher on future processors. 

That's _meant_ to be really scary and keep folks from turning this on by
default, aka. what this patch does.  Your new CPU will be really slow if
you turn this on!  Boo!

All that said, and given the information that Intel has released, I
think this patch is generally the right thing to do.  I don't think
people are wrong for looking at "DODT" as being a new vulnerability.
Intel obviously doesn't see it that way, which is why "DODT" has (as far
as I can tell) not been treated with the same security pomp and
circumstance as other stuff.

Last, if you're going to propose that this be turned on, I expect to see
at least _some_ performance data.  DOITM=1 isn't free, even on Ice Lake.