Message ID | 20230207171354.4012821-1-pgonda@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp2970905wrn; Tue, 7 Feb 2023 09:17:14 -0800 (PST) X-Google-Smtp-Source: AK7set8jBrLKiJNsz/lS4FiOcKtAwRy8oer6oby5XgmCRKwa2priKSZfXGKU8BQGHT/4LkegNgQ/ X-Received: by 2002:a17:903:2444:b0:189:5ef4:6ae9 with SMTP id l4-20020a170903244400b001895ef46ae9mr4542023pls.45.1675790233917; Tue, 07 Feb 2023 09:17:13 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1675790233; cv=none; d=google.com; s=arc-20160816; b=GpBrwrknkmqkYaqfWCzEstDG8SSocdOwMie5W1aBwKpJX2O2dCT9e6CbGwNCBXfwzr cp4GCmnLqgONrIg2KkPLJDtDCmgZ8cKxrnLo+ugv+It8irqlpQW2TJDEE7SMQPPWj6XW Lo2cwHKx/YxT7tQu3qnPRFZNnLFUk+Xd624/AxRSciDoWUGwaqCILdZr5zviemtnFBZQ 6vLki6pSraBDiPFRXlqrtwA3PI8uzh9nStBl6F6/lK8US8kIn6uWveHB7YotRy53g4VI A9fHj1NIIn36sNY0t2r+Wu24+9T22wBqGaZsAGbSwh3ZOwGsASMJKVPQns01pFi+vqXH EpXQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:mime-version:message-id:date :dkim-signature; bh=gaqlkUHlROHo1Iu/mvu/VhOVerpPZOfXGd0nbt/7zs8=; b=e9hKYXOY6gijsnSkVI15FubPyG8hACtf5nczW6IKJr+YodgDmpM859/3W23G/9NW5E WmF2kCthd4YrwwzeLRd1pbxpl1jfFfBgO0MC4QDLdzELnk3T+yvDK2LowRRye+lhszn1 l06nfdsHJxdYUPO8q+0HFmkANrrpzqTflC+E8syeNojRmPBfuA1G8yqFlLgWiepP7ZTi Wnd5lYQN8CSz7udGD1eCltmfTWwLtpEIPTLAIA17iOi8BTd7asE/gJ9kHi4WKxrpH03r MugJ3mYQTQKbsAWpFw9MUFtiCIM/uTzeRrYFlRz23xho1AIQ4Z8NZpkWbvC+tGkI8ooz B2Tw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=aFSF+mDk; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id k23-20020a170902761700b0019489437a80si13833355pll.144.2023.02.07.09.17.01; Tue, 07 Feb 2023 09:17:13 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=aFSF+mDk; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231785AbjBGRPJ (ORCPT <rfc822;kmanaouilinux@gmail.com> + 99 others); Tue, 7 Feb 2023 12:15:09 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50608 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232249AbjBGROu (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 7 Feb 2023 12:14:50 -0500 Received: from mail-pg1-x54a.google.com (mail-pg1-x54a.google.com [IPv6:2607:f8b0:4864:20::54a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3E4428698 for <linux-kernel@vger.kernel.org>; Tue, 7 Feb 2023 09:14:00 -0800 (PST) Received: by mail-pg1-x54a.google.com with SMTP id h126-20020a636c84000000b004d31ad79086so7082420pgc.23 for <linux-kernel@vger.kernel.org>; Tue, 07 Feb 2023 09:14:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:mime-version:message-id:date:from:to:cc:subject :date:message-id:reply-to; bh=gaqlkUHlROHo1Iu/mvu/VhOVerpPZOfXGd0nbt/7zs8=; b=aFSF+mDkwXMPhNXoGe9Q/j26WS0JRqyy750UYsBMIHe2uxsn5RTNKfWiXtceSvhxa5 jtE+wKy5v7XDg0HNBzSH2kcs5k0RPiVdlmiHEigKt+L/Dx/GRZ95cfV/8haSHCcm6e/6 OybLUuEDpssvqVROntd3VOoMTRGRn3FgHA9N+aXExjoJwgShq1wHpm1S9S4NV9pWuTKE i5zm3mZuh6Ca6I6q+iuYH0CnU3MAA2KomIjb8tuhX7oH1H7M0Apn3rrYCm++euStY1AV PNE0SoqrGOrbbcoQEfj7ugB8nDtisZYJRPB0h5jKDccuxm6DHEpGAQlJRqPlDIfjEJLR nlSA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:mime-version:message-id:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=gaqlkUHlROHo1Iu/mvu/VhOVerpPZOfXGd0nbt/7zs8=; b=JcKRtAF23reVy3BSyvgibVPmyon0S46qXBwM0V8oR4U8MLHukalEY5GazhjzuZ7SSp Tg7NBtQV3vuV82SJYlZwlzrohMqjARUqtUReQKIRSruaT8lA0HlTaBPQDXJ9C1cD5QS2 S9pOXZk48oBSbQc4b0xUhlFpno6wTrXvnmN7uIfHwDw0bZdGQUsLnaezqcsofOBn00zP zqaqnNuX9P2Be3NU4vGabfrJE5kFumu6l2HzC1G7ytgm1hvUD4Itx3RaJmvCY3c5ENHI Js9iti97vmobhHQc+PPYIkN/YOv8vfRJh/t5ORZAgK7sbuPN0pEenM8vqaYZVz+IhKBX jvqw== X-Gm-Message-State: AO0yUKVNSsKaP/MIqvU/lrRhOJir9Amdj1QoUPj/wJpXnFy3gHFIDNCC IC0g9xoMB2Syc5Jiz20etr6VfPFdCYs= X-Received: from pgonda1.kir.corp.google.com ([2620:0:1008:11:346e:6fd8:c3bf:b38f]) (user=pgonda job=sendgmr) by 2002:a17:902:a3ce:b0:196:3672:f24b with SMTP id q14-20020a170902a3ce00b001963672f24bmr868930plb.32.1675790039532; Tue, 07 Feb 2023 09:13:59 -0800 (PST) Date: Tue, 7 Feb 2023 09:13:54 -0800 Message-Id: <20230207171354.4012821-1-pgonda@google.com> Mime-Version: 1.0 X-Mailer: git-send-email 2.39.1.519.gcb327c4b5f-goog Subject: [PATCH V2] KVM: sev: Fix potential overflow send|recieve_update_data From: Peter Gonda <pgonda@google.com> To: kvm@vger.kernel.org Cc: Peter Gonda <pgonda@google.com>, Andy Nguyen <theflow@google.com>, Thomas Lendacky <thomas.lendacky@amd.com>, David Rientjes <rientjes@google.com>, Paolo Bonzini <pbonzini@redhat.com>, Sean Christopherson <seanjc@google.com>, stable@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1757193420279056903?= X-GMAIL-MSGID: =?utf-8?q?1757193420279056903?= |
Series |
[V2] KVM: sev: Fix potential overflow send|recieve_update_data
|
|
Commit Message
Peter Gonda
Feb. 7, 2023, 5:13 p.m. UTC
KVM_SEV_SEND_UPDATE_DATA and KVM_SEV_RECEIVE_UPDATE_DATA have an integer
overflow issue. Params.guest_len and offset are both 32bite wide, with a
large params.guest_len the check to confirm a page boundary is not
crossed can falsely pass:
/* Check if we are crossing the page boundary *
offset = params.guest_uaddr & (PAGE_SIZE - 1);
if ((params.guest_len + offset > PAGE_SIZE))
Add an additional check to this conditional to confirm that
params.guest_len itself is not greater than PAGE_SIZE.
The current code is can only overflow with a params.guest_len of greater
than 0xfffff000. And the FW spec says these commands fail with lengths
greater than 16KB. So this issue should not be a security concern
Fixes: 15fb7de1a7f5 ("KVM: SVM: Add KVM_SEV_RECEIVE_UPDATE_DATA command")
Fixes: d3d1af85e2c7 ("KVM: SVM: Add KVM_SEND_UPDATE_DATA command")
Reported-by: Andy Nguyen <theflow@google.com>
Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Peter Gonda <pgonda@google.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: kvm@vger.kernel.org
Cc: stable@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
V2
* Updated conditional based on feedback from Tom.
---
arch/x86/kvm/svm/sev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
On 2/7/23 11:13, Peter Gonda wrote: > KVM_SEV_SEND_UPDATE_DATA and KVM_SEV_RECEIVE_UPDATE_DATA have an integer > overflow issue. Params.guest_len and offset are both 32bite wide, with a > large params.guest_len the check to confirm a page boundary is not > crossed can falsely pass: > > /* Check if we are crossing the page boundary * > offset = params.guest_uaddr & (PAGE_SIZE - 1); > if ((params.guest_len + offset > PAGE_SIZE)) > > Add an additional check to this conditional to confirm that > params.guest_len itself is not greater than PAGE_SIZE. > > The current code is can only overflow with a params.guest_len of greater > than 0xfffff000. And the FW spec says these commands fail with lengths > greater than 16KB. So this issue should not be a security concern > > Fixes: 15fb7de1a7f5 ("KVM: SVM: Add KVM_SEV_RECEIVE_UPDATE_DATA command") > Fixes: d3d1af85e2c7 ("KVM: SVM: Add KVM_SEND_UPDATE_DATA command") > Reported-by: Andy Nguyen <theflow@google.com> > Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com> > Signed-off-by: Peter Gonda <pgonda@google.com> > Cc: David Rientjes <rientjes@google.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Sean Christopherson <seanjc@google.com> > Cc: kvm@vger.kernel.org > Cc: stable@vger.kernel.org > Cc: linux-kernel@vger.kernel.org Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > > V2 > * Updated conditional based on feedback from Tom. > > --- > arch/x86/kvm/svm/sev.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index 273cba809328..3d74facaead8 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -1294,7 +1294,7 @@ static int sev_send_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp) > > /* Check if we are crossing the page boundary */ > offset = params.guest_uaddr & (PAGE_SIZE - 1); > - if ((params.guest_len + offset > PAGE_SIZE)) > + if (params.guest_len > PAGE_SIZE || (params.guest_len + offset) > PAGE_SIZE) > return -EINVAL; > > /* Pin guest memory */ > @@ -1474,7 +1474,7 @@ static int sev_receive_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp) > > /* Check if we are crossing the page boundary */ > offset = params.guest_uaddr & (PAGE_SIZE - 1); > - if ((params.guest_len + offset > PAGE_SIZE)) > + if (params.guest_len > PAGE_SIZE || (params.guest_len + offset) > PAGE_SIZE) > return -EINVAL; > > hdr = psp_copy_user_blob(params.hdr_uaddr, params.hdr_len);
For now at least, I want to keep with "KVM: SVM:" instead of using "KVM: SEV:". Many commits that touch SEV aren't strictly isolated to SEV, which means the "SEV" tag is unreliable. There's also the question of taggin SEV vs. SEV-ES vs. SEV-SNP. It's usually easy enough to squeeze SEV (or SEV-ES or SNP) into the shortlog, e.g. KVM: SVM: Fix potential overflow in SEV's send|receive_update_data() On Tue, Feb 07, 2023, Peter Gonda wrote: > KVM_SEV_SEND_UPDATE_DATA and KVM_SEV_RECEIVE_UPDATE_DATA have an integer > overflow issue. Params.guest_len and offset are both 32bite wide, with a "32 bits" > large params.guest_len the check to confirm a page boundary is not > crossed can falsely pass: > > /* Check if we are crossing the page boundary * > offset = params.guest_uaddr & (PAGE_SIZE - 1); > if ((params.guest_len + offset > PAGE_SIZE)) > > Add an additional check to this conditional to confirm that Eh, "to this conditional" is unnecessarily precise. > params.guest_len itself is not greater than PAGE_SIZE. > > The current code is can only overflow with a params.guest_len of greater "is can", though I vote to omit the "current code" part entirely, it should be obvious that this is talking about the pre-patched code. > than 0xfffff000. And the FW spec says these commands fail with lengths > greater than 16KB. So this issue should not be a security concern Slightly reworded, how about this for the "not a security concern" disclaimer? Note, this isn't a security concern as overflow can happen if and only if params.guest_len is greater than 0xfffff000, and the FW spec says these commands fail with lengths greater than 16KB, i.e. the PSP will detect KVM's goof. No need to send a v3, I'll fix up the changelog when applying. Holler if you disagree with anything though. Thanks!
On Tue, 07 Feb 2023 09:13:54 -0800, Peter Gonda wrote: > KVM_SEV_SEND_UPDATE_DATA and KVM_SEV_RECEIVE_UPDATE_DATA have an integer > overflow issue. Params.guest_len and offset are both 32bite wide, with a > large params.guest_len the check to confirm a page boundary is not > crossed can falsely pass: > > /* Check if we are crossing the page boundary * > offset = params.guest_uaddr & (PAGE_SIZE - 1); > if ((params.guest_len + offset > PAGE_SIZE)) > > [...] Applied to kvm-x86 svm, thanks! [1/1] KVM: sev: Fix potential overflow send|recieve_update_data https://github.com/kvm-x86/linux/commit/f94f053aa3a5 -- https://github.com/kvm-x86/linux/tree/next https://github.com/kvm-x86/linux/tree/fixes
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 273cba809328..3d74facaead8 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -1294,7 +1294,7 @@ static int sev_send_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp) /* Check if we are crossing the page boundary */ offset = params.guest_uaddr & (PAGE_SIZE - 1); - if ((params.guest_len + offset > PAGE_SIZE)) + if (params.guest_len > PAGE_SIZE || (params.guest_len + offset) > PAGE_SIZE) return -EINVAL; /* Pin guest memory */ @@ -1474,7 +1474,7 @@ static int sev_receive_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp) /* Check if we are crossing the page boundary */ offset = params.guest_uaddr & (PAGE_SIZE - 1); - if ((params.guest_len + offset > PAGE_SIZE)) + if (params.guest_len > PAGE_SIZE || (params.guest_len + offset) > PAGE_SIZE) return -EINVAL; hdr = psp_copy_user_blob(params.hdr_uaddr, params.hdr_len);