From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754778AbbCRFEp (ORCPT ); Wed, 18 Mar 2015 01:04:45 -0400 Received: from mga14.intel.com ([192.55.52.115]:40839 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752050AbbCRFEl (ORCPT ); Wed, 18 Mar 2015 01:04:41 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,420,1422950400"; d="scan'208";a="681692777" Date: Wed, 18 Mar 2015 07:04:36 +0200 From: Jarkko Sakkinen To: Peter Huewe , Ashley Lai , Marcel Selhorst Cc: tpmdd-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, christophe.ricard@gmail.com, jason.gunthorpe@obsidianresearch.com, stefanb@linux.vnet.ibm.com Subject: Re: [PATCH] tpm: fix: sanitized code paths in tpm_chip_register() Message-ID: <20150318050436.GA12520@intel.com> References: <1426654201-4761-1-git-send-email-jarkko.sakkinen@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1426654201-4761-1-git-send-email-jarkko.sakkinen@linux.intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo 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 Resending v2 soon. This version was from wrong branch :( Sorry. On Wed, Mar 18, 2015 at 06:50:01AM +0200, Jarkko Sakkinen wrote: > I started to work with PPI interface so that it would be available > under character device sysfs directory and realized that chip > registeration was still too messy. > > In TPM 1.x in some rare scenarios (errors that almost never occur) > wrong order in deinitialization steps was taken in teardown. I > reproduced these scenarios by manually inserting error codes in the > place of the corresponding function calls. > > The key problem is that the teardown is messy with two separate code > paths (this was inherited when moving code from tpm-interface.c). > > Moved TPM 1.x specific register/unregister functionality to own helper > functions and added single code path for teardown in tpm_chip_register(). > Now the code paths have been fixed and it should be easier to review > later on this part of the code. > > Signed-off-by: Jarkko Sakkinen > --- > drivers/char/tpm/tpm-chip.c | 56 +++++++++++++++++++++++++++++---------------- > 1 file changed, 36 insertions(+), 20 deletions(-) > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > index cf43f4b..e5b0481 100644 > --- a/drivers/char/tpm/tpm-chip.c > +++ b/drivers/char/tpm/tpm-chip.c > @@ -170,6 +170,33 @@ static void tpm_dev_del_device(struct tpm_chip *chip) > device_unregister(&chip->dev); > } > > +static int tpm1_chip_register(struct tpm_chip *chip) > +{ > + int rc; > + > + if (chip->flags & TPM_CHIP_FLAG_TPM2) > + return 0; > + > + rc = tpm_sysfs_add_device(chip); > + if (rc) > + return rc; > + > + chip->bios_dir = tpm_bios_log_setup(chip->devname); > + > + return 0; > +} > + > +static void tpm1_chip_unregister(struct tpm_chip *chip) > +{ > + if (chip->flags & TPM_CHIP_FLAG_TPM2) > + return; > + > + if (chip->bios_dir) > + tpm_bios_log_teardown(chip->bios_dir); > + > + tpm_sysfs_del_device(chip); > +} > + > /* > * tpm_chip_register() - create a character device for the TPM chip > * @chip: TPM chip to use. > @@ -185,22 +212,17 @@ int tpm_chip_register(struct tpm_chip *chip) > { > int rc; > > - /* Populate sysfs for TPM1 devices. */ > - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) { > - rc = tpm_sysfs_add_device(chip); > - if (rc) > - goto del_misc; > - > - chip->bios_dir = tpm_bios_log_setup(chip->devname); > - } > + rc = tpm1_chip_register(chip); > + if (rc) > + return rc; > > rc = tpm_add_ppi(chip); > if (rc) > - goto del_sysfs; > + goto out_err; > > rc = tpm_dev_add_device(chip); > if (rc) > - return rc; > + goto out_err; > > /* Make the chip available. */ > spin_lock(&driver_lock); > @@ -210,10 +232,9 @@ int tpm_chip_register(struct tpm_chip *chip) > chip->flags |= TPM_CHIP_FLAG_REGISTERED; > > return 0; > -del_sysfs: > - tpm_sysfs_del_device(chip); > -del_misc: > - tpm_dev_del_device(chip); > +out_err: > + tpm_remove_ppi(chip); > + tpm1_chip_unregister(chip); > return rc; > } > EXPORT_SYMBOL_GPL(tpm_chip_register); > @@ -238,13 +259,8 @@ void tpm_chip_unregister(struct tpm_chip *chip) > spin_unlock(&driver_lock); > synchronize_rcu(); > > - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) { > - if (chip->bios_dir) > - tpm_bios_log_teardown(chip->bios_dir); > - tpm_sysfs_del_device(chip); > - } > - > tpm_remove_ppi(chip); > + tpm1_chip_unregister(chip); > tpm_dev_del_device(chip); > } > EXPORT_SYMBOL_GPL(tpm_chip_unregister); > -- > 2.1.4 >