diff mbox series

[v10,RESEND,4/6] tpm: move tpm_chip definition to include/linux/tpm.h

Message ID 20190206162452.7749-5-roberto.sassu@huawei.com (mailing list archive)
State New, archived
Headers show
Series tpm: retrieve digest size of unknown algorithms from TPM | expand

Commit Message

Roberto Sassu Feb. 6, 2019, 4:24 p.m. UTC
The tpm_chip structure contains the list of PCR banks currently allocated
in the TPM. When support for crypto agility will be added to the TPM
driver, users of the driver have to provide a digest for each allocated
bank to tpm_pcr_extend(). With this patch, they can obtain the PCR bank
algorithms directly from chip->allocated_banks.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm.h | 101 ++---------------------------------------
 include/linux/tpm.h    |  91 +++++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+), 97 deletions(-)

Comments

Nathan Chancellor Feb. 8, 2019, 4:24 a.m. UTC | #1
On Wed, Feb 06, 2019 at 05:24:50PM +0100, Roberto Sassu wrote:
> The tpm_chip structure contains the list of PCR banks currently allocated
> in the TPM. When support for crypto agility will be added to the TPM
> driver, users of the driver have to provide a digest for each allocated
> bank to tpm_pcr_extend(). With this patch, they can obtain the PCR bank
> algorithms directly from chip->allocated_banks.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  drivers/char/tpm/tpm.h | 101 ++---------------------------------------
>  include/linux/tpm.h    |  91 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 95 insertions(+), 97 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 4efa304e9ece..4f85ce909122 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -25,30 +25,22 @@
>  
>  #include <linux/module.h>
>  #include <linux/delay.h>
> -#include <linux/fs.h>
> -#include <linux/hw_random.h>
>  #include <linux/mutex.h>
>  #include <linux/sched.h>
>  #include <linux/platform_device.h>
>  #include <linux/io.h>
>  #include <linux/tpm.h>
> -#include <linux/acpi.h>
> -#include <linux/cdev.h>
>  #include <linux/highmem.h>
>  #include <linux/tpm_eventlog.h>
> -#include <crypto/hash_info.h>
>  
>  #ifdef CONFIG_X86
>  #include <asm/intel-family.h>
>  #endif
>  
> -enum tpm_const {
> -	TPM_MINOR = 224,	/* officially assigned */
> -	TPM_BUFSIZE = 4096,
> -	TPM_NUM_DEVICES = 65536,
> -	TPM_RETRY = 50,		/* 5 seconds */
> -	TPM_NUM_EVENT_LOG_FILES = 3,
> -};
> +#define TPM_MINOR		224	/* officially assigned */
> +#define TPM_BUFSIZE		4096
> +#define TPM_NUM_DEVICES		65536
> +#define TPM_RETRY		50
>  
>  enum tpm_timeout {
>  	TPM_TIMEOUT = 5,	/* msecs */
> @@ -65,16 +57,6 @@ enum tpm_addr {
>  	TPM_ADDR = 0x4E,
>  };
>  
> -/* Indexes the duration array */
> -enum tpm_duration {
> -	TPM_SHORT = 0,
> -	TPM_MEDIUM = 1,
> -	TPM_LONG = 2,
> -	TPM_LONG_LONG = 3,
> -	TPM_UNDEFINED,
> -	TPM_NUM_DURATIONS = TPM_UNDEFINED,
> -};
> -
>  #define TPM_WARN_RETRY          0x800
>  #define TPM_WARN_DOING_SELFTEST 0x802
>  #define TPM_ERR_DEACTIVATED     0x6
> @@ -179,15 +161,6 @@ enum tpm2_cc_attrs {
>  #define TPM_VID_WINBOND  0x1050
>  #define TPM_VID_STM      0x104A
>  
> -#define TPM_PPI_VERSION_LEN		3
> -
> -struct tpm_space {
> -	u32 context_tbl[3];
> -	u8 *context_buf;
> -	u32 session_tbl[3];
> -	u8 *session_buf;
> -};
> -
>  enum tpm_chip_flags {
>  	TPM_CHIP_FLAG_TPM2		= BIT(1),
>  	TPM_CHIP_FLAG_IRQ		= BIT(2),
> @@ -196,72 +169,6 @@ enum tpm_chip_flags {
>  	TPM_CHIP_FLAG_ALWAYS_POWERED	= BIT(5),
>  };
>  
> -struct tpm_bios_log {
> -	void *bios_event_log;
> -	void *bios_event_log_end;
> -};
> -
> -struct tpm_chip_seqops {
> -	struct tpm_chip *chip;
> -	const struct seq_operations *seqops;
> -};
> -
> -struct tpm_chip {
> -	struct device dev;
> -	struct device devs;
> -	struct cdev cdev;
> -	struct cdev cdevs;
> -
> -	/* A driver callback under ops cannot be run unless ops_sem is held
> -	 * (sometimes implicitly, eg for the sysfs code). ops becomes null
> -	 * when the driver is unregistered, see tpm_try_get_ops.
> -	 */
> -	struct rw_semaphore ops_sem;
> -	const struct tpm_class_ops *ops;
> -
> -	struct tpm_bios_log log;
> -	struct tpm_chip_seqops bin_log_seqops;
> -	struct tpm_chip_seqops ascii_log_seqops;
> -
> -	unsigned int flags;
> -
> -	int dev_num;		/* /dev/tpm# */
> -	unsigned long is_open;	/* only one allowed */
> -
> -	char hwrng_name[64];
> -	struct hwrng hwrng;
> -
> -	struct mutex tpm_mutex;	/* tpm is processing */
> -
> -	unsigned long timeout_a; /* jiffies */
> -	unsigned long timeout_b; /* jiffies */
> -	unsigned long timeout_c; /* jiffies */
> -	unsigned long timeout_d; /* jiffies */
> -	bool timeout_adjusted;
> -	unsigned long duration[TPM_NUM_DURATIONS]; /* jiffies */
> -	bool duration_adjusted;
> -
> -	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
> -
> -	const struct attribute_group *groups[3];
> -	unsigned int groups_cnt;
> -
> -	u32 nr_allocated_banks;
> -	struct tpm_bank_info *allocated_banks;
> -#ifdef CONFIG_ACPI
> -	acpi_handle acpi_dev_handle;
> -	char ppi_version[TPM_PPI_VERSION_LEN + 1];
> -#endif /* CONFIG_ACPI */
> -
> -	struct tpm_space work_space;
> -	u32 last_cc;
> -	u32 nr_commands;
> -	u32 *cc_attrs_tbl;
> -
> -	/* active locality */
> -	int locality;
> -};
> -
>  #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
>  
>  struct tpm_header {
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index afd022fc9d3d..816e686a73ac 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -22,6 +22,10 @@
>  #ifndef __LINUX_TPM_H__
>  #define __LINUX_TPM_H__
>  
> +#include <linux/hw_random.h>
> +#include <linux/acpi.h>
> +#include <linux/cdev.h>
> +#include <linux/fs.h>
>  #include <crypto/hash_info.h>
>  
>  #define TPM_DIGEST_SIZE 20	/* Max TPM v1.2 PCR size */
> @@ -75,6 +79,93 @@ struct tpm_class_ops {
>  	void (*clk_enable)(struct tpm_chip *chip, bool value);
>  };
>  
> +#define TPM_NUM_EVENT_LOG_FILES		3
> +
> +/* Indexes the duration array */
> +enum tpm_duration {
> +	TPM_SHORT = 0,
> +	TPM_MEDIUM = 1,
> +	TPM_LONG = 2,
> +	TPM_LONG_LONG = 3,
> +	TPM_UNDEFINED,
> +	TPM_NUM_DURATIONS = TPM_UNDEFINED,
> +};
> +
> +#define TPM_PPI_VERSION_LEN		3
> +
> +struct tpm_space {
> +	u32 context_tbl[3];
> +	u8 *context_buf;
> +	u32 session_tbl[3];
> +	u8 *session_buf;
> +};
> +
> +struct tpm_bios_log {
> +	void *bios_event_log;
> +	void *bios_event_log_end;
> +};
> +
> +struct tpm_chip_seqops {
> +	struct tpm_chip *chip;
> +	const struct seq_operations *seqops;
> +};
> +
> +struct tpm_chip {
> +	struct device dev;
> +	struct device devs;
> +	struct cdev cdev;
> +	struct cdev cdevs;
> +
> +	/* A driver callback under ops cannot be run unless ops_sem is held
> +	 * (sometimes implicitly, eg for the sysfs code). ops becomes null
> +	 * when the driver is unregistered, see tpm_try_get_ops.
> +	 */
> +	struct rw_semaphore ops_sem;
> +	const struct tpm_class_ops *ops;
> +
> +	struct tpm_bios_log log;
> +	struct tpm_chip_seqops bin_log_seqops;
> +	struct tpm_chip_seqops ascii_log_seqops;
> +
> +	unsigned int flags;
> +
> +	int dev_num;		/* /dev/tpm# */
> +	unsigned long is_open;	/* only one allowed */
> +
> +	char hwrng_name[64];
> +	struct hwrng hwrng;
> +
> +	struct mutex tpm_mutex;	/* tpm is processing */
> +
> +	unsigned long timeout_a; /* jiffies */
> +	unsigned long timeout_b; /* jiffies */
> +	unsigned long timeout_c; /* jiffies */
> +	unsigned long timeout_d; /* jiffies */
> +	bool timeout_adjusted;
> +	unsigned long duration[TPM_NUM_DURATIONS]; /* jiffies */
> +	bool duration_adjusted;
> +
> +	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
> +
> +	const struct attribute_group *groups[3];
> +	unsigned int groups_cnt;
> +
> +	u32 nr_allocated_banks;
> +	struct tpm_bank_info *allocated_banks;
> +#ifdef CONFIG_ACPI
> +	acpi_handle acpi_dev_handle;
> +	char ppi_version[TPM_PPI_VERSION_LEN + 1];
> +#endif /* CONFIG_ACPI */
> +
> +	struct tpm_space work_space;
> +	u32 last_cc;
> +	u32 nr_commands;
> +	u32 *cc_attrs_tbl;
> +
> +	/* active locality */
> +	int locality;
> +};
> +
>  #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
>  
>  extern int tpm_is_tpm2(struct tpm_chip *chip);
> -- 
> 2.17.1
> 

Hi Robert,

This patch causes a build error with Clang (bisected on next-20190207):

security/integrity/ima/ima.h:191:2: error: redefinition of enumerator 'NONE'
        __ima_hooks(__ima_hook_enumify)
        ^
security/integrity/ima/ima.h:176:7: note: expanded from macro '__ima_hooks'
        hook(NONE)                      \
             ^
include/linux/efi.h:1709:2: note: previous definition is here
        NONE,
        ^
1 error generated.

I am not sure how to reconcile this otherwise I would have sent a patch.

Thanks,
Nathan
Roberto Sassu Feb. 8, 2019, 8:41 a.m. UTC | #2
On 2/8/2019 5:24 AM, Nathan Chancellor wrote:
>> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
>> index afd022fc9d3d..816e686a73ac 100644
>> --- a/include/linux/tpm.h
>> +++ b/include/linux/tpm.h
>> @@ -22,6 +22,10 @@
>>   #ifndef __LINUX_TPM_H__
>>   #define __LINUX_TPM_H__
>>   
>> +#include <linux/hw_random.h>
>> +#include <linux/acpi.h>

Hi Nathan

I think the error comes from the line above.
security/integrity/ima/ima.h includes <linux/tpm.h>, which now includes
<linux/acpi.h>, which includes <asm/acpi.h>, which includes
<linux/efi.h> (for the arm64 architecture only). Both ima.h and efi.h
define 'NONE'.

The solution would be to rename one of them. I'm not familiar with the
EFI part. Renaming 'NONE' in IMA should be easy as it is not used
anywhere.

Thanks

Roberto


>> +#include <linux/cdev.h>
>> +#include <linux/fs.h>
>>   #include <crypto/hash_info.h>
>>   
>>   #define TPM_DIGEST_SIZE 20	/* Max TPM v1.2 PCR size */
>> @@ -75,6 +79,93 @@ struct tpm_class_ops {
>>   	void (*clk_enable)(struct tpm_chip *chip, bool value);
>>   };
>>   
>> +#define TPM_NUM_EVENT_LOG_FILES		3
>> +
>> +/* Indexes the duration array */
>> +enum tpm_duration {
>> +	TPM_SHORT = 0,
>> +	TPM_MEDIUM = 1,
>> +	TPM_LONG = 2,
>> +	TPM_LONG_LONG = 3,
>> +	TPM_UNDEFINED,
>> +	TPM_NUM_DURATIONS = TPM_UNDEFINED,
>> +};
>> +
>> +#define TPM_PPI_VERSION_LEN		3
>> +
>> +struct tpm_space {
>> +	u32 context_tbl[3];
>> +	u8 *context_buf;
>> +	u32 session_tbl[3];
>> +	u8 *session_buf;
>> +};
>> +
>> +struct tpm_bios_log {
>> +	void *bios_event_log;
>> +	void *bios_event_log_end;
>> +};
>> +
>> +struct tpm_chip_seqops {
>> +	struct tpm_chip *chip;
>> +	const struct seq_operations *seqops;
>> +};
>> +
>> +struct tpm_chip {
>> +	struct device dev;
>> +	struct device devs;
>> +	struct cdev cdev;
>> +	struct cdev cdevs;
>> +
>> +	/* A driver callback under ops cannot be run unless ops_sem is held
>> +	 * (sometimes implicitly, eg for the sysfs code). ops becomes null
>> +	 * when the driver is unregistered, see tpm_try_get_ops.
>> +	 */
>> +	struct rw_semaphore ops_sem;
>> +	const struct tpm_class_ops *ops;
>> +
>> +	struct tpm_bios_log log;
>> +	struct tpm_chip_seqops bin_log_seqops;
>> +	struct tpm_chip_seqops ascii_log_seqops;
>> +
>> +	unsigned int flags;
>> +
>> +	int dev_num;		/* /dev/tpm# */
>> +	unsigned long is_open;	/* only one allowed */
>> +
>> +	char hwrng_name[64];
>> +	struct hwrng hwrng;
>> +
>> +	struct mutex tpm_mutex;	/* tpm is processing */
>> +
>> +	unsigned long timeout_a; /* jiffies */
>> +	unsigned long timeout_b; /* jiffies */
>> +	unsigned long timeout_c; /* jiffies */
>> +	unsigned long timeout_d; /* jiffies */
>> +	bool timeout_adjusted;
>> +	unsigned long duration[TPM_NUM_DURATIONS]; /* jiffies */
>> +	bool duration_adjusted;
>> +
>> +	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
>> +
>> +	const struct attribute_group *groups[3];
>> +	unsigned int groups_cnt;
>> +
>> +	u32 nr_allocated_banks;
>> +	struct tpm_bank_info *allocated_banks;
>> +#ifdef CONFIG_ACPI
>> +	acpi_handle acpi_dev_handle;
>> +	char ppi_version[TPM_PPI_VERSION_LEN + 1];
>> +#endif /* CONFIG_ACPI */
>> +
>> +	struct tpm_space work_space;
>> +	u32 last_cc;
>> +	u32 nr_commands;
>> +	u32 *cc_attrs_tbl;
>> +
>> +	/* active locality */
>> +	int locality;
>> +};
>> +
>>   #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
>>   
>>   extern int tpm_is_tpm2(struct tpm_chip *chip);
>> -- 
>> 2.17.1
>>
> 
> Hi Robert,
> 
> This patch causes a build error with Clang (bisected on next-20190207):
> 
> security/integrity/ima/ima.h:191:2: error: redefinition of enumerator 'NONE'
>          __ima_hooks(__ima_hook_enumify)
>          ^
> security/integrity/ima/ima.h:176:7: note: expanded from macro '__ima_hooks'
>          hook(NONE)                      \
>               ^
> include/linux/efi.h:1709:2: note: previous definition is here
>          NONE,
>          ^
> 1 error generated.
> 
> I am not sure how to reconcile this otherwise I would have sent a patch.
> 
> Thanks,
> Nathan
>
Nathan Chancellor Feb. 8, 2019, 4:16 p.m. UTC | #3
On Fri, Feb 08, 2019 at 09:41:14AM +0100, Roberto Sassu wrote:
> On 2/8/2019 5:24 AM, Nathan Chancellor wrote:
> > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > > index afd022fc9d3d..816e686a73ac 100644
> > > --- a/include/linux/tpm.h
> > > +++ b/include/linux/tpm.h
> > > @@ -22,6 +22,10 @@
> > >   #ifndef __LINUX_TPM_H__
> > >   #define __LINUX_TPM_H__
> > > +#include <linux/hw_random.h>
> > > +#include <linux/acpi.h>
> 
> Hi Nathan
> 
> I think the error comes from the line above.
> security/integrity/ima/ima.h includes <linux/tpm.h>, which now includes
> <linux/acpi.h>, which includes <asm/acpi.h>, which includes
> <linux/efi.h> (for the arm64 architecture only). Both ima.h and efi.h
> define 'NONE'.
> 

Thank you for providing that analysis, I appreciate it!

> The solution would be to rename one of them. I'm not familiar with the
> EFI part. Renaming 'NONE' in IMA should be easy as it is not used
> anywhere.
> 

This seems reasonable, no?

Thanks,
Nathan

========================================================================

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d213e835c498..f203a86f1f23 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -173,7 +173,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
 }
 
 #define __ima_hooks(hook)              \
-       hook(NONE)                      \
+       hook(NO_CHECK)                  \
        hook(FILE_CHECK)                \
        hook(MMAP_CHECK)                \
        hook(BPRM_CHECK)                \

> Thanks
> 
> Roberto
> 
> 
> > > +#include <linux/cdev.h>
> > > +#include <linux/fs.h>
> > >   #include <crypto/hash_info.h>
> > >   #define TPM_DIGEST_SIZE 20	/* Max TPM v1.2 PCR size */
> > > @@ -75,6 +79,93 @@ struct tpm_class_ops {
> > >   	void (*clk_enable)(struct tpm_chip *chip, bool value);
> > >   };
> > > +#define TPM_NUM_EVENT_LOG_FILES		3
> > > +
> > > +/* Indexes the duration array */
> > > +enum tpm_duration {
> > > +	TPM_SHORT = 0,
> > > +	TPM_MEDIUM = 1,
> > > +	TPM_LONG = 2,
> > > +	TPM_LONG_LONG = 3,
> > > +	TPM_UNDEFINED,
> > > +	TPM_NUM_DURATIONS = TPM_UNDEFINED,
> > > +};
> > > +
> > > +#define TPM_PPI_VERSION_LEN		3
> > > +
> > > +struct tpm_space {
> > > +	u32 context_tbl[3];
> > > +	u8 *context_buf;
> > > +	u32 session_tbl[3];
> > > +	u8 *session_buf;
> > > +};
> > > +
> > > +struct tpm_bios_log {
> > > +	void *bios_event_log;
> > > +	void *bios_event_log_end;
> > > +};
> > > +
> > > +struct tpm_chip_seqops {
> > > +	struct tpm_chip *chip;
> > > +	const struct seq_operations *seqops;
> > > +};
> > > +
> > > +struct tpm_chip {
> > > +	struct device dev;
> > > +	struct device devs;
> > > +	struct cdev cdev;
> > > +	struct cdev cdevs;
> > > +
> > > +	/* A driver callback under ops cannot be run unless ops_sem is held
> > > +	 * (sometimes implicitly, eg for the sysfs code). ops becomes null
> > > +	 * when the driver is unregistered, see tpm_try_get_ops.
> > > +	 */
> > > +	struct rw_semaphore ops_sem;
> > > +	const struct tpm_class_ops *ops;
> > > +
> > > +	struct tpm_bios_log log;
> > > +	struct tpm_chip_seqops bin_log_seqops;
> > > +	struct tpm_chip_seqops ascii_log_seqops;
> > > +
> > > +	unsigned int flags;
> > > +
> > > +	int dev_num;		/* /dev/tpm# */
> > > +	unsigned long is_open;	/* only one allowed */
> > > +
> > > +	char hwrng_name[64];
> > > +	struct hwrng hwrng;
> > > +
> > > +	struct mutex tpm_mutex;	/* tpm is processing */
> > > +
> > > +	unsigned long timeout_a; /* jiffies */
> > > +	unsigned long timeout_b; /* jiffies */
> > > +	unsigned long timeout_c; /* jiffies */
> > > +	unsigned long timeout_d; /* jiffies */
> > > +	bool timeout_adjusted;
> > > +	unsigned long duration[TPM_NUM_DURATIONS]; /* jiffies */
> > > +	bool duration_adjusted;
> > > +
> > > +	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
> > > +
> > > +	const struct attribute_group *groups[3];
> > > +	unsigned int groups_cnt;
> > > +
> > > +	u32 nr_allocated_banks;
> > > +	struct tpm_bank_info *allocated_banks;
> > > +#ifdef CONFIG_ACPI
> > > +	acpi_handle acpi_dev_handle;
> > > +	char ppi_version[TPM_PPI_VERSION_LEN + 1];
> > > +#endif /* CONFIG_ACPI */
> > > +
> > > +	struct tpm_space work_space;
> > > +	u32 last_cc;
> > > +	u32 nr_commands;
> > > +	u32 *cc_attrs_tbl;
> > > +
> > > +	/* active locality */
> > > +	int locality;
> > > +};
> > > +
> > >   #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
> > >   extern int tpm_is_tpm2(struct tpm_chip *chip);
> > > -- 
> > > 2.17.1
> > > 
> > 
> > Hi Robert,
> > 
> > This patch causes a build error with Clang (bisected on next-20190207):
> > 
> > security/integrity/ima/ima.h:191:2: error: redefinition of enumerator 'NONE'
> >          __ima_hooks(__ima_hook_enumify)
> >          ^
> > security/integrity/ima/ima.h:176:7: note: expanded from macro '__ima_hooks'
> >          hook(NONE)                      \
> >               ^
> > include/linux/efi.h:1709:2: note: previous definition is here
> >          NONE,
> >          ^
> > 1 error generated.
> > 
> > I am not sure how to reconcile this otherwise I would have sent a patch.
> > 
> > Thanks,
> > Nathan
> > 
> 
> -- 
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Bo PENG, Jian LI, Yanli SHI
Roberto Sassu Feb. 8, 2019, 4:38 p.m. UTC | #4
On 2/8/2019 5:16 PM, Nathan Chancellor wrote:
> On Fri, Feb 08, 2019 at 09:41:14AM +0100, Roberto Sassu wrote:
>> On 2/8/2019 5:24 AM, Nathan Chancellor wrote:
>>>> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
>>>> index afd022fc9d3d..816e686a73ac 100644
>>>> --- a/include/linux/tpm.h
>>>> +++ b/include/linux/tpm.h
>>>> @@ -22,6 +22,10 @@
>>>>    #ifndef __LINUX_TPM_H__
>>>>    #define __LINUX_TPM_H__
>>>> +#include <linux/hw_random.h>
>>>> +#include <linux/acpi.h>
>>
>> Hi Nathan
>>
>> I think the error comes from the line above.
>> security/integrity/ima/ima.h includes <linux/tpm.h>, which now includes
>> <linux/acpi.h>, which includes <asm/acpi.h>, which includes
>> <linux/efi.h> (for the arm64 architecture only). Both ima.h and efi.h
>> define 'NONE'.
>>
> 
> Thank you for providing that analysis, I appreciate it!

You're welcome!


>> The solution would be to rename one of them. I'm not familiar with the
>> EFI part. Renaming 'NONE' in IMA should be easy as it is not used
>> anywhere.
>>
> 
> This seems reasonable, no?

Mimi, is the patch ok?

Thanks

Roberto


> Thanks,
> Nathan
> 
> ========================================================================
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index d213e835c498..f203a86f1f23 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -173,7 +173,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
>   }
>   
>   #define __ima_hooks(hook)              \
> -       hook(NONE)                      \
> +       hook(NO_CHECK)                  \
>          hook(FILE_CHECK)                \
>          hook(MMAP_CHECK)                \
>          hook(BPRM_CHECK)                \
> 
>> Thanks
>>
>> Roberto
>>
>>
>>>> +#include <linux/cdev.h>
>>>> +#include <linux/fs.h>
>>>>    #include <crypto/hash_info.h>
>>>>    #define TPM_DIGEST_SIZE 20	/* Max TPM v1.2 PCR size */
>>>> @@ -75,6 +79,93 @@ struct tpm_class_ops {
>>>>    	void (*clk_enable)(struct tpm_chip *chip, bool value);
>>>>    };
>>>> +#define TPM_NUM_EVENT_LOG_FILES		3
>>>> +
>>>> +/* Indexes the duration array */
>>>> +enum tpm_duration {
>>>> +	TPM_SHORT = 0,
>>>> +	TPM_MEDIUM = 1,
>>>> +	TPM_LONG = 2,
>>>> +	TPM_LONG_LONG = 3,
>>>> +	TPM_UNDEFINED,
>>>> +	TPM_NUM_DURATIONS = TPM_UNDEFINED,
>>>> +};
>>>> +
>>>> +#define TPM_PPI_VERSION_LEN		3
>>>> +
>>>> +struct tpm_space {
>>>> +	u32 context_tbl[3];
>>>> +	u8 *context_buf;
>>>> +	u32 session_tbl[3];
>>>> +	u8 *session_buf;
>>>> +};
>>>> +
>>>> +struct tpm_bios_log {
>>>> +	void *bios_event_log;
>>>> +	void *bios_event_log_end;
>>>> +};
>>>> +
>>>> +struct tpm_chip_seqops {
>>>> +	struct tpm_chip *chip;
>>>> +	const struct seq_operations *seqops;
>>>> +};
>>>> +
>>>> +struct tpm_chip {
>>>> +	struct device dev;
>>>> +	struct device devs;
>>>> +	struct cdev cdev;
>>>> +	struct cdev cdevs;
>>>> +
>>>> +	/* A driver callback under ops cannot be run unless ops_sem is held
>>>> +	 * (sometimes implicitly, eg for the sysfs code). ops becomes null
>>>> +	 * when the driver is unregistered, see tpm_try_get_ops.
>>>> +	 */
>>>> +	struct rw_semaphore ops_sem;
>>>> +	const struct tpm_class_ops *ops;
>>>> +
>>>> +	struct tpm_bios_log log;
>>>> +	struct tpm_chip_seqops bin_log_seqops;
>>>> +	struct tpm_chip_seqops ascii_log_seqops;
>>>> +
>>>> +	unsigned int flags;
>>>> +
>>>> +	int dev_num;		/* /dev/tpm# */
>>>> +	unsigned long is_open;	/* only one allowed */
>>>> +
>>>> +	char hwrng_name[64];
>>>> +	struct hwrng hwrng;
>>>> +
>>>> +	struct mutex tpm_mutex;	/* tpm is processing */
>>>> +
>>>> +	unsigned long timeout_a; /* jiffies */
>>>> +	unsigned long timeout_b; /* jiffies */
>>>> +	unsigned long timeout_c; /* jiffies */
>>>> +	unsigned long timeout_d; /* jiffies */
>>>> +	bool timeout_adjusted;
>>>> +	unsigned long duration[TPM_NUM_DURATIONS]; /* jiffies */
>>>> +	bool duration_adjusted;
>>>> +
>>>> +	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
>>>> +
>>>> +	const struct attribute_group *groups[3];
>>>> +	unsigned int groups_cnt;
>>>> +
>>>> +	u32 nr_allocated_banks;
>>>> +	struct tpm_bank_info *allocated_banks;
>>>> +#ifdef CONFIG_ACPI
>>>> +	acpi_handle acpi_dev_handle;
>>>> +	char ppi_version[TPM_PPI_VERSION_LEN + 1];
>>>> +#endif /* CONFIG_ACPI */
>>>> +
>>>> +	struct tpm_space work_space;
>>>> +	u32 last_cc;
>>>> +	u32 nr_commands;
>>>> +	u32 *cc_attrs_tbl;
>>>> +
>>>> +	/* active locality */
>>>> +	int locality;
>>>> +};
>>>> +
>>>>    #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
>>>>    extern int tpm_is_tpm2(struct tpm_chip *chip);
>>>> -- 
>>>> 2.17.1
>>>>
>>>
>>> Hi Robert,
>>>
>>> This patch causes a build error with Clang (bisected on next-20190207):
>>>
>>> security/integrity/ima/ima.h:191:2: error: redefinition of enumerator 'NONE'
>>>           __ima_hooks(__ima_hook_enumify)
>>>           ^
>>> security/integrity/ima/ima.h:176:7: note: expanded from macro '__ima_hooks'
>>>           hook(NONE)                      \
>>>                ^
>>> include/linux/efi.h:1709:2: note: previous definition is here
>>>           NONE,
>>>           ^
>>> 1 error generated.
>>>
>>> I am not sure how to reconcile this otherwise I would have sent a patch.
>>>
>>> Thanks,
>>> Nathan
>>>
>>
>> -- 
>> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
>> Managing Director: Bo PENG, Jian LI, Yanli SHI
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4efa304e9ece..4f85ce909122 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -25,30 +25,22 @@ 
 
 #include <linux/module.h>
 #include <linux/delay.h>
-#include <linux/fs.h>
-#include <linux/hw_random.h>
 #include <linux/mutex.h>
 #include <linux/sched.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/tpm.h>
-#include <linux/acpi.h>
-#include <linux/cdev.h>
 #include <linux/highmem.h>
 #include <linux/tpm_eventlog.h>
-#include <crypto/hash_info.h>
 
 #ifdef CONFIG_X86
 #include <asm/intel-family.h>
 #endif
 
-enum tpm_const {
-	TPM_MINOR = 224,	/* officially assigned */
-	TPM_BUFSIZE = 4096,
-	TPM_NUM_DEVICES = 65536,
-	TPM_RETRY = 50,		/* 5 seconds */
-	TPM_NUM_EVENT_LOG_FILES = 3,
-};
+#define TPM_MINOR		224	/* officially assigned */
+#define TPM_BUFSIZE		4096
+#define TPM_NUM_DEVICES		65536
+#define TPM_RETRY		50
 
 enum tpm_timeout {
 	TPM_TIMEOUT = 5,	/* msecs */
@@ -65,16 +57,6 @@  enum tpm_addr {
 	TPM_ADDR = 0x4E,
 };
 
-/* Indexes the duration array */
-enum tpm_duration {
-	TPM_SHORT = 0,
-	TPM_MEDIUM = 1,
-	TPM_LONG = 2,
-	TPM_LONG_LONG = 3,
-	TPM_UNDEFINED,
-	TPM_NUM_DURATIONS = TPM_UNDEFINED,
-};
-
 #define TPM_WARN_RETRY          0x800
 #define TPM_WARN_DOING_SELFTEST 0x802
 #define TPM_ERR_DEACTIVATED     0x6
@@ -179,15 +161,6 @@  enum tpm2_cc_attrs {
 #define TPM_VID_WINBOND  0x1050
 #define TPM_VID_STM      0x104A
 
-#define TPM_PPI_VERSION_LEN		3
-
-struct tpm_space {
-	u32 context_tbl[3];
-	u8 *context_buf;
-	u32 session_tbl[3];
-	u8 *session_buf;
-};
-
 enum tpm_chip_flags {
 	TPM_CHIP_FLAG_TPM2		= BIT(1),
 	TPM_CHIP_FLAG_IRQ		= BIT(2),
@@ -196,72 +169,6 @@  enum tpm_chip_flags {
 	TPM_CHIP_FLAG_ALWAYS_POWERED	= BIT(5),
 };
 
-struct tpm_bios_log {
-	void *bios_event_log;
-	void *bios_event_log_end;
-};
-
-struct tpm_chip_seqops {
-	struct tpm_chip *chip;
-	const struct seq_operations *seqops;
-};
-
-struct tpm_chip {
-	struct device dev;
-	struct device devs;
-	struct cdev cdev;
-	struct cdev cdevs;
-
-	/* A driver callback under ops cannot be run unless ops_sem is held
-	 * (sometimes implicitly, eg for the sysfs code). ops becomes null
-	 * when the driver is unregistered, see tpm_try_get_ops.
-	 */
-	struct rw_semaphore ops_sem;
-	const struct tpm_class_ops *ops;
-
-	struct tpm_bios_log log;
-	struct tpm_chip_seqops bin_log_seqops;
-	struct tpm_chip_seqops ascii_log_seqops;
-
-	unsigned int flags;
-
-	int dev_num;		/* /dev/tpm# */
-	unsigned long is_open;	/* only one allowed */
-
-	char hwrng_name[64];
-	struct hwrng hwrng;
-
-	struct mutex tpm_mutex;	/* tpm is processing */
-
-	unsigned long timeout_a; /* jiffies */
-	unsigned long timeout_b; /* jiffies */
-	unsigned long timeout_c; /* jiffies */
-	unsigned long timeout_d; /* jiffies */
-	bool timeout_adjusted;
-	unsigned long duration[TPM_NUM_DURATIONS]; /* jiffies */
-	bool duration_adjusted;
-
-	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
-
-	const struct attribute_group *groups[3];
-	unsigned int groups_cnt;
-
-	u32 nr_allocated_banks;
-	struct tpm_bank_info *allocated_banks;
-#ifdef CONFIG_ACPI
-	acpi_handle acpi_dev_handle;
-	char ppi_version[TPM_PPI_VERSION_LEN + 1];
-#endif /* CONFIG_ACPI */
-
-	struct tpm_space work_space;
-	u32 last_cc;
-	u32 nr_commands;
-	u32 *cc_attrs_tbl;
-
-	/* active locality */
-	int locality;
-};
-
 #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
 
 struct tpm_header {
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index afd022fc9d3d..816e686a73ac 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -22,6 +22,10 @@ 
 #ifndef __LINUX_TPM_H__
 #define __LINUX_TPM_H__
 
+#include <linux/hw_random.h>
+#include <linux/acpi.h>
+#include <linux/cdev.h>
+#include <linux/fs.h>
 #include <crypto/hash_info.h>
 
 #define TPM_DIGEST_SIZE 20	/* Max TPM v1.2 PCR size */
@@ -75,6 +79,93 @@  struct tpm_class_ops {
 	void (*clk_enable)(struct tpm_chip *chip, bool value);
 };
 
+#define TPM_NUM_EVENT_LOG_FILES		3
+
+/* Indexes the duration array */
+enum tpm_duration {
+	TPM_SHORT = 0,
+	TPM_MEDIUM = 1,
+	TPM_LONG = 2,
+	TPM_LONG_LONG = 3,
+	TPM_UNDEFINED,
+	TPM_NUM_DURATIONS = TPM_UNDEFINED,
+};
+
+#define TPM_PPI_VERSION_LEN		3
+
+struct tpm_space {
+	u32 context_tbl[3];
+	u8 *context_buf;
+	u32 session_tbl[3];
+	u8 *session_buf;
+};
+
+struct tpm_bios_log {
+	void *bios_event_log;
+	void *bios_event_log_end;
+};
+
+struct tpm_chip_seqops {
+	struct tpm_chip *chip;
+	const struct seq_operations *seqops;
+};
+
+struct tpm_chip {
+	struct device dev;
+	struct device devs;
+	struct cdev cdev;
+	struct cdev cdevs;
+
+	/* A driver callback under ops cannot be run unless ops_sem is held
+	 * (sometimes implicitly, eg for the sysfs code). ops becomes null
+	 * when the driver is unregistered, see tpm_try_get_ops.
+	 */
+	struct rw_semaphore ops_sem;
+	const struct tpm_class_ops *ops;
+
+	struct tpm_bios_log log;
+	struct tpm_chip_seqops bin_log_seqops;
+	struct tpm_chip_seqops ascii_log_seqops;
+
+	unsigned int flags;
+
+	int dev_num;		/* /dev/tpm# */
+	unsigned long is_open;	/* only one allowed */
+
+	char hwrng_name[64];
+	struct hwrng hwrng;
+
+	struct mutex tpm_mutex;	/* tpm is processing */
+
+	unsigned long timeout_a; /* jiffies */
+	unsigned long timeout_b; /* jiffies */
+	unsigned long timeout_c; /* jiffies */
+	unsigned long timeout_d; /* jiffies */
+	bool timeout_adjusted;
+	unsigned long duration[TPM_NUM_DURATIONS]; /* jiffies */
+	bool duration_adjusted;
+
+	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
+
+	const struct attribute_group *groups[3];
+	unsigned int groups_cnt;
+
+	u32 nr_allocated_banks;
+	struct tpm_bank_info *allocated_banks;
+#ifdef CONFIG_ACPI
+	acpi_handle acpi_dev_handle;
+	char ppi_version[TPM_PPI_VERSION_LEN + 1];
+#endif /* CONFIG_ACPI */
+
+	struct tpm_space work_space;
+	u32 last_cc;
+	u32 nr_commands;
+	u32 *cc_attrs_tbl;
+
+	/* active locality */
+	int locality;
+};
+
 #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE)
 
 extern int tpm_is_tpm2(struct tpm_chip *chip);