From patchwork Mon Mar 6 23:19:46 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Gunthorpe X-Patchwork-Id: 735980 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.sourceforge.net (lists.sourceforge.net [216.34.181.88]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3vcbNl6Gq7z9s7y for ; Tue, 7 Mar 2017 10:20:07 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=sfs-ml-3.v29.ch3.sourceforge.com) by sfs-ml-3.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1cl1v7-0004lS-Fg; Mon, 06 Mar 2017 23:20:01 +0000 Received: from sog-mx-2.v43.ch3.sourceforge.com ([172.29.43.192] helo=mx.sourceforge.net) by sfs-ml-3.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1cl1v6-0004lI-Px for tpmdd-devel@lists.sourceforge.net; Mon, 06 Mar 2017 23:20:00 +0000 Received-SPF: pass (sog-mx-2.v43.ch3.sourceforge.com: domain of obsidianresearch.com designates 184.70.90.242 as permitted sender) client-ip=184.70.90.242; envelope-from=jgunthorpe@obsidianresearch.com; helo=quartz.orcorp.ca; Received: from quartz.orcorp.ca ([184.70.90.242]) by sog-mx-2.v43.ch3.sourceforge.com with esmtps (TLSv1:AES128-SHA:128) (Exim 4.76) id 1cl1v5-0006U3-BJ for tpmdd-devel@lists.sourceforge.net; Mon, 06 Mar 2017 23:20:00 +0000 Received: from [10.0.0.156] (helo=jggl.edm.orcorp.ca) by quartz.orcorp.ca with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1cl1us-0002dl-An; Mon, 06 Mar 2017 16:19:46 -0700 Received: from jgg by jggl.edm.orcorp.ca with local (Exim 4.86_2) (envelope-from ) id 1cl1us-0006d0-8V; Mon, 06 Mar 2017 16:19:46 -0700 Date: Mon, 6 Mar 2017 16:19:46 -0700 From: Jason Gunthorpe To: "Hon Ching(Vicky) Lo" Message-ID: <20170306231946.GA23953@obsidianresearch.com> References: <1488839535-11822-1-git-send-email-honclo@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1488839535-11822-1-git-send-email-honclo@linux.vnet.ibm.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Broken-Reverse-DNS: no host name found for IP address 10.0.0.156 X-Spam-Score: -1.4 (-) X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -1.5 SPF_CHECK_PASS SPF reports sender host as permitted sender for sender-domain -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -0.0 SPF_PASS SPF: sender matches SPF record 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid 0.0 T_DKIM_INVALID DKIM-Signature header exists but is not valid X-Headers-End: 1cl1v5-0006U3-BJ Cc: stable@vger.kernel.org, Peter Huewe , tpmdd-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, Ashley Lai Subject: Re: [tpmdd-devel] [PATCH] vTPM: Fix missing NULL check X-BeenThere: tpmdd-devel@lists.sourceforge.net X-Mailman-Version: 2.1.9 Precedence: list List-Id: Tpm Device Driver maintainance List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: tpmdd-devel-bounces@lists.sourceforge.net On Mon, Mar 06, 2017 at 05:32:15PM -0500, Hon Ching(Vicky) Lo wrote: > The current code passes the address of tpm_chip as the argument to > dev_get_drvdata() without prior NULL check in > tpm_ibmvtpm_get_desired_dma. This resulted an oops during kernel > boot when vTPM is enabled in Power partition configured in active > memory sharing mode. > > The vio_driver's get_desired_dma() is called before the probe(), which > for vtpm is tpm_ibmvtpm_probe, and it's this latter function that > initializes the driver and set data. Attempting to get data before > the probe() caused the problem. > > This patch adds a NULL check to the tpm_ibmvtpm_get_desired_dma. Does this also need a hunk in tpm_ibmvtpm_remove to null the drvdata after removal, or does something in the driver code guarentee it is null'd after remove? We don't want to use-after-free chip on the next probe cycle. > static unsigned long tpm_ibmvtpm_get_desired_dma(struct vio_dev *vdev) > { > struct tpm_chip *chip = dev_get_drvdata(&vdev->dev); > - struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev); > + struct ibmvtpm_dev *ibmvtpm = NULL; > + > + if (chip) > + ibmvtpm = dev_get_drvdata(&chip->dev); Maybe just do this, clearer that it is chip that can be null. We do not want to see drivers testing their chip drvdata against null. Also, how does locking work here? Does the vio core prevent tpm_ibmvtpm_get_desired_dma and tpm_ibmvtpm_remove from running concurrently? ------------------------------------------------------------------------------ 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 diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index 946025a7413b6b..ced6b9f0008dc2 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -294,6 +294,8 @@ static int tpm_ibmvtpm_remove(struct vio_dev *vdev) kfree(ibmvtpm->rtce_buf); } + /* For tpm_ibmvtpm_get_desired_dma */ + dev_set_drvdata(&vdev->dev, NULL); kfree(ibmvtpm); return 0; @@ -309,15 +311,16 @@ static int tpm_ibmvtpm_remove(struct vio_dev *vdev) static unsigned long tpm_ibmvtpm_get_desired_dma(struct vio_dev *vdev) { struct tpm_chip *chip = dev_get_drvdata(&vdev->dev); - struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev); + struct ibmvtpm_dev *ibmvtpm; /* ibmvtpm initializes at probe time, so the data we are * asking for may not be set yet. Estimate that 4K required * for TCE-mapped buffer in addition to CRQ. */ - if (!ibmvtpm) + if (!chip) return CRQ_RES_BUF_SIZE + PAGE_SIZE; + ibmvtpm = dev_get_drvdata(&chip->dev); return CRQ_RES_BUF_SIZE + ibmvtpm->rtce_size; }