linux-kernel - Re: [PATCH 1/6] integrity: TPM internel kernel 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]

Message-ID: <4935BDB9.90806@garzik.org>
Date:	Tue, 02 Dec 2008 17:59:05 -0500
From:	Jeff Garzik <jeff@...zik.org>
To:	unlisted-recipients:; (no To-header on input)
CC:	linux-kernel@...r.kernel.org,
	Mimi Zohar <zohar@...ux.vnet.ibm.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	James Morris <jmorris@...ei.org>,
	Christoph Hellwig <hch@...radead.org>,
	Al Viro <viro@...IV.linux.org.uk>,
	David Safford <safford@...son.ibm.com>,
	Serge Hallyn <serue@...ux.vnet.ibm.com>,
	Rajiv Andrade <srajiv@...ibm.com>
Subject: Re: [PATCH 1/6] integrity: TPM internel kernel interface

Mimi Zohar wrote:
> This patch adds internal kernel support for:
>   - reading/extending a pcr value
>   - looking up the tpm_chip for a given chip number and type
> 
> Signed-off-by: Mimi Zohar <zohar@...ux.vnet.ibm.com>
> Signed-off-by: Rajiv Andrade <srajiv@...ibm.com>
> ---
> diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
> index 9c47dc4..17d2849 100644
> --- a/drivers/char/tpm/tpm.c
> +++ b/drivers/char/tpm/tpm.c
> @@ -1,11 +1,12 @@
>  /*
> - * Copyright (C) 2004 IBM Corporation
> + * Copyright (C) 2004,2007,2008 IBM Corporation
>   *
>   * Authors:
>   * Leendert van Doorn <leendert@...son.ibm.com>
>   * Dave Safford <safford@...son.ibm.com>
>   * Reiner Sailer <sailer@...son.ibm.com>
>   * Kylene Hall <kjhall@...ibm.com>
> + * Debora Velarde <dvelarde@...ibm.com>
>   *
>   * Maintained by: <tpmdd-devel@...ts.sourceforge.net>
>   *
> @@ -28,6 +29,14 @@
>  #include <linux/spinlock.h>
>  #include <linux/smp_lock.h>
>  
> +#include <linux/mm.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/crypto.h>
> +#include <linux/fs.h>
> +#include <linux/scatterlist.h>
> +#include <linux/rcupdate.h>
> +#include <asm/unaligned.h>
>  #include "tpm.h"
>  
>  enum tpm_const {
> @@ -50,6 +59,8 @@ enum tpm_duration {
>  static LIST_HEAD(tpm_chip_list);
>  static DEFINE_SPINLOCK(driver_lock);
>  static DECLARE_BITMAP(dev_mask, TPM_NUM_DEVICES);
> +#define TPM_CHIP_NUM_MASK       0x0000ffff
> +#define TPM_CHIP_TYPE_SHIFT     16
>  
>  /*
>   * Array with one entry per ordinal defining the maximum amount
> @@ -366,8 +377,7 @@ EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
>  /*
>   * Internal kernel interface to transmit TPM commands
>   */
> -static ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
> -			    size_t bufsiz)
> +ssize_t tpm_transmit(struct tpm_chip *chip, char *buf, size_t bufsiz)
>  {
>  	ssize_t rc;
>  	u32 count, ordinal;
> @@ -425,6 +435,7 @@ out:
>  	mutex_unlock(&chip->tpm_mutex);
>  	return rc;
>  }
> +EXPORT_SYMBOL_GPL(tpm_transmit);
>  
>  #define TPM_DIGEST_SIZE 20
>  #define TPM_ERROR_SIZE 10
> @@ -717,6 +728,7 @@ ssize_t tpm_show_temp_deactivated(struct device * dev,
>  }
>  EXPORT_SYMBOL_GPL(tpm_show_temp_deactivated);
>  
> +#define READ_PCR_RESULT_SIZE 30
[...]
> +#ifndef __LINUX_TPM_H__
> +#define __LINUX_TPM_H__
> +
> +#define PCI_DEVICE_ID_AMD_8111_LPC    0x7468
> +
> +/*
> + * Chip type is one of these values in the upper two bytes of chip_id
> + */
> +enum tpm_chip_type {
> +	TPM_HW_TYPE = 0x0,
> +	TPM_SW_TYPE = 0x1,
> +	TPM_ANY_TYPE = 0xFFFF,
> +};
> +
> +/*
> + * Chip num is this value or a valid tpm idx in lower two bytes of chip_id
> + */
> +enum tpm_chip_num {
> +	TPM_ANY_NUM = 0xFFFF,
> +};
> +
> +
> +#if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
> +
> +extern int tpm_pcr_read(u32 chip_id, int pcr_idx, u8 *res_buf);
> +extern int tpm_pcr_extend(u32 chip_id, int pcr_idx, const u8 *hash);
> +#endif
> +#endif


Minor nits:

* this is a bit schizophrenic with regards to defining named constants; 
sometimes enum is used, other times #define is used.  It would be better 
to just use enum, which is what the current code does (see top of 
pre-patched tpm.c)

* you really shouldn't hide PCI_DEVICE_ID_xxx constants in places other 
than pci_ids.h.

* furthermore, is that constant even used?  used in a later patch?  if 
the constant is only used once, e.g. in a pci_device_list list, then 
consider eliminating the constant altogether and directly using the PCI 
device id hexidecimal number in the target location.

	Jeff



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