linux-kernel - Re: [PATCH 2/2] CHROMIUM: tpm: tpm_i2c_infineon: Lock the I2C adapter for a sequence of requests.

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]

Message-ID: <20120613093849.Horde.mruRClm5A05P2EOJUErGqvA@mail.selhorst.net>
Date:	Wed, 13 Jun 2012 09:38:49 +0200
From:	Marcel Selhorst <tpmdd@...horst.net>
To:	Peter Huewe <peter.huewe@...ineon.com>
Cc:	srajiv@...ux.vnet.ibm.com, tpmdd-devel@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org, Olof Johansson <olof@...om.net>,
	Luigi Semenzato <semenzato@...gle.com>, andi.shyti@...il.com,
	Bryan Freed <bfreed@...omium.org>
Subject: Re: [PATCH 2/2] CHROMIUM: tpm: tpm_i2c_infineon: Lock the I2C
 adapter for a sequence of requests.


Signed-off-by: Marcel Selhorst <tpmdd@...horst.net>

Zitat von Peter Huewe <peter.huewe@...ineon.com>:

> From: Bryan Freed <bfreed@...omium.org>
>
> This is derived from Peter Huewe's recommended fix:
>
> On some ChromeOS systems, a TPM sharing the I2C bus with another device
> gets confused when it sees I2C requests to that other device.
> This change locks the I2C adapter for the duration of the full sequence
> of I2C requests the TPM needs to complete.
>
> smbus_xfer is not supported, but SMBUS is not supported by the original
> driver, either.
>
> Signed-off-by: Bryan Freed <bfreed@...omium.org>
> Signed-off-by: Peter Huewe <peter.huewe@...ineon.com>
> ---
>  drivers/char/tpm/tpm_i2c_infineon.c |   45  
> +++++++++++++++++++++++++++++++---
>  1 files changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_i2c_infineon.c  
> b/drivers/char/tpm/tpm_i2c_infineon.c
> index 54b9a5e..1794a09 100644
> --- a/drivers/char/tpm/tpm_i2c_infineon.c
> +++ b/drivers/char/tpm/tpm_i2c_infineon.c
> @@ -68,6 +68,31 @@ struct tpm_inf_dev {
>  static struct tpm_inf_dev tpm_dev;
>  static struct i2c_driver tpm_tis_i2c_driver;
>
> +
> +/*
> + * Copy i2c-core:i2c_transfer() as close as possible without the  
> adapter locks
> + * and algorithm check.  These are done by the caller for atomicity.
> + * Unfortunately we have to use this as a workaround in multislave  
> environments
> + * as no other suitable and working mechanism is available.
> + */
> +static int i2c_transfer_nolock(struct i2c_adapter *adap, struct  
> i2c_msg *msgs,
> +			       int num)
> +{
> +	unsigned long orig_jiffies;
> +	int ret, try;
> +
> +	/* Retry automatically on arbitration loss */
> +	orig_jiffies = jiffies;
> +	for (ret = 0, try = 0; try <= adap->retries; try++) {
> +		ret = adap->algo->master_xfer(adap, msgs, num);
> +		if (ret != -EAGAIN)
> +			break;
> +		if (time_after(jiffies, orig_jiffies + adap->timeout))
> +			break;
> +	}
> +	return ret;
> +}
> +
>  /*
>   * iic_tpm_read() - read from TPM register
>   * @addr: register address to read from
> @@ -96,8 +121,13 @@ static int iic_tpm_read(u8 addr, u8 *buffer, size_t len)
>  	int rc;
>  	int count;
>
> +	/* Lock the adapter for the duration of the whole sequence. */
> +	if (!tpm_dev.client->adapter->algo->master_xfer)
> +		return -EOPNOTSUPP;
> +	i2c_lock_adapter(tpm_dev.client->adapter);
> +
>  	for (count = 0; count < MAX_COUNT; count++) {
> -		rc = i2c_transfer(tpm_dev.client->adapter, &msg1, 1);
> +		rc = i2c_transfer_nolock(tpm_dev.client->adapter, &msg1, 1);
>  		if (rc > 0)
>  			break;	/* break here to skip sleep */
>
> @@ -105,19 +135,21 @@ static int iic_tpm_read(u8 addr, u8 *buffer,  
> size_t len)
>  	}
>
>  	if (rc <= 0)
> -		return -EIO;
> +		goto out;
>
>  	/* After the TPM has successfully received the register address it needs
>  	 * some time, thus we're sleeping here again, before retrieving the data
>  	 */
>  	for (count = 0; count < MAX_COUNT; count++) {
>  		usleep_range(SLEEP_DURATION_LOW, SLEEP_DURATION_HI);
> -		rc = i2c_transfer(tpm_dev.client->adapter, &msg2, 1);
> +		rc = i2c_transfer_nolock(tpm_dev.client->adapter, &msg2, 1);
>  		if (rc > 0)
>  			break;
>
>  	}
>
> +out:
> +	i2c_unlock_adapter(tpm_dev.client->adapter);
>  	if (rc <= 0)
>  		return -EIO;
>
> @@ -136,6 +168,10 @@ static int iic_tpm_write_generic(u8 addr, u8  
> *buffer, size_t len,
>  	if (len > TPM_BUFSIZE)
>  		return -EINVAL;
>
> +	if (!tpm_dev.client->adapter->algo->master_xfer)
> +		return -EOPNOTSUPP;
> +	i2c_lock_adapter(tpm_dev.client->adapter);
> +
>  	/* prepend the 'register address' to the buffer */
>  	tpm_dev.buf[0] = addr;
>  	memcpy(&(tpm_dev.buf[1]), buffer, len);
> @@ -145,13 +181,14 @@ static int iic_tpm_write_generic(u8 addr, u8  
> *buffer, size_t len,
>  	 * cannot rely on the standard behavior of i2c_transfer.
>  	 */
>  	for (count = 0; count < max_count; count++) {
> -		rc = i2c_transfer(tpm_dev.client->adapter, &msg1, 1);
> +		rc = i2c_transfer_nolock(tpm_dev.client->adapter, &msg1, 1);
>  		if (rc > 0)
>  			break;
>
>  		usleep_range(sleep_low, sleep_hi);
>  	}
>
> +	i2c_unlock_adapter(tpm_dev.client->adapter);
>  	if (rc <= 0)
>  		return -EIO;
>
> --
> 1.7.6.msysgit.0



--
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/