linux-kernel - Re: [tpmdd-devel] [PATCH v7 08/10] tpm: TPM 2.0 CRB Interface

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 28 Nov 2014 12:23:51 -0500
From:	Stefan Berger <stefanb@...ux.vnet.ibm.com>
To:	Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
CC:	Peter Huewe <peterhuewe@....de>, Ashley Lai <ashley@...leylai.com>,
	Marcel Selhorst <tpmdd@...horst.net>,
	christophe.ricard@...il.com, josh.triplett@...el.com,
	linux-api@...r.kernel.org, linux-kernel@...r.kernel.org,
	tpmdd-devel@...ts.sourceforge.net,
	jason.gunthorpe@...idianresearch.com,
	trousers-tech@...ts.sourceforge.net
Subject: Re: [tpmdd-devel] [PATCH v7 08/10] tpm: TPM 2.0 CRB Interface

On 11/27/2014 10:40 AM, Jarkko Sakkinen wrote:
> On Wed, Nov 26, 2014 at 09:06:57AM -0500, Stefan Berger wrote:
>> On 11/11/2014 08:45 AM, Jarkko Sakkinen wrote:
>>> tpm_crb is a driver for TPM 2.0 Command Response Buffer (CRB) Interface
>>> as defined in PC Client Platform TPM Profile (PTP) Specification.
>>>
>>> Only polling and single locality is supported as these are the limitations
>>> of the available hardware, Platform Trust Techonlogy (PTT) in Haswell
>>> CPUs.
>>>
>>> The driver always applies CRB with ACPI start because PTT reports using
>>> only ACPI start as start method but as a result of my testing it requires
>>> also CRB start.
>>>
>>> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
>>> ---
>>>
[...]
>>> +static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
>>> +{
>>> +	struct crb_priv *priv = chip->vendor.priv;
>>> +	struct crb_control_area *cca;
>>> +	u8 *cmd;
>>> +	int rc = 0;
>>> +
>>> +	cca = priv->cca;
>>> +
>>> +	if (len > le32_to_cpu(cca->cmd_size)) {
>>> +		dev_err(&chip->dev,
>>> +			"invalid command count value %x %zx\n",
>>> +			(unsigned int) len,
>>> +			(size_t) le32_to_cpu(cca->cmd_size));
>>> +		return -E2BIG;
>>> +	}
>>> +
>>> +	cmd = (u8 *) ((unsigned long) cca + le64_to_cpu(cca->cmd_pa) -
>>> +		      priv->cca_pa);
>> cca = priv->cca per statement above     -> cmd = cca + x - cca = x
>>
>> -> cmd = le64_to_cpu(cca->cmd_pa);
>>
>> Should do the trick, no ?
> Virtual address might be different where CCA is ioremapped.

My bad. Is the 'Command Address' then guaranteed to lie within the 
remapped area of the 'Control Area' or should there be a check whether 
you would need a separate remapping? It's not clear from the specs where 
it lies (2.1 4th paragraph: "They can all be in system RAM, or all be in 
device memory, or any combination"). Same is true for the 'Response 
Address'.  Maybe you should have the ioremapped address for the 'Command 
Address' calculated elsewhere and simplify the address calculation here 
to basically cmd = priv->command_address.

Make the address calculations in cbr_recv and crb_send look the same.

[So, (cca - priv->cca_pa)  calculates the offset of the remapping. 
Adding to this the address of the command buffer gives the ioremapped 
address of the command buffer, assuming it lies within that remapped area. ]


     Stefan

>
>>> +	memcpy(cmd, buf, len);
>>> +
>>> +	/* Make sure that cmd is populated before issuing start. */
>>> +	wmb();
>>> +
>>> +	cca->start = cpu_to_le32(1);
>>> +	rc = crb_do_acpi_start(chip);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/