From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755160AbdLUUbU convert rfc822-to-8bit (ORCPT ); Thu, 21 Dec 2017 15:31:20 -0500 Received: from mga05.intel.com ([192.55.52.43]:51739 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752601AbdLUUbP (ORCPT ); Thu, 21 Dec 2017 15:31:15 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,437,1508828400"; d="scan'208";a="4765300" From: "Shaikh, Azhar" To: Jason Gunthorpe CC: "jarkko.sakkinen@linux.intel.com" , "javierm@redhat.com" , "peterhuewe@gmx.de" , "linux-security-module@vger.kernel.org" , "linux-integrity@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "tpmdd-devel@lists.sourceforge.net" Subject: RE: [PATCH] tpm: Fix the driver cleanup code Thread-Topic: [PATCH] tpm: Fix the driver cleanup code Thread-Index: AQHTepoOkKvxqwDV30CBEJrcFFLGoqNOP2HQ Date: Thu, 21 Dec 2017 20:31:14 +0000 Message-ID: <5FFFAD06ADE1CA4381B3F0F7C6AF5828989144@ORSMSX109.amr.corp.intel.com> References: <1513887422-123222-1-git-send-email-azhar.shaikh@intel.com> <20171221202652.GH20015@ziepe.ca> In-Reply-To: <20171221202652.GH20015@ziepe.ca> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZDA2OGNlYWQtZTM1Yi00OTQxLTg5NmEtMmIzYWY2MmY3N2JmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJxVGlSZjc2dERnWndxSHpSc3YxalFnTUp2REJ5TkJ5cHgxRUI5SWNIQ09WNTgrMlJaUEk2NmVVenhQZXk4ZlJtIn0= dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.22.254.140] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >-----Original Message----- >From: Jason Gunthorpe [mailto:jgg@ziepe.ca] >Sent: Thursday, December 21, 2017 12:27 PM >To: Shaikh, Azhar >Cc: jarkko.sakkinen@linux.intel.com; javierm@redhat.com; >peterhuewe@gmx.de; linux-security-module@vger.kernel.org; linux- >integrity@vger.kernel.org; linux-kernel@vger.kernel.org; tpmdd- >devel@lists.sourceforge.net >Subject: Re: [PATCH] tpm: Fix the driver cleanup code > >On Thu, Dec 21, 2017 at 12:17:02PM -0800, Azhar Shaikh wrote: >> Commit 3c1701339284353c41 ("tpm: Keep CLKRUN enabled throughout the >> duration of transmit_cmd()") added code which accessed >> chip->ops, even after it was set to NULL in tpm_del_char_device(), >> called from tpm_chip_unregister() in error / driver exit paths. >> So fix this code. >> >> Fixes: 3c1701339284353c41 ("tpm: Keep CLKRUN enabled throughout the >> duration of transmit_cmd()") >> >> Suggested-by: Javier Martinez Canillas >> Suggested-by: Jason Gunthorpe >> Signed-off-by: Azhar Shaikh >> drivers/char/tpm/tpm-chip.c | 5 +++++ >> drivers/char/tpm/tpm.h | 1 + >> drivers/char/tpm/tpm_tis.c | 8 ++------ >> drivers/char/tpm/tpm_tis_core.c | 23 +++++++++++++++-------- >> drivers/char/tpm/tpm_tis_spi.c | 1 + >> 5 files changed, 24 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c >> index 0a62c19937b6..c88ec9a32a7e 100644 >> +++ b/drivers/char/tpm/tpm-chip.c >> @@ -346,6 +346,10 @@ static void tpm_del_char_device(struct tpm_chip >*chip) >> down_write(&chip->ops_sem); >> if (chip->flags & TPM_CHIP_FLAG_TPM2) >> tpm2_shutdown(chip, TPM2_SU_CLEAR); >> + if (chip->flags & TPM_CHIP_FLAG_DO_NOT_CLEAR_OPS) { >> + up_write(&chip->ops_sem); >> + return; >> + } > >Yikes, too ugly. Figure something else out.. ops->clk_enable is always >tpm_tis_clkrun_enable at this point, so maybe call it directly in the removal >path in tpm_tis_core? > Yes I thought about it too. But if some other chip->ops function in future, which *might* be in this same case, hence for that introduced this flag. I can use ops->clk_enable instead of introducing the new flag. >Jason Regards, Azhar Shaikh