From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Gunthorpe Subject: Re: [PATCH] vTPM: Fix missing NULL check Date: Wed, 8 Mar 2017 10:17:29 -0700 Message-ID: <20170308171729.GC16709@obsidianresearch.com> References: <1488839535-11822-1-git-send-email-honclo@linux.vnet.ibm.com> <20170306231946.GA23953@obsidianresearch.com> <1488946363.15595.17.camel@vtpm2014.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1488946363.15595.17.camel-0wQ8x0XRVuT1ENwx4SLHqw@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: tpmdd-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: "Hon Ching(Vicky) Lo" Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Peter Huewe , tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ashley Lai List-Id: tpmdd-devel@lists.sourceforge.net On Tue, Mar 07, 2017 at 11:12:43PM -0500, Hon Ching(Vicky) Lo wrote: > On Mon, 2017-03-06 at 16:19 -0700, Jason Gunthorpe wrote: > > Also, how does locking work here? Does the vio core prevent > > tpm_ibmvtpm_get_desired_dma and tpm_ibmvtpm_remove from running > > concurrently? > > No, vio core doesn't prevent tpm_ibmvtpm_get_desired_dma and tpm_ibmvtpm_remove > from running concurrently. > > vio_bus_probe calls vio_cmo_bus_probe which calls tpm_ibmvtpm_get_desired_dma. > tpm_ibmvtpm_get_desired_dma is called before the code enters critical section. > > There is no locking mechanism around tpm_ibmvtpm_remove in vio_bus_remove. > > What's the concern here? tpm_ibmvtpm_remove makes the pointer that tpm_ibmvtpm_get_desired_dma is accessing invalid, so some kind of locking is technically required so that the two things do not create a use after free race: > > + /* For tpm_ibmvtpm_get_desired_dma */ > > + dev_set_drvdata(&vdev->dev, NULL); > > kfree(ibmvtpm); Eg with the kfree above. It may be that the driver core prevents probe/remove from running concurrently and things are fine, but this is something to confirm.. Jason ------------------------------------------------------------------------------ Announcing the Oxford Dictionaries API! The API offers world-renowned dictionary content that is easy and intuitive to access. Sign up for an account today to start using our lexical data to power your apps and projects. Get started today and enter our developer competition. http://sdm.link/oxford From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753958AbdCHSJT (ORCPT ); Wed, 8 Mar 2017 13:09:19 -0500 Received: from quartz.orcorp.ca ([184.70.90.242]:35720 "EHLO quartz.orcorp.ca" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752639AbdCHSJQ (ORCPT ); Wed, 8 Mar 2017 13:09:16 -0500 Date: Wed, 8 Mar 2017 10:17:29 -0700 From: Jason Gunthorpe To: "Hon Ching(Vicky) Lo" Cc: tpmdd-devel@lists.sourceforge.net, Ashley Lai , linux-kernel@vger.kernel.org, stable@vger.kernel.org, Peter Huewe Subject: Re: [tpmdd-devel] [PATCH] vTPM: Fix missing NULL check Message-ID: <20170308171729.GC16709@obsidianresearch.com> References: <1488839535-11822-1-git-send-email-honclo@linux.vnet.ibm.com> <20170306231946.GA23953@obsidianresearch.com> <1488946363.15595.17.camel@vtpm2014.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1488946363.15595.17.camel@vtpm2014.ibm.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 07, 2017 at 11:12:43PM -0500, Hon Ching(Vicky) Lo wrote: > On Mon, 2017-03-06 at 16:19 -0700, Jason Gunthorpe wrote: > > Also, how does locking work here? Does the vio core prevent > > tpm_ibmvtpm_get_desired_dma and tpm_ibmvtpm_remove from running > > concurrently? > > No, vio core doesn't prevent tpm_ibmvtpm_get_desired_dma and tpm_ibmvtpm_remove > from running concurrently. > > vio_bus_probe calls vio_cmo_bus_probe which calls tpm_ibmvtpm_get_desired_dma. > tpm_ibmvtpm_get_desired_dma is called before the code enters critical section. > > There is no locking mechanism around tpm_ibmvtpm_remove in vio_bus_remove. > > What's the concern here? tpm_ibmvtpm_remove makes the pointer that tpm_ibmvtpm_get_desired_dma is accessing invalid, so some kind of locking is technically required so that the two things do not create a use after free race: > > + /* For tpm_ibmvtpm_get_desired_dma */ > > + dev_set_drvdata(&vdev->dev, NULL); > > kfree(ibmvtpm); Eg with the kfree above. It may be that the driver core prevents probe/remove from running concurrently and things are fine, but this is something to confirm.. Jason