From patchwork Thu Nov 17 12:35:05 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Berger X-Patchwork-Id: 9434217 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id C379B6047D for ; Thu, 17 Nov 2016 12:35:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A970229440 for ; Thu, 17 Nov 2016 12:35:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9AB0F29482; Thu, 17 Nov 2016 12:35:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 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 mail.wl.linuxfoundation.org (Postfix) with ESMTPS id B9A4E29440 for ; Thu, 17 Nov 2016 12:35:27 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=sfs-ml-2.v29.ch3.sourceforge.com) by sfs-ml-2.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1c7LuW-00041b-5u; Thu, 17 Nov 2016 12:35:24 +0000 Received: from sog-mx-4.v43.ch3.sourceforge.com ([172.29.43.194] helo=mx.sourceforge.net) by sfs-ml-2.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1c7LuV-00041T-9E for tpmdd-devel@lists.sourceforge.net; Thu, 17 Nov 2016 12:35:23 +0000 X-ACL-Warn: Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]) by sog-mx-4.v43.ch3.sourceforge.com with esmtps (TLSv1:AES256-SHA:256) (Exim 4.76) id 1c7LuP-0002fx-7K for tpmdd-devel@lists.sourceforge.net; Thu, 17 Nov 2016 12:35:23 +0000 Received: from pps.filterd (m0098393.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id uAHCXiM4113876 for ; Thu, 17 Nov 2016 07:35:11 -0500 Received: from e38.co.us.ibm.com (e38.co.us.ibm.com [32.97.110.159]) by mx0a-001b2d01.pphosted.com with ESMTP id 26saddygcc-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 17 Nov 2016 07:35:11 -0500 Received: from localhost by e38.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 17 Nov 2016 05:35:10 -0700 Received: from d03dlp02.boulder.ibm.com (9.17.202.178) by e38.co.us.ibm.com (192.168.1.138) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Thu, 17 Nov 2016 05:35:07 -0700 Received: from b03cxnp08027.gho.boulder.ibm.com (b03cxnp08027.gho.boulder.ibm.com [9.17.130.19]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id 3224C3E4003F; Thu, 17 Nov 2016 05:35:07 -0700 (MST) Received: from b03ledav005.gho.boulder.ibm.com (b03ledav005.gho.boulder.ibm.com [9.17.130.236]) by b03cxnp08027.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id uAHCZ7PB40894582; Thu, 17 Nov 2016 05:35:07 -0700 Received: from b03ledav005.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0EA5FBE039; Thu, 17 Nov 2016 05:35:07 -0700 (MST) Received: from sbct-3.watson.ibm.com (unknown [9.2.141.158]) by b03ledav005.gho.boulder.ibm.com (Postfix) with ESMTP id A60F2BE038; Thu, 17 Nov 2016 05:35:06 -0700 (MST) To: Jason Gunthorpe References: <1479306245-14456-1-git-send-email-stefanb@linux.vnet.ibm.com> <20161116153731.pmmnxiai7ouuj6qf@intel.com> <3a38ddc6-1758-ae82-3df3-9cc55906880d@linux.vnet.ibm.com> <65f392b6-5141-c726-dacb-a1649ea215de@linux.vnet.ibm.com> <20161116200759.GA19593@obsidianresearch.com> From: Stefan Berger Date: Thu, 17 Nov 2016 07:35:05 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <20161116200759.GA19593@obsidianresearch.com> X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16111712-0028-0000-0000-0000060D82B9 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00006093; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000189; SDB=6.00781920; UDB=6.00377247; IPR=6.00559425; BA=6.00004889; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00013361; XFM=3.00000011; UTC=2016-11-17 12:35:09 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16111712-0029-0000-0000-000030EAD114 Message-Id: X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-11-17_06:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609300000 definitions=main-1611170229 X-Headers-End: 1c7LuP-0002fx-7K Cc: tpmdd-devel@lists.sourceforge.net Subject: Re: [tpmdd-devel] [PATCH] tpm: vtpm_proxy: Do not access host's event log 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 X-Virus-Scanned: ClamAV using ClamSMTP On 11/16/2016 03:07 PM, Jason Gunthorpe wrote: > On Wed, Nov 16, 2016 at 12:07:23PM -0500, Stefan Berger wrote: >> The culprit seems to be 'tpm: fix the missing .owner in >> tpm_bios_measurements_ops' > That is unlikely, it is probably the patch before which calls read_log > unconditionally now. That suggests the crashing is a little random.. I ran the vtpm driver test suite (with -j32) a few times at that patch and it didn't crash. It crashes severely with later patches applied. Here's the current experimental patch that fixes these problems: iff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c index 0cb43ef..a73295a 100644 don't want to attempt to read the log there, either. I think the most straight-forward way would be to gate this whole function with a flag that only the vtpm proxy driver has: TPM_CHIP_FLAG_NO_FIRMWARE_LOG. return rc; Stefan > > tpm_read_log_acpi should check if the chip has a acpi_dev_handle > before running, but it also shouldn't crash - so there are two bugs > here I guess.. Please do that instead of the checking the virual flag. > > Jarkko, you know acpi better, we switched ppi to search starting from > the acpi_dev_handle for its data - can we do the same for event log? > >> The crash looks like this: >> [ 173.608722] [] dump_stack+0x63/0x82 >> [ 173.608722] [] iounmap.part.1+0x7f/0x90 >> [ 173.608722] [] iounmap+0x2c/0x30 >> [ 173.608722] [] acpi_os_map_cleanup.part.10+0x31/0x3e >> [ 173.608722] [] acpi_os_unmap_iomem+0xcb/0xd2 >> [ 173.608722] [] read_log+0xc8/0x19e [tpm] > This seems really strange ACPI should not crash like this - yes it > will wrongly read the log for the system into the vtpm, but that > *should* work. > > Are you doing anything special with this test like high parallism or > something? Any chance you can look at little more? The tpm code looks > OK to me, the map and unmap are properly paired - but the bad address > from the iounmap suggests the refcounting in acpi is not working or > something along those lines? > > Jason > ------------------------------------------------------------------------------ --- a/drivers/char/tpm/tpm_acpi.c +++ b/drivers/char/tpm/tpm_acpi.c @@ -56,6 +56,9 @@ int tpm_read_log_acpi(struct tpm_chip *chip) log = &chip->log; + if (!chip->acpi_dev_handle) + return 0; + // So ACPI is not supported on this device, but ACPI support is compiled in. I am returning 0 here, assuming it's not an OF device and the corresponding OF function need not be called (see below). /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */ status = acpi_get_table(ACPI_SIG_TCPA, 1, (struct acpi_table_header **)&buff); diff --git a/drivers/char/tpm/tpm_eventlog.c b/drivers/char/tpm/tpm_eventlog.c index fb603a7..12b0356 100644 --- a/drivers/char/tpm/tpm_eventlog.c +++ b/drivers/char/tpm/tpm_eventlog.c @@ -380,7 +380,8 @@ static int tpm_read_log(struct tpm_chip *chip) if ((rc == 0) || (rc == -ENOMEM)) return rc; - rc = tpm_read_log_of(chip); + if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL)) + rc = tpm_read_log_of(chip); // I am not sure how to handle this case, in case we get here, which would only be on an OF device (following 'return 0;' above), but we