From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Gunthorpe Subject: Re: [PATCH v5 4/7] tpm: fix the race condition between event log access and chip getting unregistered Date: Mon, 7 Nov 2016 17:29:43 -0700 Message-ID: <20161108002943.GB13959@obsidianresearch.com> References: <1476838185-24007-1-git-send-email-nayna@linux.vnet.ibm.com> <1476838185-24007-5-git-send-email-nayna@linux.vnet.ibm.com> <20161104051410.iqxb5k6fwizv7inc@intel.com> <20161107234839.GC7002@obsidianresearch.com> <20161108002642.4pvvubjcz57m4nov@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20161108002642.4pvvubjcz57m4nov-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: tpmdd-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Jarkko Sakkinen Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: tpmdd-devel@lists.sourceforge.net On Mon, Nov 07, 2016 at 04:26:43PM -0800, Jarkko Sakkinen wrote: > On Mon, Nov 07, 2016 at 04:48:39PM -0700, Jason Gunthorpe wrote: > > On Thu, Nov 03, 2016 at 11:14:10PM -0600, Jarkko Sakkinen wrote: > > > On Tue, Oct 18, 2016 at 08:49:42PM -0400, Nayna Jain wrote: > > > > Currently, the event log file operations are not serialized with > > > > tpm_chip_unregister(), which can possibly cause a race condition. > > > > > > > > This patch fixes the race condition by: > > > > - moving read_log() from fops to chip register. > > > > > > What is "chip register"? Please use exact names. > > > > > > > - disallowing event log file operations when chip unregister is in > > > > progress. > > > > > > Could you elaborate this sentence? > > > > > > > - guarding event log memory using chip krefs. > > > > > > Could you elaborate this sentence? > > > > > > Please describe how the race condition could happen and provide the > > > "Fixes:" line for the commit ID that caused it. Otherwise, your commit > > > message won't make any sense. I cannot apply this commit with this > > > commit message. > > > > > > The commit message does not make much sense... > > > > Lets get this moving along, it is hard to keep everything straight > > over months.. > > > > Nayna: This commit message should work: > > > > tpm: Have eventlog use the tpm_chip > > > > Move the backing memory for the eventlog into tpm_chip and push > > the tpm_chip into read_log. This optimizes read_log processing by > > only doing it once and prepares things for the next patches in the > > series which require the tpm_chip to locate the event log via > > ACPI and OF handles instead of searching. > > > > This is straightfoward except for the issue of passing a kref through > > i_private with securityfs. Since securityfs_remove does not have any > > removal fencing like sysfs we use the inode lock to safely get a > > kref on the tpm_chip. > > Perfect. Thank you. > > With this > > Reviewed-by: Jarkko Sakkinen > > No need for resend. The missing null still needs to be fixed, and the inode lock needs to be held only over get_device in open() Jason ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi