Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751229AbdH1Emf (ORCPT ); Mon, 28 Aug 2017 00:42:35 -0400 Received: from ozlabs.org ([103.22.144.67]:42439 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751182AbdH1Emd (ORCPT ); Mon, 28 Aug 2017 00:42:33 -0400 Date: Mon, 28 Aug 2017 14:38:37 +1000 From: Paul Mackerras To: Al Viro Cc: Nixiaoming , David Hildenbrand , "agraf@suse.com" , "pbonzini@redhat.com" , "rkrcmar@redhat.com" , "benh@kernel.crashing.org" , "mpe@ellerman.id.au" , "kvm-ppc@vger.kernel.org" , "kvm@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" , "linux-kernel@vger.kernel.org" Subject: Re: Re: [PATCH] fix memory leak on kvm_vm_ioctl_create_spapr_tce Message-ID: <20170828043837.GA12629@fergus.ozlabs.ibm.com> References: <20170822142823.69425-1-nixiaoming@huawei.com> <95fe182a-fd21-77a1-33df-0e609c2845fd@redhat.com> <8fd9a878-a8ce-5576-9a5c-1c221ff6ded7@redhat.com> <20170823060624.GA13958@fergus.ozlabs.ibm.com> <20170827210220.GG5426@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170827210220.GG5426@ZenIV.linux.org.uk> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 749 Lines: 20 On Sun, Aug 27, 2017 at 10:02:20PM +0100, Al Viro wrote: > On Wed, Aug 23, 2017 at 04:06:24PM +1000, Paul Mackerras wrote: > > > It seems to me that it would be better to do the anon_inode_getfd() > > call before the kvm_get_kvm() call, and go to the fail label if it > > fails. > > And what happens if another thread does close() on the (guessed) fd? Chaos ensues, but mostly because we don't have proper mutual exclusion on the modifications to the list. I'll add a mutex_lock/unlock to kvm_spapr_tce_release() and move the anon_inode_getfd() call inside the mutex. It looks like the other possible uses of the fd (mmap, and passing it as a parameter to the KVM_DEV_VFIO_GROUP_SET_SPAPR_TCE ioctl on a KVM device fd) are safe. Thanks, Paul.