All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: Tomas Winkler <tomas.winkler@intel.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	Jason Gunthorpe <jgg@ziepe.ca>,
	Nayna Jain <nayna@linux.vnet.ibm.com>,
	Alexander Usyskin <alexander.usyskin@intel.com>,
	Tadeusz Struk <tadeusz.struk@intel.com>,
	linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 21/21] tpm: use u32 instead of int for PCR index
Date: Tue, 23 Oct 2018 21:44:57 +0300 (EEST)	[thread overview]
Message-ID: <alpine.DEB.2.21.1810232144440.3433@jsakkine-mobl1> (raw)
In-Reply-To: <20181019182307.17745-22-tomas.winkler@intel.com>

On Fri, 19 Oct 2018, Tomas Winkler wrote:
> The TPM specs defines PCR index as a positive number, and there is
> no reason to use a signed number. It is also a possible security
> issue as currently no functions check for a negative index,
> which may become a large number when converted to u32.
>
> Adjust the API to use u32 instead of int in all PCR related
> functions.
>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
> V3: New in the series.
> V4: Separate unrelated change to another patches.
> V5: Fix the commit message.
> V6: Rebased.
> V7: Resend.
>
> drivers/char/tpm/tpm-interface.c    |  6 +++---
> drivers/char/tpm/tpm-sysfs.c        |  2 +-
> drivers/char/tpm/tpm.h              | 10 +++++-----
> drivers/char/tpm/tpm1-cmd.c         |  6 +++---
> drivers/char/tpm/tpm2-cmd.c         |  5 ++---
> include/linux/tpm.h                 | 11 +++++++----
> security/integrity/ima/ima_crypto.c |  5 +++--
> 7 files changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index acd647476ae1..d9439f9abe78 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -38,7 +38,7 @@
>  * recently changed pcr on suspend, so force the flush
>  * with an extend to the selected _unused_ non-volatile pcr.
>  */
> -static int tpm_suspend_pcr;
> +static u32 tpm_suspend_pcr;
> module_param_named(suspend_pcr, tpm_suspend_pcr, uint, 0644);
> MODULE_PARM_DESC(suspend_pcr,
> 		 "PCR to use for dummy writes to facilitate flush on suspend.");
> @@ -455,7 +455,7 @@ EXPORT_SYMBOL_GPL(tpm_is_tpm2);
>  *
>  * Return: same as with tpm_transmit_cmd()
>  */
> -int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> +int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
> {
> 	int rc;
>
> @@ -485,7 +485,7 @@ EXPORT_SYMBOL_GPL(tpm_pcr_read);
>  *
>  * Return: same as with tpm_transmit_cmd()
>  */
> -int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
> +int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash)
> {
> 	int rc;
> 	struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index 96fc7433c57d..b88e08ec2c59 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -102,7 +102,7 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
> 	cap_t cap;
> 	u8 digest[TPM_DIGEST_SIZE];
> 	ssize_t rc;
> -	int i, j, num_pcrs;
> +	u32 i, j, num_pcrs;
> 	char *str = buf;
> 	struct tpm_chip *chip = to_tpm_chip(dev);
>
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index e0778d19da98..f27d1f38a93d 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -509,14 +509,14 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> int tpm_get_timeouts(struct tpm_chip *);
> int tpm_auto_startup(struct tpm_chip *chip);
>
> -int tpm1_pm_suspend(struct tpm_chip *chip, int tpm_suspend_pcr);
> +int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr);
> int tpm1_auto_startup(struct tpm_chip *chip);
> int tpm1_do_selftest(struct tpm_chip *chip);
> int tpm1_get_timeouts(struct tpm_chip *chip);
> unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> -int tpm1_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash,
> +int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
> 		    const char *log_msg);
> -int tpm1_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> +int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
> ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
> 		    const char *desc, size_t min_cap_length);
> int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> @@ -558,8 +558,8 @@ static inline u32 tpm2_rc_value(u32 rc)
> }
>
> int tpm2_get_timeouts(struct tpm_chip *chip);
> -int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> -int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
> +int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
> +int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
> 		    struct tpm2_digest *digests);
> int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
> void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
> diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> index 6b04648f8184..6f306338953b 100644
> --- a/drivers/char/tpm/tpm1-cmd.c
> +++ b/drivers/char/tpm/tpm1-cmd.c
> @@ -449,7 +449,7 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
> }
>
> #define TPM_ORD_PCR_EXTEND 20
> -int tpm1_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash,
> +int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
> 		    const char *log_msg)
> {
> 	struct tpm_buf buf;
> @@ -572,7 +572,7 @@ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> }
>
> #define TPM_ORD_PCRREAD 21
> -int tpm1_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> +int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
> {
> 	struct tpm_buf buf;
> 	int rc;
> @@ -729,7 +729,7 @@ int tpm1_auto_startup(struct tpm_chip *chip)
>  * * 0 on success,
>  * * < 0 on error.
>  */
> -int tpm1_pm_suspend(struct tpm_chip *chip, int tpm_suspend_pcr)
> +int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
> {
> 	u8 dummy_hash[TPM_DIGEST_SIZE] = { 0 };
> 	struct tpm_buf buf;
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 6ca4fc0a0d6f..ae86fb0218ab 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -175,7 +175,7 @@ struct tpm2_pcr_read_out {
>  *
>  * Return: Same as with tpm_transmit_cmd.
>  */
> -int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> +int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
> {
> 	int rc;
> 	struct tpm_buf buf;
> @@ -225,7 +225,7 @@ struct tpm2_null_auth_area {
>  *
>  * Return: Same as with tpm_transmit_cmd.
>  */
> -int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
> +int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
> 		    struct tpm2_digest *digests)
> {
> 	struct tpm_buf buf;
> @@ -272,7 +272,6 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
> 	return rc;
> }
>
> -
> struct tpm2_get_random_out {
> 	__be16 size;
> 	u8 buffer[TPM_MAX_RNG_DATA];
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 4609b94142d4..b49a55cf775f 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -53,8 +53,8 @@ struct tpm_class_ops {
> #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
>
> extern int tpm_is_tpm2(struct tpm_chip *chip);
> -extern int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> -extern int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
> +extern int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
> +extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash);
> extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen);
> extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
> extern int tpm_seal_trusted(struct tpm_chip *chip,
> @@ -69,15 +69,18 @@ static inline int tpm_is_tpm2(struct tpm_chip *chip)
> {
> 	return -ENODEV;
> }
> -static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> +
> +static inline int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
> {
> 	return -ENODEV;
> }
> -static inline int tpm_pcr_extend(struct tpm_chip *chip, int pcr_idx,
> +
> +static inline int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
> 				 const u8 *hash)
> {
> 	return -ENODEV;
> }
> +
> static inline int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
> {
> 	return -ENODEV;
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 7e7e7e7c250a..959d9edc113a 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -629,7 +629,7 @@ int ima_calc_buffer_hash(const void *buf, loff_t len,
> 	return calc_buffer_shash(buf, len, hash);
> }
>
> -static void __init ima_pcrread(int idx, u8 *pcr)
> +static void __init ima_pcrread(u32 idx, u8 *pcr)
> {
> 	if (!ima_tpm_chip)
> 		return;
> @@ -645,7 +645,8 @@ static int __init ima_calc_boot_aggregate_tfm(char *digest,
> 					      struct crypto_shash *tfm)
> {
> 	u8 pcr_i[TPM_DIGEST_SIZE];
> -	int rc, i;
> +	int rc;
> +	u32 i;
> 	SHASH_DESC_ON_STACK(shash, tfm);
>
> 	shash->tfm = tfm;
>

Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

  reply	other threads:[~2018-10-23 18:45 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-19 18:22 [PATCH v7 00/21] tpm: separate tpm 1.x and tpm 2.x commands Tomas Winkler
2018-10-19 18:22 ` [PATCH v7 01/21] tpm: tpm_i2c_nuvoton: use correct command duration for TPM 2.x Tomas Winkler
2018-10-20  0:51   ` Jarkko Sakkinen
2018-10-19 18:22 ` [PATCH v7 02/21] tpm2: add new tpm2 commands according to TCG 1.36 Tomas Winkler
2018-10-20  0:53   ` Jarkko Sakkinen
2018-10-19 18:22 ` [PATCH v7 03/21] tpm: sort objects in the Makefile Tomas Winkler
2018-10-19 18:22 ` [PATCH v7 04/21] tpm: factor out tpm 1.x duration calculation to tpm1-cmd.c Tomas Winkler
2018-10-20  0:56   ` Jarkko Sakkinen
2018-10-19 18:22 ` [PATCH v7 05/21] tpm: add tpm_calc_ordinal_duration() wrapper Tomas Winkler
2018-10-19 18:22 ` [PATCH v7 06/21] tpm: factor out tpm_get_timeouts() Tomas Winkler
2018-10-19 18:22 ` [PATCH v7 07/21] tpm: move tpm1_pcr_extend to tpm1-cmd.c Tomas Winkler
2018-10-19 18:22 ` [PATCH v7 08/21] tpm: move tpm_getcap " Tomas Winkler
2018-10-19 18:22 ` [PATCH v7 09/21] tpm: factor out tpm1_get_random into tpm1-cmd.c Tomas Winkler
2018-10-20  0:58   ` Jarkko Sakkinen
2018-10-23 18:42   ` Jarkko Sakkinen
2018-10-19 18:22 ` [PATCH v7 10/21] tpm: move tpm 1.x selftest code from tpm-interface.c tpm1-cmd.c Tomas Winkler
2018-10-19 18:22 ` [PATCH v7 11/21] tpm: factor out tpm 1.x pm suspend flow into tpm1-cmd.c Tomas Winkler
2018-10-20  1:05   ` Jarkko Sakkinen
2018-10-23 18:43   ` Jarkko Sakkinen
2018-10-19 18:22 ` [PATCH v7 12/21] tpm: factor out tpm_startup function Tomas Winkler
2018-10-20  1:06   ` Jarkko Sakkinen
2018-10-19 18:22 ` [PATCH v7 13/21] tpm: add tpm_auto_startup() into tpm-interface.c Tomas Winkler
2018-10-20  1:09   ` Jarkko Sakkinen
2018-10-19 18:23 ` [PATCH v7 14/21] tpm: tpm-interface.c drop unused macros Tomas Winkler
2018-10-19 18:23 ` [PATCH v7 15/21] tpm: tpm-space.c remove unneeded semicolon Tomas Winkler
2018-10-19 18:23 ` [PATCH v7 16/21] tpm: tpm1: rewrite tpm1_get_random() using tpm_buf structure Tomas Winkler
2018-10-19 18:23 ` [PATCH v7 17/21] tpm1: implement tpm1_pcr_read_dev() " Tomas Winkler
2018-10-19 18:23 ` [PATCH v7 18/21] tpm1: rename tpm1_pcr_read_dev to tpm1_pcr_read() Tomas Winkler
2018-10-23 18:44   ` Jarkko Sakkinen
2018-10-19 18:23 ` [PATCH v7 19/21] tpm1: reimplement SAVESTATE using tpm_buf Tomas Winkler
2018-10-19 18:23 ` [PATCH v7 20/21] tpm1: reimplement tpm1_continue_selftest() " Tomas Winkler
2018-10-19 18:23 ` [PATCH v7 21/21] tpm: use u32 instead of int for PCR index Tomas Winkler
2018-10-23 18:44   ` Jarkko Sakkinen [this message]
2018-10-23 18:55   ` Jarkko Sakkinen
2018-10-23 18:56     ` Winkler, Tomas
2018-10-20  1:19 ` [PATCH v7 00/21] tpm: separate tpm 1.x and tpm 2.x commands Jarkko Sakkinen
2018-10-22 17:12   ` Winkler, Tomas
2018-10-23 13:14     ` Jarkko Sakkinen
2018-10-23 13:35       ` Winkler, Tomas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.1810232144440.3433@jsakkine-mobl1 \
    --to=jarkko.sakkinen@linux.intel.com \
    --cc=alexander.usyskin@intel.com \
    --cc=jgg@ziepe.ca \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=nayna@linux.vnet.ibm.com \
    --cc=tadeusz.struk@intel.com \
    --cc=tomas.winkler@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.