From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f202.google.com (mail-pg1-f202.google.com [209.85.215.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0B9FD1A0BD0 for ; Tue, 24 Feb 2026 01:27:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.202 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771896477; cv=none; b=TWA4swRb/SRGJJsXdiJQX5sc9OmhOtxz2Yh/T2C1EBX8SzAgjX7oAJFhvL0+8LfJKmtU0G2X2R4u72JMBY7lNysjj3aLqHWelbWyekXfN/yk8TE/TLjPkVWpXT19TvivvTgm1rcnO9OWclCgwYkz7x+6wZKnKkjRtwzmvlRJYh8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1771896477; c=relaxed/simple; bh=vvdEvLfoWawoL6K+kTA9bEko7R7Ro3WCOIidwUL60Mo=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=rL5Q4KtEDvwCApG4Wyx/LhLtH3PvtAtGKCuCAN0640wAFAPo/oWkGFXTBA4ATUzZd2Dr86XcX130LUFnyMHHq1ExMMmwAKhclrsqvhD1mRpinbCIJM78tHdAjlJVxGouPeumsmFz3RXO7RqSNjgATvmKC3m1sXflOh8cvGyE7js= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=jqwA3zQY; arc=none smtp.client-ip=209.85.215.202 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="jqwA3zQY" Received: by mail-pg1-f202.google.com with SMTP id 41be03b00d2f7-c6dde310601so2975260a12.1 for ; Mon, 23 Feb 2026 17:27:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1771896475; x=1772501275; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=imQRkcmOy4DPdumAym14O8Grk4dKcX7e7AP+6BT4u5Y=; b=jqwA3zQYt/wuT5YntfNvrapgTe/b6tzqNv0WGhLFaMVXR3JiRJ2anubQh49f0P8Gd5 f0kpUEWwwMXZzXzITGyjau64nQOVpT3zwJ7bywKuoEzuK+LypjSCmyDaVvNil1xbYg/q BOgvot4u9fVFsm+n4LzgIQkbz7eYjbK74OnAvL7s362NCgt6dTQUwctV2WtT1Db4Ouo6 6kJvbrkTNlRkwQGwn5BBt7VXDsbfS2pW7Ci6XgGBGZqCeiVKisBM9LP/D9gf3BECWDWy 4bmeTa9eArA9GK6gIIPtIX70iDblaFnkOHP2IJhIEYutbl4fmlrwnXfzpaVNZo5oUUts 3Bbg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1771896475; x=1772501275; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=imQRkcmOy4DPdumAym14O8Grk4dKcX7e7AP+6BT4u5Y=; b=PYZuaKJDA9mR/iTPn9XNSwMmd0FRUYkQTW8x4IKsP/NtNbZ6qC8D5jrnN42nZZ3VBy PqV90D8E1G0qmBtAdgWYUEXTcChoKw+5+swRG3vq1HZTK6MC87cRntmCXHOvrUc++Qfs D+z4ZTkU8GcG/h8l/G1SaA7GlyzRawSdXr7km/EGh5BmocIaF1eOjkOHqW36QTaCmufm 5coDQg5lGTWhDDnkMow3su7iWOUrRzMASnLVppO/EdOMH0Ltw6T64T3cVrrHFJ4KDH/Q khoGt3TMCFWZ8WFjPQxvTNx6iDPk7pzpcE7m7gdkAevLXOBnCqQWbguLaqFCHuuzzoN4 cjFg== X-Forwarded-Encrypted: i=1; AJvYcCXQN9tj7yS/Q8Ck++bhdP2Ckx0sitB4L79f412my3zO7MUEhlBGSvA/O55f5MDYn0W6OrOAMpV3DPQDsP0=@vger.kernel.org X-Gm-Message-State: AOJu0Yxb1qd9lb4BDNqMiD2sbvoVHj7x+qamPYKaBT8tHW2mzO8oEwSx cPupWnSeLpGhpv2MO7BwUKsRWIK1eibji7ItlU6umVVknGYJFKd0Uuc16oe5ZBSDo/JEorBfgjm NvuYW2g== X-Received: from pgbfp12.prod.google.com ([2002:a05:6a02:2cec:b0:c52:a841:c79d]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6a20:748b:b0:38b:e55a:97c9 with SMTP id adf61e73a8af0-39545ebb1b9mr8817920637.28.1771896475278; Mon, 23 Feb 2026 17:27:55 -0800 (PST) Date: Mon, 23 Feb 2026 17:27:54 -0800 In-Reply-To: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20260206190851.860662-1-yosry.ahmed@linux.dev> <20260206190851.860662-26-yosry.ahmed@linux.dev> Message-ID: Subject: Re: [PATCH v5 25/26] KVM: nSVM: Sanitize control fields copied from VMCB12 From: Sean Christopherson To: Yosry Ahmed Cc: Yosry Ahmed , Paolo Bonzini , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Jim Mattson Content-Type: text/plain; charset="us-ascii" On Mon, Feb 23, 2026, Yosry Ahmed wrote: > > > @@ -499,32 +499,35 @@ void __nested_copy_vmcb_control_to_cache(struct kvm_vcpu *vcpu, > > > if (!guest_cpu_cap_has(vcpu, X86_FEATURE_NPT)) > > > to->misc_ctl &= ~SVM_MISC_ENABLE_NP; > > > > > > - to->iopm_base_pa = from->iopm_base_pa; > > > - to->msrpm_base_pa = from->msrpm_base_pa; > > > + /* > > > + * Copy the ASID here because nested_vmcb_check_controls() will check > > > + * it. The ASID could be invalid, or conflict with another VM's ASID , > > > > Spurious space before the command. > > > > > + * so it should never be used directly to run L2. > > > + */ > > > + to->asid = from->asid; > > > + > > > + /* Lower bits of IOPM_BASE_PA and MSRPM_BASE_PA are ignored */ > > > + to->iopm_base_pa = from->iopm_base_pa & PAGE_MASK; > > > + to->msrpm_base_pa = from->msrpm_base_pa & PAGE_MASK; > > >> to->tsc_offset = from->tsc_offset; > > > - to->tlb_ctl = from->tlb_ctl; > > > > I don't think we should completely drop tlb_ctl. KVM doesn't do anything with > > vmcb12's tlb_ctl only because we haven't addressed the TODO list in > > nested_svm_transition_tlb_flush(). I think I would rather update this code to > > sanitize the field now, as opposed to waiting until we address that TODO. > > > > KVM advertises X86_FEATURE_FLUSHBYASID, so I think we can do the right thing > > without having to speculate on what the future will bring. > > > > Alternatively, we could add a TODO here or update the one in > > nested_svm_transition_tlb_flush(), but that seems like more overall work than > > just hardening the code. > > I will drop the ASID change. > > I honestly don't know where to draw the line at this point. Should I > split sanitizing all different fields into different patches? Or just > split the tlb_ctl change? What about the I/O and MSR bitmap change? FWIW, my rule of thumb is that, when writing the changelog, if I find myself either (a) having to explain mostly unrelated things or (b) generalizing away details, then I should split the patch. In this case, I would do: 1. int_ctl and friends 2. tlb_cnt (with a changelog explaining why we're keeping it even though it's unused) 3. ~0xfffull => PAGE_MASK cleanup 4. ASID comment Though honestly, I would drop the ASID change. The comment is misleading and arguably flat out wrong. This The ASID could be invalid, or conflict with another VM's ASID, so it should never be used directly to run L2. isn't why KVM musn't use the ASID from vmcb12 verbatim, the real reason is that the ASID space managed by KVM is logical different than the one managed by L1. I.e. KVM needs to shadow L1's ASID space, for all intents and purposes. And so KVM should never use vmcb12's ASID verbatim because it needs to be "translated".