From patchwork Mon Aug 18 22:46:06 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Matlack X-Patchwork-Id: 4739581 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 32B839F3EE for ; Mon, 18 Aug 2014 22:47:22 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 40A6520121 for ; Mon, 18 Aug 2014 22:47:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5375C200F3 for ; Mon, 18 Aug 2014 22:47:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752269AbaHRWqm (ORCPT ); Mon, 18 Aug 2014 18:46:42 -0400 Received: from mail-yh0-f48.google.com ([209.85.213.48]:61967 "EHLO mail-yh0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752193AbaHRWql (ORCPT ); Mon, 18 Aug 2014 18:46:41 -0400 Received: by mail-yh0-f48.google.com with SMTP id i57so5066463yha.7 for ; Mon, 18 Aug 2014 15:46:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=hLNHIWda5YhXQKjJnWy2PVcE3jklceh4wM0acG/jjzM=; b=mzGm7eL1qz8+YvZhar3jk8kDcXGNEvkTSwguP5mZbvJqjm8QW5/laI1pGzxXGB7DG1 GKG7ZXEUXQ8QZv36kfyJVmahrdJm9nlUBBSrVUHgV0Lq3LBiEeLIgsBleVrBGlzeJc2Z bpfDEzMBmSHvbZsX8Kogsz0o3I6mwNOyFO+f+WVJaSmMURJUJ27gB3ho3jAnRwfdU1NT 08PrjAT56lVI9Sdmr/FjgibcEjt+PK6e7SmG7ppVR8nXSNtBaNpUXpleKfJlNO7KIIkN iHmoQnPpaiaCJ1Zl5GunbDQIkmsAVtN/KcQLoAFs6kMeFPrOwINr9gNHtVeZbSt0LrtT Ik2A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=hLNHIWda5YhXQKjJnWy2PVcE3jklceh4wM0acG/jjzM=; b=KNgEZ7ybe3K+JIS0sHgAxTwmU80NkW0dUAdKgMITb92UHS5tJGSrGF9oVpl2lCGArx BbtBx/Sqp54DQsBc+cdc/bLf17wMK0D5SdpPTTpBHX57JA/asR6jLBIdesPMV+LIXSlG zP6LHLgTgA7yEZ93FCpmKaIhiEB0gsYW2g+qasTfZBhAjtDkJVNdInC+QxZFxOHG2YOk nofWSTTHsLBmkTe8L9KsGJX69nddy/O+HmmeCY7MWlAqjF38W93TLgic6URUPyqlhzcn fNvk5SYnodCG+P8W4Nw+EyIdyfWpPG0BiipIyY3OcJLPSGtoUgkcsLSRQf6pFUpO6xZW HcmA== X-Gm-Message-State: ALoCoQlmDSKZox7jNlccRdc47vNDXL3hBMuWoR9TJSav6Ntx4I5/MCFtGQfpWb3P9ZqTZBFUcerV X-Received: by 10.236.11.45 with SMTP id 33mr5011114yhw.153.1408402000630; Mon, 18 Aug 2014 15:46:40 -0700 (PDT) Received: from dmatlack-linux.kir.corp.google.com (dmatlack-linux.kir.corp.google.com [172.31.88.63]) by mx.google.com with ESMTPSA id v56sm38460909yha.27.2014.08.18.15.46.38 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 18 Aug 2014 15:46:39 -0700 (PDT) From: David Matlack To: pbonzini@redhat.com Cc: gleb@redhat.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, avi.kivity@gmail.com, mtosatti@redhat.com, David Matlack , stable@vger.kernel.org, Xiao Guangrong Subject: [PATCH 1/2] kvm: fix potentially corrupt mmio cache Date: Mon, 18 Aug 2014 15:46:06 -0700 Message-Id: <1408401967-27211-1-git-send-email-dmatlack@google.com> X-Mailer: git-send-email 2.1.0.rc2.206.gedb03e5 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP vcpu exits and memslot mutations can run concurrently as long as the vcpu does not aquire the slots mutex. Thus it is theoretically possible for memslots to change underneath a vcpu that is handling an exit. If we increment the memslot generation number again after synchronize_srcu_expedited(), vcpus can safely cache memslot generation without maintaining a single rcu_dereference through an entire vm exit. And much of the x86/kvm code does not maintain a single rcu_dereference of the current memslots during each exit. We can prevent the following case: vcpu (CPU 0) | thread (CPU 1) --------------------------------------------+-------------------------- 1 vm exit | 2 decide to cache something based on | old memslots | 3 | change memslots 4 | increment generation 5 tag cache with new memslot generation | ... | | ... | | By incrementing the generation again after synchronizing kvm->srcu readers, we guarantee the generation cached in (5) will very soon become invalid. Cc: stable@vger.kernel.org Cc: Xiao Guangrong Signed-off-by: David Matlack --- virt/kvm/kvm_main.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 4b6c01b..86d3697 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -95,8 +95,6 @@ static int hardware_enable_all(void); static void hardware_disable_all(void); static void kvm_io_bus_destroy(struct kvm_io_bus *bus); -static void update_memslots(struct kvm_memslots *slots, - struct kvm_memory_slot *new, u64 last_generation); static void kvm_release_pfn_dirty(pfn_t pfn); static void mark_page_dirty_in_slot(struct kvm *kvm, @@ -685,8 +683,7 @@ static void sort_memslots(struct kvm_memslots *slots) } static void update_memslots(struct kvm_memslots *slots, - struct kvm_memory_slot *new, - u64 last_generation) + struct kvm_memory_slot *new) { if (new) { int id = new->id; @@ -697,8 +694,6 @@ static void update_memslots(struct kvm_memslots *slots, if (new->npages != npages) sort_memslots(slots); } - - slots->generation = last_generation + 1; } static int check_memory_region_flags(struct kvm_userspace_memory_region *mem) @@ -720,10 +715,19 @@ static struct kvm_memslots *install_new_memslots(struct kvm *kvm, { struct kvm_memslots *old_memslots = kvm->memslots; - update_memslots(slots, new, kvm->memslots->generation); + slots->generation = old_memslots->generation + 1; + + update_memslots(slots, new); rcu_assign_pointer(kvm->memslots, slots); synchronize_srcu_expedited(&kvm->srcu); + /* + * Increment the new memslot generation a second time. This prevents + * vm exits that race with memslot updates from caching a memslot + * generation that will (potentially) be valid forever. + */ + slots->generation++; + kvm_arch_memslots_updated(kvm); return old_memslots;