linux-kernel - Re: [PATCH v2 1/4] tpm: ignore burstcount to improve tpm

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: <20170906161246.GA9747@obsidianresearch.com>
Date:   Wed, 6 Sep 2017 10:12:46 -0600
From:   Jason Gunthorpe <jgunthorpe@...idianresearch.com>
To:     Nayna Jain <nayna@...ux.vnet.ibm.com>
Cc:     tpmdd-devel@...ts.sourceforge.net, peterhuewe@....de,
        tpmdd@...horst.net, jarkko.sakkinen@...ux.intel.com,
        linux-security-module@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        linux-ima-devel@...ts.sourceforge.net, patrickc@...ibm.com
Subject: Re: [PATCH v2 1/4] tpm: ignore burstcount to improve tpm_tis send()
 performance.

On Wed, Sep 06, 2017 at 08:56:36AM -0400, Nayna Jain wrote:

> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 4e303be83df6..3c59bb91e1ee 100644
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1465,6 +1465,14 @@
>  			mode generally follows that for the NaN encoding,
>  			except where unsupported by hardware.
>  
> +	ignore_burst_count [TPM_TIS_CORE]
> +			tpm_tis_core driver queries for the burstcount before
> +			every send call in a loop. However, it causes delay to
> +			the send command for TPMs with low burstcount value.
> +			Setting this value to 1, will make driver to query for
> +			burstcount only once in the loop to improve the
> +			performance. By default, its value is set to 0.

Really don't want to see a kernel command line parameter for this..

Please figure out a different approach or at least a better name..

> +		/*
> +		 * Get the initial burstcount to ensure TPM is ready to
> +		 * accept data, even when waiting for burstcount is disabled.
> +		 */
>  		burstcnt = get_burstcount(chip);
>  		if (burstcnt < 0) {
>  			dev_err(&chip->dev, "Unable to read burstcount\n");
>  			rc = burstcnt;
>  			goto out_err;
>  		}
> -		burstcnt = min_t(int, burstcnt, len - count - 1);
> +
> +		if (ignore_burst_count)
> +			sendcnt = len - 1;
> +		else
> +			sendcnt = min_t(int, burstcnt, len - count - 1);
> +
>  		rc = tpm_tis_write_bytes(priv, TPM_DATA_FIFO(priv->locality),
> -					 burstcnt, buf + count);
> +					 sendcnt, buf + count);

The problem with this approach is that the TPM could totally block the CPU for
very long periods of time.

It seems very risky to enable..

Jason