Received: by 2002:a05:6358:3188:b0:123:57c1:9b43 with SMTP id q8csp1230081rwd; Thu, 8 Jun 2023 14:30:52 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6MMHo9StKonhF4T0M/6i4cZqTJABpTocQ5Zy7I69oU72KN4P+Qkd/+ipXACsuwUaKSbj4H X-Received: by 2002:a05:6a00:2e01:b0:64a:7723:fe04 with SMTP id fc1-20020a056a002e0100b0064a7723fe04mr7856866pfb.4.1686259852057; Thu, 08 Jun 2023 14:30:52 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1686259852; cv=none; d=google.com; s=arc-20160816; b=zUBeVCzWll05L+bPRpurEpK/y1yGWGOs9NUXRBF3fpW33JhK2zXi7uo6EsI8THigfN x8uTVDansLJP1D5h/Qtnn2X0RPWKa12YBuhA0opmsqdZux3dLXxPrzeVVtymrzuVF0QW lCmmQKAGIXoe7qoTUGGlESV8NZs1pcKf8Zkw2EbjCfzvu5uwCNEOl9ri/j5/j17b7D23 lmlqLB08hlzt/uccJtUOr17QvlOEu0iCBo+mH77u1//lyghtWABfU7qRG+s0Cnm+pDpS Y/RS0iItbWJnFz4ZUZJjGqbZHlPFFzQe7nzwVzvrG/HctCugQWA412Ht7VskCLqhIeb+ YfYg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:subject:cc:to:from:date :dkim-signature; bh=9PJ8juZ8Q2bIBQLeKki4qKbKgEzmr7QQFLcUkAqFdNw=; b=L95M0QxtGgD8M9pFidls8Ce96C5KYxtEkooRMIwKxEGGeHI6EiJZCiE+Oi14nfgYIf TvUgJHycqYE/RT4PlP24ugwE29hnjOaY7OspsKtp27sWlUItDMf1JNY/y9tTSseVDZE3 5+dmuoDIMJqUQgCyCwaYyUt+lMEhebRcyyKTCqCCfF2FuLjZ9ECPIdmzvCb1H6kpCT+F O0dpKR9Rp7jZPyurVnLDcWjmHcgFsdpsKMtHIpbO9rbRcg8UEJfVc3LfCrfBZ/+qPNLw n3zeA/5WD+cU22sTyVK9UvQByLEERfXByykSC49qcbDbum7MjWmjb4ekJa2jkZzJCrL3 J5rw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=BAas7k9q; 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=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id w18-20020aa79a12000000b0063d45a6d0e5si1372097pfj.17.2023.06.08.14.30.39; Thu, 08 Jun 2023 14:30:52 -0700 (PDT) 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=@redhat.com header.s=mimecast20190719 header.b=BAas7k9q; 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=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236051AbjFHU5T (ORCPT + 99 others); Thu, 8 Jun 2023 16:57:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52084 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229927AbjFHU5R (ORCPT ); Thu, 8 Jun 2023 16:57:17 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9211918C for ; Thu, 8 Jun 2023 13:56:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1686257796; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=9PJ8juZ8Q2bIBQLeKki4qKbKgEzmr7QQFLcUkAqFdNw=; b=BAas7k9qYbuKVWEIc1HVjimMbooxzQMZGcl0PV0MBmUcI71Mhr+Qy2Q5G5O5wgBP6O/Dsf q2u1vweIaYnFQizneIDl6s+cvvdfaA4dL/UUYp44wU0zenQskZZbhJqu9yShFtUBHFflCD 7mjowXX6hZvBepsuF3jW0Ccw78pUGKM= Received: from mail-io1-f69.google.com (mail-io1-f69.google.com [209.85.166.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-316-Wix3_xyZPnqg9WZVR37R_A-1; Thu, 08 Jun 2023 16:56:35 -0400 X-MC-Unique: Wix3_xyZPnqg9WZVR37R_A-1 Received: by mail-io1-f69.google.com with SMTP id ca18e2360f4ac-77accdaa0e0so90233039f.0 for ; Thu, 08 Jun 2023 13:56:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686257794; x=1688849794; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=9PJ8juZ8Q2bIBQLeKki4qKbKgEzmr7QQFLcUkAqFdNw=; b=H6LE/wlg5SYAFftiNT9j4OlpNa7LhEbJGjg6YbREqcmaBXhTPvBZATZKoI1xaiKJc1 qm0aByjHAIY74lIyrqxnh5hu9rb8t+N+bDF28MY4BJ1DRr5xxMTLVGeFW8IbfiKPUPiy iyrLFHUJtVUuy+aGkihYqc4yYkHgXUBDkSTrEsWIJ222yWHESEQE4+vQ3608l6NuC9lQ oESFyCkwk+GvT3ZB94CS2xHG2egW1lI6eqzRwedK2yDDozUSlWo9om1DGRK59d7n9gaN VQEPbE5aRhsbFGLw1mmTkBpGMr/OIT8bqvrnzn4r/iDC2asgY/RLyfly/lMuuae9FQJr M3Fg== X-Gm-Message-State: AC+VfDytzrg339ZDnnGOyJQEJGiYJqNp0VX4iSeO4uKzraX4MPhrpkqG LQxnMqfjGQ3wJsEb7SvZpCwmwxBVvzgXJUlVo8NsdhROt9nc6rXIhYja5zw5ThAXMF0lzGgehIV 8LyCsGuFLaMOodmDhvNAlnTqA X-Received: by 2002:a5e:c119:0:b0:774:84b5:cb77 with SMTP id v25-20020a5ec119000000b0077484b5cb77mr9289134iol.18.1686257794566; Thu, 08 Jun 2023 13:56:34 -0700 (PDT) X-Received: by 2002:a5e:c119:0:b0:774:84b5:cb77 with SMTP id v25-20020a5ec119000000b0077484b5cb77mr9289129iol.18.1686257794339; Thu, 08 Jun 2023 13:56:34 -0700 (PDT) Received: from redhat.com ([38.15.36.239]) by smtp.gmail.com with ESMTPSA id z20-20020a029f14000000b00408b3bc8061sm500297jal.43.2023.06.08.13.56.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Jun 2023 13:56:33 -0700 (PDT) Date: Thu, 8 Jun 2023 14:56:32 -0600 From: Alex Williamson To: Sidhartha Kumar Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, khalid.aziz@oracle.com Subject: Re: [PATCH] vfio/iommu_type1: acquire iommu lock in vfio_iommu_type1_release() Message-ID: <20230608145632.41073b65.alex.williamson@redhat.com> In-Reply-To: <8bf63ad2-43fd-41cd-c3dc-0507701eb5c1@oracle.com> References: <20230607190752.216801-1-sidhartha.kumar@oracle.com> <20230607134037.6d81e288.alex.williamson@redhat.com> <8bf63ad2-43fd-41cd-c3dc-0507701eb5c1@oracle.com> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.35; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE 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: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 8 Jun 2023 13:16:12 -0700 Sidhartha Kumar wrote: > On 6/7/23 12:40 PM, Alex Williamson wrote: > > On Wed, 7 Jun 2023 12:07:52 -0700 > > Sidhartha Kumar wrote: > > > >> From vfio_iommu_type1_release() there is a code path: > >> > >> vfio_iommu_unmap_unpin_all() > >> vfio_remove_dma() > >> vfio_unmap_unpin() > >> unmap_unpin_slow() > >> vfio_unpin_pages_remote() > >> vfio_find_vpfn() > >> > >> This path is taken without acquiring the iommu lock so it could lead to > >> a race condition in the traversal of the pfn_list rb tree. > > > > What's the competing thread for the race, vfio_remove_dma() tests: > > > > WARN_ON(!RB_EMPTY_ROOT(&dma->pfn_list)); > > > > The fix is not unreasonable, but is this a theoretical fix upstream > > that's tickled by some downstream additions, or are we actually > > competing against page pinning by an mdev driver after the container is > > released? Thanks, > > > > Hello, > > In a stress test of starting and stopping multiple VMs for a few days we > observed a memory leak that occurs after a few days. These guests have > their memory pinned via the pin_user_pages_remote() call in > vaddr_get_pfns(). From examining the vfio/iommu_type1 code this > potential race condition was noticed, but we have not root caused this > race to be the cause of the memory leak. Ok, I think we're going to need to wait for that root cause. It's trivial to hold the lock here, but we're imminently freeing the object hosting that lock, so we have bigger problems than threads stepping on each other in the pfn_list if this lock were to actually resolve anything. It's not an oversight that we don't hold the lock here, there should be no outstanding references to the iommu object at this point. Thanks, Alex > >> The lack of > >> the iommu lock in vfio_iommu_type1_release() was confirmed by adding a > >> > >> WARN_ON(!mutex_is_locked(&iommu->lock)) > >> > >> which was reported in dmesg. Fix this potential race by adding a iommu > >> lock and release in vfio_iommu_type1_release(). > >> > >> Suggested-by: Khalid Aziz > >> Signed-off-by: Sidhartha Kumar > >> --- > >> drivers/vfio/vfio_iommu_type1.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > >> index 306e6f1d1c70e..7d2fea1b483dc 100644 > >> --- a/drivers/vfio/vfio_iommu_type1.c > >> +++ b/drivers/vfio/vfio_iommu_type1.c > >> @@ -2601,7 +2601,9 @@ static void vfio_iommu_type1_release(void *iommu_data) > >> kfree(group); > >> } > >> > >> + mutex_lock(&iommu->lock); > >> vfio_iommu_unmap_unpin_all(iommu); > >> + mutex_unlock(&iommu->lock); > >> > >> list_for_each_entry_safe(domain, domain_tmp, > >> &iommu->domain_list, next) { > > >