From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753898AbcDSKAk (ORCPT ); Tue, 19 Apr 2016 06:00:40 -0400 Received: from mga09.intel.com ([134.134.136.24]:14486 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753238AbcDSKAi (ORCPT ); Tue, 19 Apr 2016 06:00:38 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,506,1455004800"; d="scan'208";a="787812963" Date: Tue, 19 Apr 2016 13:00:27 +0300 From: Jarkko Sakkinen To: Peter Huewe Cc: linux-security-module@vger.kernel.org, stable@vger.kernel.org, Marcel Selhorst , Jason Gunthorpe , "moderated list:TPM DEVICE DRIVER" , open list Subject: Re: [PATCH] tpm_crb: fix mapping of the buffers Message-ID: <20160419100027.GA10080@intel.com> References: <1461059658-8884-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: <1461059658-8884-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.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 19, 2016 at 12:54:18PM +0300, Jarkko Sakkinen wrote: > On my Lenovo x250 the following situation occurs: > > [18697.813871] tpm_crb MSFT0101:00: can't request region for resource > [mem 0xacdff080-0xacdfffff] > > The mapping of the control area overlaps the mapping of the command > buffer. The control area is mapped over page, which is not right. It > should mapped over sizeof(struct crb_control_area). > > Fixing this issue unmasks another issue. Command and response buffers > can overlap and they do interleave on this machine. According to the PTP > specification the overlapping means that they are mapped to the same > buffer. > > The commit has been also on a Haswell NUC where things worked before > applying this fix so that the both code paths for response buffer > initialization are tested. > > Cc: stable@vger.kernel.org > Fixes: 1bd047be37d9 ("tpm_crb: Use devm_ioremap_resource") > Signed-off-by: Jarkko Sakkinen Forgot to add --subject-prefix="PATCH v2", sorry. /Jarkko > --- > drivers/char/tpm/tpm_crb.c | 39 ++++++++++++++++++++++++++++----------- > 1 file changed, 28 insertions(+), 11 deletions(-) > > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c > index 733cd0e..5afe684 100644 > --- a/drivers/char/tpm/tpm_crb.c > +++ b/drivers/char/tpm/tpm_crb.c > @@ -259,7 +259,10 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv, > struct list_head resources; > struct resource io_res; > struct device *dev = &device->dev; > - u64 pa; > + u64 cmd_pa; > + u32 cmd_size; > + u64 rsp_pa; > + u32 rsp_size; > int ret; > > INIT_LIST_HEAD(&resources); > @@ -280,22 +283,36 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv, > return PTR_ERR(priv->iobase); > > priv->cca = crb_map_res(dev, priv, &io_res, buf->control_address, > - 0x1000); > + sizeof(struct crb_control_area)); > if (IS_ERR(priv->cca)) > return PTR_ERR(priv->cca); > > - pa = ((u64) ioread32(&priv->cca->cmd_pa_high) << 32) | > - (u64) ioread32(&priv->cca->cmd_pa_low); > - priv->cmd = crb_map_res(dev, priv, &io_res, pa, > - ioread32(&priv->cca->cmd_size)); > + cmd_pa = ((u64) ioread32(&priv->cca->cmd_pa_high) << 32) | > + (u64) ioread32(&priv->cca->cmd_pa_low); > + cmd_size = ioread32(&priv->cca->cmd_size); > + priv->cmd = crb_map_res(dev, priv, &io_res, cmd_pa, cmd_size); > if (IS_ERR(priv->cmd)) > return PTR_ERR(priv->cmd); > > - memcpy_fromio(&pa, &priv->cca->rsp_pa, 8); > - pa = le64_to_cpu(pa); > - priv->rsp = crb_map_res(dev, priv, &io_res, pa, > - ioread32(&priv->cca->rsp_size)); > - return PTR_ERR_OR_ZERO(priv->rsp); > + memcpy_fromio(&rsp_pa, &priv->cca->rsp_pa, 8); > + rsp_pa = le64_to_cpu(rsp_pa); > + rsp_size = ioread32(&priv->cca->rsp_size); > + > + if (cmd_pa != rsp_pa) { > + priv->rsp = crb_map_res(dev, priv, &io_res, rsp_pa, rsp_size); > + return PTR_ERR_OR_ZERO(priv->rsp); > + } > + > + /* According to the PTP specification, overlapping command and response > + * buffer sizes must be identical. > + */ > + if (cmd_size != rsp_size) { > + dev_err(dev, FW_BUG "overlapping command and response buffer sizes are not identical"); > + return -EINVAL; > + } > + > + priv->rsp = priv->cmd; > + return 0; > } > > static int crb_acpi_add(struct acpi_device *device) > -- > 2.7.4 > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 19 Apr 2016 13:00:27 +0300 From: Jarkko Sakkinen To: Peter Huewe Cc: linux-security-module@vger.kernel.org, stable@vger.kernel.org, Marcel Selhorst , Jason Gunthorpe , "moderated list:TPM DEVICE DRIVER" , open list Subject: Re: [PATCH] tpm_crb: fix mapping of the buffers Message-ID: <20160419100027.GA10080@intel.com> References: <1461059658-8884-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: <1461059658-8884-1-git-send-email-jarkko.sakkinen@linux.intel.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: On Tue, Apr 19, 2016 at 12:54:18PM +0300, Jarkko Sakkinen wrote: > On my Lenovo x250 the following situation occurs: > > [18697.813871] tpm_crb MSFT0101:00: can't request region for resource > [mem 0xacdff080-0xacdfffff] > > The mapping of the control area overlaps the mapping of the command > buffer. The control area is mapped over page, which is not right. It > should mapped over sizeof(struct crb_control_area). > > Fixing this issue unmasks another issue. Command and response buffers > can overlap and they do interleave on this machine. According to the PTP > specification the overlapping means that they are mapped to the same > buffer. > > The commit has been also on a Haswell NUC where things worked before > applying this fix so that the both code paths for response buffer > initialization are tested. > > Cc: stable@vger.kernel.org > Fixes: 1bd047be37d9 ("tpm_crb: Use devm_ioremap_resource") > Signed-off-by: Jarkko Sakkinen Forgot to add --subject-prefix="PATCH v2", sorry. /Jarkko > --- > drivers/char/tpm/tpm_crb.c | 39 ++++++++++++++++++++++++++++----------- > 1 file changed, 28 insertions(+), 11 deletions(-) > > diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c > index 733cd0e..5afe684 100644 > --- a/drivers/char/tpm/tpm_crb.c > +++ b/drivers/char/tpm/tpm_crb.c > @@ -259,7 +259,10 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv, > struct list_head resources; > struct resource io_res; > struct device *dev = &device->dev; > - u64 pa; > + u64 cmd_pa; > + u32 cmd_size; > + u64 rsp_pa; > + u32 rsp_size; > int ret; > > INIT_LIST_HEAD(&resources); > @@ -280,22 +283,36 @@ static int crb_map_io(struct acpi_device *device, struct crb_priv *priv, > return PTR_ERR(priv->iobase); > > priv->cca = crb_map_res(dev, priv, &io_res, buf->control_address, > - 0x1000); > + sizeof(struct crb_control_area)); > if (IS_ERR(priv->cca)) > return PTR_ERR(priv->cca); > > - pa = ((u64) ioread32(&priv->cca->cmd_pa_high) << 32) | > - (u64) ioread32(&priv->cca->cmd_pa_low); > - priv->cmd = crb_map_res(dev, priv, &io_res, pa, > - ioread32(&priv->cca->cmd_size)); > + cmd_pa = ((u64) ioread32(&priv->cca->cmd_pa_high) << 32) | > + (u64) ioread32(&priv->cca->cmd_pa_low); > + cmd_size = ioread32(&priv->cca->cmd_size); > + priv->cmd = crb_map_res(dev, priv, &io_res, cmd_pa, cmd_size); > if (IS_ERR(priv->cmd)) > return PTR_ERR(priv->cmd); > > - memcpy_fromio(&pa, &priv->cca->rsp_pa, 8); > - pa = le64_to_cpu(pa); > - priv->rsp = crb_map_res(dev, priv, &io_res, pa, > - ioread32(&priv->cca->rsp_size)); > - return PTR_ERR_OR_ZERO(priv->rsp); > + memcpy_fromio(&rsp_pa, &priv->cca->rsp_pa, 8); > + rsp_pa = le64_to_cpu(rsp_pa); > + rsp_size = ioread32(&priv->cca->rsp_size); > + > + if (cmd_pa != rsp_pa) { > + priv->rsp = crb_map_res(dev, priv, &io_res, rsp_pa, rsp_size); > + return PTR_ERR_OR_ZERO(priv->rsp); > + } > + > + /* According to the PTP specification, overlapping command and response > + * buffer sizes must be identical. > + */ > + if (cmd_size != rsp_size) { > + dev_err(dev, FW_BUG "overlapping command and response buffer sizes are not identical"); > + return -EINVAL; > + } > + > + priv->rsp = priv->cmd; > + return 0; > } > > static int crb_acpi_add(struct acpi_device *device) > -- > 2.7.4 >