| Message ID | 20230124175516.5984-2-James.Bottomley@HansenPartnership.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add integrity and security to TPM2 transactions | expand |
Hi James,
I love your patch! Perhaps something to improve:
[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5 next-20230124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
patch link: https://lore.kernel.org/r/20230124175516.5984-2-James.Bottomley%40HansenPartnership.com
patch subject: [PATCH v2 01/11] tpm: move buffer handling from static inlines to real functions
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20230125/202301250315.ZgtsNzSm-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/484b879d0bfceec899fea147a76d12f7072d9f23
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146
git checkout 484b879d0bfceec899fea147a76d12f7072d9f23
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash drivers/char/tpm/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/char/tpm/tpm-buf.c:46:5: warning: no previous prototype for 'tpm_buf_tag' [-Wmissing-prototypes]
46 | u16 tpm_buf_tag(struct tpm_buf *buf)
| ^~~~~~~~~~~
vim +/tpm_buf_tag +46 drivers/char/tpm/tpm-buf.c
45
> 46 u16 tpm_buf_tag(struct tpm_buf *buf)
47 {
48 struct tpm_header *head = (struct tpm_header *)buf->data;
49
50 return be16_to_cpu(head->tag);
51 }
52 EXPORT_SYMBOL_GPL(tpm_buf_tag);
53
On Wed, 2023-01-25 at 03:57 +0800, kernel test robot wrote: > Hi James, > > I love your patch! Perhaps something to improve: > > [auto build test WARNING on char-misc/char-misc-testing] > [also build test WARNING on char-misc/char-misc-next char-misc/char- > misc-linus zohar-integrity/next-integrity linus/master v6.2-rc5 next- > 20230124] > [If your patch is applied to the wrong git tree, kindly drop us a > note. > And when submitting patch, we suggest to use '--base' as documented > in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: > https://github.com/intel-lab-lkp/linux/commits/James-Bottomley/tpm-move-buffer-handling-from-static-inlines-to-real-functions/20230125-020146 > patch link: > https://lore.kernel.org/r/20230124175516.5984-2-James.Bottomley%40HansenPartnership.com > patch subject: [PATCH v2 01/11] tpm: move buffer handling from static > inlines to real functions > config: sparc-allyesconfig > (https://download.01.org/0day-ci/archive/20230125/202301250315.ZgtsNz > Sm-lkp@intel.com/config) > compiler: sparc64-linux-gcc (GCC) 12.1.0 > reproduce (this is a W=1 build): > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross > -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # > https://github.com/intel-lab-lkp/linux/commit/484b879d0bfceec899fea147a76d12f7072d9f23 > git remote add linux-review > https://github.com/intel-lab-lkp/linux > git fetch --no-tags linux-review James-Bottomley/tpm-move- > buffer-handling-from-static-inlines-to-real-functions/20230125-020146 > git checkout 484b879d0bfceec899fea147a76d12f7072d9f23 > # save the config file > mkdir build_dir && cp config build_dir/.config > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 > make.cross W=1 O=build_dir ARCH=sparc olddefconfig > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 > make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash > drivers/char/tpm/ > > If you fix the issue, kindly add following tag where applicable > > Reported-by: kernel test robot <lkp@intel.com> > > All warnings (new ones prefixed by >>): > > > > drivers/char/tpm/tpm-buf.c:46:5: warning: no previous prototype > > > for 'tpm_buf_tag' [-Wmissing-prototypes] > 46 | u16 tpm_buf_tag(struct tpm_buf *buf) > | ^~~~~~~~~~~ This looks like an initial header got missed in the code motion. However, the fact that nothing broke shows that tpm_buf_tag isn't used outside of the tpm-buf.c functions, so I think I can make it static to them. James
diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile index 0222b1ddb310..ad3594e383e1 100644 --- a/drivers/char/tpm/Makefile +++ b/drivers/char/tpm/Makefile @@ -15,6 +15,7 @@ tpm-y += tpm-sysfs.o tpm-y += eventlog/common.o tpm-y += eventlog/tpm1.o tpm-y += eventlog/tpm2.o +tpm-y += tpm-buf.o tpm-$(CONFIG_ACPI) += tpm_ppi.o eventlog/acpi.o tpm-$(CONFIG_EFI) += eventlog/efi.o diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c new file mode 100644 index 000000000000..4e6e8b281f6a --- /dev/null +++ b/drivers/char/tpm/tpm-buf.c @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * Handing for tpm_buf structures to facilitate the building of commands + */ + +#include <linux/module.h> +#include <linux/tpm.h> + +int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal) +{ + buf->data = (u8 *)__get_free_page(GFP_KERNEL); + if (!buf->data) + return -ENOMEM; + + buf->flags = 0; + tpm_buf_reset(buf, tag, ordinal); + return 0; +} +EXPORT_SYMBOL_GPL(tpm_buf_init); + +void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal) +{ + struct tpm_header *head = (struct tpm_header *) buf->data; + + head->tag = cpu_to_be16(tag); + head->length = cpu_to_be32(sizeof(*head)); + head->ordinal = cpu_to_be32(ordinal); +} +EXPORT_SYMBOL_GPL(tpm_buf_reset); + +void tpm_buf_destroy(struct tpm_buf *buf) +{ + free_page((unsigned long)buf->data); +} +EXPORT_SYMBOL_GPL(tpm_buf_destroy); + +u32 tpm_buf_length(struct tpm_buf *buf) +{ + struct tpm_header *head = (struct tpm_header *)buf->data; + + return be32_to_cpu(head->length); +} +EXPORT_SYMBOL_GPL(tpm_buf_length); + +u16 tpm_buf_tag(struct tpm_buf *buf) +{ + struct tpm_header *head = (struct tpm_header *)buf->data; + + return be16_to_cpu(head->tag); +} +EXPORT_SYMBOL_GPL(tpm_buf_tag); + +void tpm_buf_append(struct tpm_buf *buf, + const unsigned char *new_data, + unsigned int new_len) +{ + struct tpm_header *head = (struct tpm_header *) buf->data; + u32 len = tpm_buf_length(buf); + + /* Return silently if overflow has already happened. */ + if (buf->flags & TPM_BUF_OVERFLOW) + return; + + if ((len + new_len) > PAGE_SIZE) { + WARN(1, "tpm_buf: overflow\n"); + buf->flags |= TPM_BUF_OVERFLOW; + return; + } + + memcpy(&buf->data[len], new_data, new_len); + head->length = cpu_to_be32(len + new_len); +} +EXPORT_SYMBOL_GPL(tpm_buf_append); + +void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value) +{ + tpm_buf_append(buf, &value, 1); +} +EXPORT_SYMBOL_GPL(tpm_buf_append_u8); + +void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value) +{ + __be16 value2 = cpu_to_be16(value); + + tpm_buf_append(buf, (u8 *) &value2, 2); +} +EXPORT_SYMBOL_GPL(tpm_buf_append_u16); + +void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value) +{ + __be32 value2 = cpu_to_be32(value); + + tpm_buf_append(buf, (u8 *) &value2, 4); +} +EXPORT_SYMBOL_GPL(tpm_buf_append_u32); diff --git a/include/linux/tpm.h b/include/linux/tpm.h index dfeb25a0362d..150b39b6190e 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -322,84 +322,16 @@ struct tpm2_hash { unsigned int tpm_id; }; -static inline void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal) -{ - struct tpm_header *head = (struct tpm_header *)buf->data; - - head->tag = cpu_to_be16(tag); - head->length = cpu_to_be32(sizeof(*head)); - head->ordinal = cpu_to_be32(ordinal); -} - -static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal) -{ - buf->data = (u8 *)__get_free_page(GFP_KERNEL); - if (!buf->data) - return -ENOMEM; - - buf->flags = 0; - tpm_buf_reset(buf, tag, ordinal); - return 0; -} - -static inline void tpm_buf_destroy(struct tpm_buf *buf) -{ - free_page((unsigned long)buf->data); -} - -static inline u32 tpm_buf_length(struct tpm_buf *buf) -{ - struct tpm_header *head = (struct tpm_header *)buf->data; - - return be32_to_cpu(head->length); -} - -static inline u16 tpm_buf_tag(struct tpm_buf *buf) -{ - struct tpm_header *head = (struct tpm_header *)buf->data; - - return be16_to_cpu(head->tag); -} - -static inline void tpm_buf_append(struct tpm_buf *buf, - const unsigned char *new_data, - unsigned int new_len) -{ - struct tpm_header *head = (struct tpm_header *)buf->data; - u32 len = tpm_buf_length(buf); - - /* Return silently if overflow has already happened. */ - if (buf->flags & TPM_BUF_OVERFLOW) - return; - - if ((len + new_len) > PAGE_SIZE) { - WARN(1, "tpm_buf: overflow\n"); - buf->flags |= TPM_BUF_OVERFLOW; - return; - } - memcpy(&buf->data[len], new_data, new_len); - head->length = cpu_to_be32(len + new_len); -} - -static inline void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value) -{ - tpm_buf_append(buf, &value, 1); -} - -static inline void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value) -{ - __be16 value2 = cpu_to_be16(value); - - tpm_buf_append(buf, (u8 *) &value2, 2); -} - -static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value) -{ - __be32 value2 = cpu_to_be32(value); - - tpm_buf_append(buf, (u8 *) &value2, 4); -} +int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal); +void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal); +void tpm_buf_destroy(struct tpm_buf *buf); +u32 tpm_buf_length(struct tpm_buf *buf); +void tpm_buf_append(struct tpm_buf *buf, const unsigned char *new_data, + unsigned int new_len); +void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value); +void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value); +void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value); /* * Check if TPM device is in the firmware upgrade mode.
separate out the tpm_buf_... handling functions from static inlines in tpm.h and move them to their own tpm-buf.c file. This is a precursor to adding new functions for other TPM type handling becuase the amount of code will grow from the current 70 lines in tpm.h to about 200 lines when the additions are done. 200 lines of inline functions is a bit too much to keep in a header file. Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> --- drivers/char/tpm/Makefile | 1 + drivers/char/tpm/tpm-buf.c | 96 ++++++++++++++++++++++++++++++++++++++ include/linux/tpm.h | 86 ++++------------------------------ 3 files changed, 106 insertions(+), 77 deletions(-) create mode 100644 drivers/char/tpm/tpm-buf.c