2021-01-16 00:15:23

by Wei Liu

[permalink] [raw]
Subject: [PATCH] fTPM: make sure TEE is initialized before fTPM

For built-in drivers, the order of initialization function invocation is
determined by their link order.

The original code linked TPM drivers before TEE driver when they were
both built in. That caused fTPM's initialization to be deferred to a
worker thread instead of running on PID 1.

That is problematic because IMA's initialization routine, which runs on
PID 1 as a late initcall, needs to have access to the default TPM
instance. If fTPM's initialization is deferred, IMA will not be able to
get hold of a TPM instance in time.

Fix this by modifying Makefile to make sure TEE is initialized before
fTPM when they are both built in.

Signed-off-by: Wei Liu <[email protected]>
---
drivers/Makefile | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/Makefile b/drivers/Makefile
index fd11b9ac4cc3..45ea5ec9d0fd 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -180,6 +180,11 @@ obj-$(CONFIG_NVMEM) += nvmem/
obj-$(CONFIG_FPGA) += fpga/
obj-$(CONFIG_FSI) += fsi/
obj-$(CONFIG_TEE) += tee/
+
+# TPM drivers must come after TEE, otherwise fTPM initialization will be
+# deferred, which causes IMA to not get a TPM device in time
+obj-$(CONFIG_TCG_TPM) += char/tpm/
+
obj-$(CONFIG_MULTIPLEXER) += mux/
obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/
obj-$(CONFIG_SIOX) += siox/
--
2.20.1


2021-01-16 00:55:36

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] fTPM: make sure TEE is initialized before fTPM

Hi,

On 1/15/21 4:12 PM, Wei Liu wrote:
> For built-in drivers, the order of initialization function invocation is
> determined by their link order.
>
> The original code linked TPM drivers before TEE driver when they were
> both built in. That caused fTPM's initialization to be deferred to a
> worker thread instead of running on PID 1.
>
> That is problematic because IMA's initialization routine, which runs on
> PID 1 as a late initcall, needs to have access to the default TPM
> instance. If fTPM's initialization is deferred, IMA will not be able to
> get hold of a TPM instance in time.
>
> Fix this by modifying Makefile to make sure TEE is initialized before
> fTPM when they are both built in.
>
> Signed-off-by: Wei Liu <[email protected]>
> ---
> drivers/Makefile | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/Makefile b/drivers/Makefile
> index fd11b9ac4cc3..45ea5ec9d0fd 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -180,6 +180,11 @@ obj-$(CONFIG_NVMEM) += nvmem/
> obj-$(CONFIG_FPGA) += fpga/
> obj-$(CONFIG_FSI) += fsi/
> obj-$(CONFIG_TEE) += tee/
> +
> +# TPM drivers must come after TEE, otherwise fTPM initialization will be
> +# deferred, which causes IMA to not get a TPM device in time
> +obj-$(CONFIG_TCG_TPM) += char/tpm/
> +
> obj-$(CONFIG_MULTIPLEXER) += mux/
> obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/
> obj-$(CONFIG_SIOX) += siox/
>

As I suspected and then tested, since you did not remove the other build
of char/tpm/, this ends up with multiple definition linker errors (below).

I would think that instead of depending on Makefile order you should use different
initcall levels as needed. Depending on Makefile order is what we did 15 years ago.


ld: drivers/char/tpm/tpm-chip.o:(.data+0xa0): multiple definition of `dev_nums_idr'; drivers/char/tpm/tpm-chip.o:(.data+0xa0): first defined here
ld: drivers/char/tpm/tpm-chip.o: in function `tpm_chip_start':
tpm-chip.c:(.text+0x119): multiple definition of `tpm_chip_start'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x119): first defined here
ld: drivers/char/tpm/tpm-chip.o: in function `tpm_chip_stop':
tpm-chip.c:(.text+0x19d): multiple definition of `tpm_chip_stop'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x19d): first defined here
ld: drivers/char/tpm/tpm-chip.o: in function `tpm_try_get_ops':
tpm-chip.c:(.text+0x217): multiple definition of `tpm_try_get_ops'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x217): first defined here
ld: drivers/char/tpm/tpm-chip.o: in function `tpm_put_ops':
tpm-chip.c:(.text+0x285): multiple definition of `tpm_put_ops'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x285): first defined here
ld: drivers/char/tpm/tpm-chip.o: in function `tpm_default_chip':
tpm-chip.c:(.text+0x2af): multiple definition of `tpm_default_chip'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x2af): first defined here
ld: drivers/char/tpm/tpm-chip.o: in function `tpm_find_get_ops':
tpm-chip.c:(.text+0x308): multiple definition of `tpm_find_get_ops'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x308): first defined here
ld: drivers/char/tpm/tpm-chip.o: in function `tpm_chip_alloc':
tpm-chip.c:(.text+0x348): multiple definition of `tpm_chip_alloc'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x348): first defined here
ld: drivers/char/tpm/tpm-chip.o:(.bss+0x10): multiple definition of `tpm_class'; drivers/char/tpm/tpm-chip.o:(.bss+0x10): first defined here
ld: drivers/char/tpm/tpm-chip.o:(.bss+0x8): multiple definition of `tpmrm_class'; drivers/char/tpm/tpm-chip.o:(.bss+0x8): first defined here
ld: drivers/char/tpm/tpm-chip.o:(.bss+0x0): multiple definition of `tpm_devt'; drivers/char/tpm/tpm-chip.o:(.bss+0x0): first defined here
ld: drivers/char/tpm/tpm-chip.o: in function `tpmm_chip_alloc':
tpm-chip.c:(.text+0x582): multiple definition of `tpmm_chip_alloc'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x582): first defined here
ld: drivers/char/tpm/tpm-chip.o: in function `tpm_chip_unregister':
tpm-chip.c:(.text+0x5cb): multiple definition of `tpm_chip_unregister'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x5cb): first defined here
ld: drivers/char/tpm/tpm-chip.o: in function `tpm_chip_register':
tpm-chip.c:(.text+0x697): multiple definition of `tpm_chip_register'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x697): first defined here
ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_common_open':
tpm-dev-common.c:(.text+0x1be): multiple definition of `tpm_common_open'; drivers/char/tpm/tpm-dev-common.o:tpm-dev-common.c:(.text+0x1be): first defined here
ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_common_read':
tpm-dev-common.c:(.text+0x2d3): multiple definition of `tpm_common_read'; drivers/char/tpm/tpm-dev-common.o:tpm-dev-common.c:(.text+0x2d3): first defined here
ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_common_write':
tpm-dev-common.c:(.text+0x3e5): multiple definition of `tpm_common_write'; drivers/char/tpm/tpm-dev-common.o:tpm-dev-common.c:(.text+0x3e5): first defined here
ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_common_poll':
tpm-dev-common.c:(.text+0x56b): multiple definition of `tpm_common_poll'; drivers/char/tpm/tpm-dev-common.o:tpm-dev-common.c:(.text+0x56b): first defined here
ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_common_release':
tpm-dev-common.c:(.text+0x5c4): multiple definition of `tpm_common_release'; drivers/char/tpm/tpm-dev-common.o:tpm-dev-common.c:(.text+0x5c4): first defined here
ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_dev_common_init':
tpm-dev-common.c:(.init.text+0x0): multiple definition of `tpm_dev_common_init'; drivers/char/tpm/tpm-dev-common.o:tpm-dev-common.c:(.init.text+0x0): first defined here
ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_dev_common_exit':
tpm-dev-common.c:(.exit.text+0x0): multiple definition of `tpm_dev_common_exit'; drivers/char/tpm/tpm-dev-common.o:tpm-dev-common.c:(.exit.text+0x0): first defined here
ld: drivers/char/tpm/tpm-dev.o:(.rodata+0x0): multiple definition of `tpm_fops'; drivers/char/tpm/tpm-dev.o:(.rodata+0x0): first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_calc_ordinal_duration':
tpm-interface.c:(.text+0x0): multiple definition of `tpm_calc_ordinal_duration'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x0): first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_transmit':
tpm-interface.c:(.text+0x13): multiple definition of `tpm_transmit'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x13): first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_transmit_cmd':
tpm-interface.c:(.text+0x2bc): multiple definition of `tpm_transmit_cmd'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x2bc): first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_get_timeouts':
tpm-interface.c:(.text+0x33a): multiple definition of `tpm_get_timeouts'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x33a): first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_is_tpm2':
tpm-interface.c:(.text+0x355): multiple definition of `tpm_is_tpm2'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x355): first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_pcr_read':
tpm-interface.c:(.text+0x37c): multiple definition of `tpm_pcr_read'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x37c): first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_pcr_extend':
tpm-interface.c:(.text+0x3d8): multiple definition of `tpm_pcr_extend'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x3d8): first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_send':
tpm-interface.c:(.text+0x46c): multiple definition of `tpm_send'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x46c): first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_auto_startup':
tpm-interface.c:(.text+0x4b9): multiple definition of `tpm_auto_startup'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x4b9): first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_pm_suspend':
tpm-interface.c:(.text+0x4db): multiple definition of `tpm_pm_suspend'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x4db): first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_pm_resume':
tpm-interface.c:(.text+0x553): multiple definition of `tpm_pm_resume'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x553): first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_get_random':
tpm-interface.c:(.text+0x55e): multiple definition of `tpm_get_random'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x55e): first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_calc_ordinal_duration':
tpm1-cmd.c:(.text+0x98): multiple definition of `tpm1_calc_ordinal_duration'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x98): first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_pcr_extend':
tpm1-cmd.c:(.text+0xc3): multiple definition of `tpm1_pcr_extend'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0xc3): first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_getcap':
tpm1-cmd.c:(.text+0x13d): multiple definition of `tpm1_getcap'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x13d): first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_get_timeouts':
tpm1-cmd.c:(.text+0x200): multiple definition of `tpm1_get_timeouts'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x200): first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_get_random':
tpm1-cmd.c:(.text+0x58b): multiple definition of `tpm1_get_random'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x58b): first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_pcr_read':
tpm1-cmd.c:(.text+0x695): multiple definition of `tpm1_pcr_read'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x695): first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_do_selftest':
tpm1-cmd.c:(.text+0x71c): multiple definition of `tpm1_do_selftest'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x71c): first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_auto_startup':
tpm1-cmd.c:(.text+0x836): multiple definition of `tpm1_auto_startup'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x836): first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_pm_suspend':
tpm1-cmd.c:(.text+0x873): multiple definition of `tpm1_pm_suspend'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x873): first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_get_pcr_allocation':
tpm1-cmd.c:(.text+0x94b): multiple definition of `tpm1_get_pcr_allocation'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x94b): first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_timeouts':
tpm2-cmd.c:(.text+0x157): multiple definition of `tpm2_get_timeouts'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x157): first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_calc_ordinal_duration':
tpm2-cmd.c:(.text+0x1b9): multiple definition of `tpm2_calc_ordinal_duration'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x1b9): first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_pcr_read':
tpm2-cmd.c:(.text+0x1e5): multiple definition of `tpm2_pcr_read'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x1e5): first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_pcr_extend':
tpm2-cmd.c:(.text+0x352): multiple definition of `tpm2_pcr_extend'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x352): first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_random':
tpm2-cmd.c:(.text+0x442): multiple definition of `tpm2_get_random'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x442): first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_flush_context':
tpm2-cmd.c:(.text+0x577): multiple definition of `tpm2_flush_context'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x577): first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_tpm_pt':
tpm2-cmd.c:(.text+0x5da): multiple definition of `tpm2_get_tpm_pt'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x5da): first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_shutdown':
tpm2-cmd.c:(.text+0x663): multiple definition of `tpm2_shutdown'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x663): first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_probe':
tpm2-cmd.c:(.text+0x6b4): multiple definition of `tpm2_probe'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x6b4): first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_pcr_allocation':
tpm2-cmd.c:(.text+0x735): multiple definition of `tpm2_get_pcr_allocation'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x735): first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_cc_attrs_tbl':
tpm2-cmd.c:(.text+0x92a): multiple definition of `tpm2_get_cc_attrs_tbl'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x92a): first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_auto_startup':
tpm2-cmd.c:(.text+0xa8e): multiple definition of `tpm2_auto_startup'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0xa8e): first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_find_cc':
tpm2-cmd.c:(.text+0xb3f): multiple definition of `tpm2_find_cc'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0xb3f): first defined here
ld: drivers/char/tpm/tpmrm-dev.o:(.rodata+0x0): multiple definition of `tpmrm_fops'; drivers/char/tpm/tpmrm-dev.o:(.rodata+0x0): first defined here
ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_init_space':
tpm2-space.c:(.text+0x2b1): multiple definition of `tpm2_init_space'; drivers/char/tpm/tpm2-space.o:tpm2-space.c:(.text+0x2b1): first defined here
ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_del_space':
tpm2-space.c:(.text+0x307): multiple definition of `tpm2_del_space'; drivers/char/tpm/tpm2-space.o:tpm2-space.c:(.text+0x307): first defined here
ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_flush_space':
tpm2-space.c:(.text+0x377): multiple definition of `tpm2_flush_space'; drivers/char/tpm/tpm2-space.o:tpm2-space.c:(.text+0x377): first defined here
ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_prepare_space':
tpm2-space.c:(.text+0x3c1): multiple definition of `tpm2_prepare_space'; drivers/char/tpm/tpm2-space.o:tpm2-space.c:(.text+0x3c1): first defined here
ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_commit_space':
tpm2-space.c:(.text+0x600): multiple definition of `tpm2_commit_space'; drivers/char/tpm/tpm2-space.o:tpm2-space.c:(.text+0x600): first defined here
ld: drivers/char/tpm/tpm-sysfs.o: in function `tpm_sysfs_add_device':
tpm-sysfs.c:(.text+0x649): multiple definition of `tpm_sysfs_add_device'; drivers/char/tpm/tpm-sysfs.o:tpm-sysfs.c:(.text+0x649): first defined here
ld: drivers/char/tpm/eventlog/common.o: in function `tpm_bios_log_teardown':
common.c:(.text+0x9c): multiple definition of `tpm_bios_log_teardown'; drivers/char/tpm/eventlog/common.o:common.c:(.text+0x9c): first defined here
ld: drivers/char/tpm/eventlog/common.o: in function `tpm_bios_log_setup':
common.c:(.text+0xf8): multiple definition of `tpm_bios_log_setup'; drivers/char/tpm/eventlog/common.o:common.c:(.text+0xf8): first defined here
ld: drivers/char/tpm/eventlog/tpm1.o:(.rodata+0x40): multiple definition of `tpm1_binary_b_measurements_seqops'; drivers/char/tpm/eventlog/tpm1.o:(.rodata+0x40): first defined here
ld: drivers/char/tpm/eventlog/tpm1.o:(.rodata+0x60): multiple definition of `tpm1_ascii_b_measurements_seqops'; drivers/char/tpm/eventlog/tpm1.o:(.rodata+0x60): first defined here
ld: drivers/char/tpm/eventlog/tpm2.o:(.rodata+0x0): multiple definition of `tpm2_binary_b_measurements_seqops'; drivers/char/tpm/eventlog/tpm2.o:(.rodata+0x0): first defined here
ld: drivers/char/tpm/tpm_ppi.o: in function `tpm_add_ppi':
tpm_ppi.c:(.text+0x5c6): multiple definition of `tpm_add_ppi'; drivers/char/tpm/tpm_ppi.o:tpm_ppi.c:(.text+0x5c6): first defined here
ld: drivers/char/tpm/eventlog/acpi.o: in function `tpm_read_log_acpi':
acpi.c:(.text+0x0): multiple definition of `tpm_read_log_acpi'; drivers/char/tpm/eventlog/acpi.o:acpi.c:(.text+0x0): first defined here
ld: drivers/char/tpm/st33zp24/st33zp24.o: in function `st33zp24_probe':
st33zp24.c:(.text+0x6fe): multiple definition of `st33zp24_probe'; drivers/char/tpm/st33zp24/st33zp24.o:st33zp24.c:(.text+0x6fe): first defined here
ld: drivers/char/tpm/st33zp24/st33zp24.o: in function `st33zp24_remove':
st33zp24.c:(.text+0x8ab): multiple definition of `st33zp24_remove'; drivers/char/tpm/st33zp24/st33zp24.o:st33zp24.c:(.text+0x8ab): first defined here
ld: drivers/char/tpm/st33zp24/st33zp24.o: in function `st33zp24_pm_suspend':
st33zp24.c:(.text+0x8b3): multiple definition of `st33zp24_pm_suspend'; drivers/char/tpm/st33zp24/st33zp24.o:st33zp24.c:(.text+0x8b3): first defined here
ld: drivers/char/tpm/st33zp24/st33zp24.o: in function `st33zp24_pm_resume':
st33zp24.c:(.text+0x8de): multiple definition of `st33zp24_pm_resume'; drivers/char/tpm/st33zp24/st33zp24.o:st33zp24.c:(.text+0x8de): first defined here


--
~Randy
You can't do anything without having to do something else first.
-- Belefant's Law

2021-01-16 05:18:10

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] fTPM: make sure TEE is initialized before fTPM

Hi Wei,

I love your patch! Yet something to improve:

[auto build test ERROR on linux/master]
[also build test ERROR on soc/for-next linus/master v5.11-rc3 next-20210115]
[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]

url: https://github.com/0day-ci/linux/commits/Wei-Liu/fTPM-make-sure-TEE-is-initialized-before-fTPM/20210116-081538
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 5ee88057889bbca5f5bb96031b62b3756b33e164
config: mips-randconfig-p002-20210115 (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.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/0day-ci/linux/commit/b82b3e643532e299d9c6e622604e6d8d090153f0
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Wei-Liu/fTPM-make-sure-TEE-is-initialized-before-fTPM/20210116-081538
git checkout b82b3e643532e299d9c6e622604e6d8d090153f0
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

mips-linux-ld: drivers/char/tpm/tpm-chip.o: in function `tpm_chip_start':
>> tpm-chip.c:(.text+0x0): multiple definition of `tpm_chip_start'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x0): first defined here
mips-linux-ld: drivers/char/tpm/tpm-chip.o: in function `tpm_chip_stop':
>> tpm-chip.c:(.text+0xe8): multiple definition of `tpm_chip_stop'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0xe8): first defined here
mips-linux-ld: drivers/char/tpm/tpm-chip.o: in function `tpm_try_get_ops':
>> tpm-chip.c:(.text+0x170): multiple definition of `tpm_try_get_ops'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x170): first defined here
mips-linux-ld: drivers/char/tpm/tpm-chip.o: in function `tpm_put_ops':
>> tpm-chip.c:(.text+0x218): multiple definition of `tpm_put_ops'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x218): first defined here
mips-linux-ld: drivers/char/tpm/tpm-chip.o: in function `tpm_default_chip':
>> tpm-chip.c:(.text+0x278): multiple definition of `tpm_default_chip'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x278): first defined here
>> mips-linux-ld: drivers/char/tpm/tpm-chip.o:(.data+0x48): multiple definition of `dev_nums_idr'; drivers/char/tpm/tpm-chip.o:(.data+0x48): first defined here
mips-linux-ld: drivers/char/tpm/tpm-chip.o: in function `tpm_chip_unregister':
>> tpm-chip.c:(.text+0x4a4): multiple definition of `tpm_chip_unregister'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x4a4): first defined here
mips-linux-ld: drivers/char/tpm/tpm-chip.o: in function `tpm_chip_register':
>> tpm-chip.c:(.text+0x584): multiple definition of `tpm_chip_register'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x584): first defined here
mips-linux-ld: drivers/char/tpm/tpm-chip.o: in function `tpm_chip_alloc':
>> tpm-chip.c:(.text+0x75c): multiple definition of `tpm_chip_alloc'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x75c): first defined here
>> mips-linux-ld: drivers/char/tpm/tpm-chip.o:(.bss+0x8): multiple definition of `tpm_class'; drivers/char/tpm/tpm-chip.o:(.bss+0x8): first defined here
>> mips-linux-ld: drivers/char/tpm/tpm-chip.o:(.bss+0x4): multiple definition of `tpmrm_class'; drivers/char/tpm/tpm-chip.o:(.bss+0x4): first defined here
>> mips-linux-ld: drivers/char/tpm/tpm-chip.o:(.bss+0x0): multiple definition of `tpm_devt'; drivers/char/tpm/tpm-chip.o:(.bss+0x0): first defined here
mips-linux-ld: drivers/char/tpm/tpm-chip.o: in function `tpmm_chip_alloc':
>> tpm-chip.c:(.text+0x9bc): multiple definition of `tpmm_chip_alloc'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0x9bc): first defined here
mips-linux-ld: drivers/char/tpm/tpm-chip.o: in function `tpm_find_get_ops':
>> tpm-chip.c:(.text+0xa58): multiple definition of `tpm_find_get_ops'; drivers/char/tpm/tpm-chip.o:tpm-chip.c:(.text+0xa58): first defined here
mips-linux-ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_common_open':
>> tpm-dev-common.c:(.text+0x22c): multiple definition of `tpm_common_open'; drivers/char/tpm/tpm-dev-common.o:tpm-dev-common.c:(.text+0x22c): first defined here
mips-linux-ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_common_read':
>> tpm-dev-common.c:(.text+0x378): multiple definition of `tpm_common_read'; drivers/char/tpm/tpm-dev-common.o:tpm-dev-common.c:(.text+0x378): first defined here
mips-linux-ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_common_write':
>> tpm-dev-common.c:(.text+0x574): multiple definition of `tpm_common_write'; drivers/char/tpm/tpm-dev-common.o:tpm-dev-common.c:(.text+0x574): first defined here
mips-linux-ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_common_poll':
>> tpm-dev-common.c:(.text+0x864): multiple definition of `tpm_common_poll'; drivers/char/tpm/tpm-dev-common.o:tpm-dev-common.c:(.text+0x864): first defined here
mips-linux-ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_common_release':
>> tpm-dev-common.c:(.text+0x8e8): multiple definition of `tpm_common_release'; drivers/char/tpm/tpm-dev-common.o:tpm-dev-common.c:(.text+0x8e8): first defined here
mips-linux-ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_dev_common_init':
>> tpm-dev-common.c:(.init.text+0x0): multiple definition of `tpm_dev_common_init'; drivers/char/tpm/tpm-dev-common.o:tpm-dev-common.c:(.init.text+0x0): first defined here
mips-linux-ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_dev_common_exit':
tpm-dev-common.c:(.exit.text+0x0): multiple definition of `tpm_dev_common_exit'; drivers/char/tpm/tpm-dev-common.o:tpm-dev-common.c:(.exit.text+0x0): first defined here
mips-linux-ld: drivers/char/tpm/tpm-dev.o:(.rodata+0x0): multiple definition of `tpm_fops'; drivers/char/tpm/tpm-dev.o:(.rodata+0x0): first defined here
mips-linux-ld: drivers/char/tpm/tpm-interface.o: in function `tpm_pm_resume':
tpm-interface.c:(.text+0x0): multiple definition of `tpm_pm_resume'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x0): first defined here
mips-linux-ld: drivers/char/tpm/tpm-interface.o: in function `tpm_calc_ordinal_duration':
tpm-interface.c:(.text+0x20): multiple definition of `tpm_calc_ordinal_duration'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x20): first defined here
mips-linux-ld: drivers/char/tpm/tpm-interface.o: in function `tpm_get_timeouts':
tpm-interface.c:(.text+0x50): multiple definition of `tpm_get_timeouts'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x50): first defined here
mips-linux-ld: drivers/char/tpm/tpm-interface.o: in function `tpm_is_tpm2':
tpm-interface.c:(.text+0x94): multiple definition of `tpm_is_tpm2'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x94): first defined here
mips-linux-ld: drivers/char/tpm/tpm-interface.o: in function `tpm_pcr_read':
tpm-interface.c:(.text+0xe4): multiple definition of `tpm_pcr_read'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0xe4): first defined here
mips-linux-ld: drivers/char/tpm/tpm-interface.o: in function `tpm_pcr_extend':
tpm-interface.c:(.text+0x184): multiple definition of `tpm_pcr_extend'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x184): first defined here
mips-linux-ld: drivers/char/tpm/tpm-interface.o: in function `tpm_pm_suspend':
tpm-interface.c:(.text+0x268): multiple definition of `tpm_pm_suspend'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x268): first defined here
mips-linux-ld: drivers/char/tpm/tpm-interface.o: in function `tpm_get_random':
tpm-interface.c:(.text+0x308): multiple definition of `tpm_get_random'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x308): first defined here
mips-linux-ld: drivers/char/tpm/tpm-interface.o: in function `tpm_transmit':
tpm-interface.c:(.text+0x3bc): multiple definition of `tpm_transmit'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x3bc): first defined here
mips-linux-ld: drivers/char/tpm/tpm-interface.o: in function `tpm_transmit_cmd':
tpm-interface.c:(.text+0x628): multiple definition of `tpm_transmit_cmd'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x628): first defined here
mips-linux-ld: drivers/char/tpm/tpm-interface.o: in function `tpm_send':
tpm-interface.c:(.text+0x698): multiple definition of `tpm_send'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x698): first defined here
mips-linux-ld: drivers/char/tpm/tpm-interface.o: in function `tpm_auto_startup':
tpm-interface.c:(.text+0x730): multiple definition of `tpm_auto_startup'; drivers/char/tpm/tpm-interface.o:tpm-interface.c:(.text+0x730): first defined here
mips-linux-ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_getcap':
tpm1-cmd.c:(.text+0x0): multiple definition of `tpm1_getcap'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x0): first defined here
mips-linux-ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_calc_ordinal_duration':
tpm1-cmd.c:(.text+0x2dc): multiple definition of `tpm1_calc_ordinal_duration'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x2dc): first defined here
mips-linux-ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_get_timeouts':
tpm1-cmd.c:(.text+0x36c): multiple definition of `tpm1_get_timeouts'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x36c): first defined here
mips-linux-ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_pcr_extend':
tpm1-cmd.c:(.text+0x7ac): multiple definition of `tpm1_pcr_extend'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x7ac): first defined here
mips-linux-ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_get_random':
tpm1-cmd.c:(.text+0x920): multiple definition of `tpm1_get_random'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x920): first defined here
mips-linux-ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_pcr_read':
tpm1-cmd.c:(.text+0xb50): multiple definition of `tpm1_pcr_read'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0xb50): first defined here
mips-linux-ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_do_selftest':
tpm1-cmd.c:(.text+0xcac): multiple definition of `tpm1_do_selftest'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0xcac): first defined here
mips-linux-ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_auto_startup':
tpm1-cmd.c:(.text+0xe54): multiple definition of `tpm1_auto_startup'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0xe54): first defined here
mips-linux-ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_pm_suspend':
tpm1-cmd.c:(.text+0xea8): multiple definition of `tpm1_pm_suspend'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0xea8): first defined here
mips-linux-ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_get_pcr_allocation':
tpm1-cmd.c:(.text+0x1018): multiple definition of `tpm1_get_pcr_allocation'; drivers/char/tpm/tpm1-cmd.o:tpm1-cmd.c:(.text+0x1018): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_flush_context':
tpm2-cmd.c:(.text+0x170): multiple definition of `tpm2_flush_context'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x170): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_tpm_pt':
tpm2-cmd.c:(.text+0x27c): multiple definition of `tpm2_get_tpm_pt'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x27c): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_probe':
tpm2-cmd.c:(.text+0x460): multiple definition of `tpm2_probe'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x460): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_cc_attrs_tbl':
tpm2-cmd.c:(.text+0x644): multiple definition of `tpm2_get_cc_attrs_tbl'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x644): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_timeouts':
tpm2-cmd.c:(.text+0x968): multiple definition of `tpm2_get_timeouts'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x968): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_calc_ordinal_duration':
tpm2-cmd.c:(.text+0x9c8): multiple definition of `tpm2_calc_ordinal_duration'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x9c8): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_pcr_read':
tpm2-cmd.c:(.text+0xa5c): multiple definition of `tpm2_pcr_read'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0xa5c): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_pcr_extend':
tpm2-cmd.c:(.text+0xd80): multiple definition of `tpm2_pcr_extend'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0xd80): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_random':
tpm2-cmd.c:(.text+0x1110): multiple definition of `tpm2_get_random'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x1110): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_shutdown':
tpm2-cmd.c:(.text+0x1350): multiple definition of `tpm2_shutdown'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x1350): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_pcr_allocation':
tpm2-cmd.c:(.text+0x145c): multiple definition of `tpm2_get_pcr_allocation'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x145c): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_auto_startup':
tpm2-cmd.c:(.text+0x1890): multiple definition of `tpm2_auto_startup'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x1890): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_find_cc':
tpm2-cmd.c:(.text+0x1a34): multiple definition of `tpm2_find_cc'; drivers/char/tpm/tpm2-cmd.o:tpm2-cmd.c:(.text+0x1a34): first defined here
mips-linux-ld: drivers/char/tpm/tpmrm-dev.o:(.rodata+0x0): multiple definition of `tpmrm_fops'; drivers/char/tpm/tpmrm-dev.o:(.rodata+0x0): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_init_space':
tpm2-space.c:(.text+0x3c8): multiple definition of `tpm2_init_space'; drivers/char/tpm/tpm2-space.o:tpm2-space.c:(.text+0x3c8): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_del_space':
tpm2-space.c:(.text+0x464): multiple definition of `tpm2_del_space'; drivers/char/tpm/tpm2-space.o:tpm2-space.c:(.text+0x464): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_flush_space':
tpm2-space.c:(.text+0x57c): multiple definition of `tpm2_flush_space'; drivers/char/tpm/tpm2-space.o:tpm2-space.c:(.text+0x57c): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_prepare_space':
tpm2-space.c:(.text+0x6bc): multiple definition of `tpm2_prepare_space'; drivers/char/tpm/tpm2-space.o:tpm2-space.c:(.text+0x6bc): first defined here
mips-linux-ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_commit_space':
tpm2-space.c:(.text+0xa7c): multiple definition of `tpm2_commit_space'; drivers/char/tpm/tpm2-space.o:tpm2-space.c:(.text+0xa7c): first defined here
mips-linux-ld: drivers/char/tpm/tpm-sysfs.o: in function `tpm_sysfs_add_device':
tpm-sysfs.c:(.text+0xa20): multiple definition of `tpm_sysfs_add_device'; drivers/char/tpm/tpm-sysfs.o:tpm-sysfs.c:(.text+0xa20): first defined here
mips-linux-ld: drivers/char/tpm/eventlog/common.o: in function `tpm_bios_log_teardown':
common.c:(.text+0xf4): multiple definition of `tpm_bios_log_teardown'; drivers/char/tpm/eventlog/common.o:common.c:(.text+0xf4): first defined here
mips-linux-ld: drivers/char/tpm/eventlog/common.o: in function `tpm_bios_log_setup':
common.c:(.text+0x1e0): multiple definition of `tpm_bios_log_setup'; drivers/char/tpm/eventlog/common.o:common.c:(.text+0x1e0): first defined here
mips-linux-ld: drivers/char/tpm/eventlog/tpm1.o:(.rodata+0x38): multiple definition of `tpm1_binary_b_measurements_seqops'; drivers/char/tpm/eventlog/tpm1.o:(.rodata+0x38): first defined here
mips-linux-ld: drivers/char/tpm/eventlog/tpm1.o:(.rodata+0x48): multiple definition of `tpm1_ascii_b_measurements_seqops'; drivers/char/tpm/eventlog/tpm1.o:(.rodata+0x48): first defined here
mips-linux-ld: drivers/char/tpm/eventlog/tpm2.o:(.rodata+0x18): multiple definition of `tpm2_binary_b_measurements_seqops'; drivers/char/tpm/eventlog/tpm2.o:(.rodata+0x18): first defined here
mips-linux-ld: drivers/char/tpm/eventlog/of.o: in function `tpm_read_log_of':
of.c:(.text+0x0): multiple definition of `tpm_read_log_of'; drivers/char/tpm/eventlog/of.o:of.c:(.text+0x0): first defined here
mips-linux-ld: drivers/char/tpm/tpm_tis_core.o: in function `tpm_tis_remove':
tpm_tis_core.c:(.text+0x1dc): multiple definition of `tpm_tis_remove'; drivers/char/tpm/tpm_tis_core.o:tpm_tis_core.c:(.text+0x1dc): first defined here
mips-linux-ld: drivers/char/tpm/tpm_tis_core.o: in function `tpm_tis_core_init':
tpm_tis_core.c:(.text+0x137c): multiple definition of `tpm_tis_core_init'; drivers/char/tpm/tpm_tis_core.o:tpm_tis_core.c:(.text+0x137c): first defined here
mips-linux-ld: drivers/char/tpm/st33zp24/st33zp24.o: in function `st33zp24_probe':

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (17.56 kB)
.config.gz (43.79 kB)
Download all attachments

2021-01-16 06:12:10

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] fTPM: make sure TEE is initialized before fTPM

Hi Wei,

I love your patch! Yet something to improve:

[auto build test ERROR on linux/master]
[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]

url: https://github.com/0day-ci/linux/commits/Wei-Liu/fTPM-make-sure-TEE-is-initialized-before-fTPM/20210116-081538
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 5ee88057889bbca5f5bb96031b62b3756b33e164
config: x86_64-rhel (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/b82b3e643532e299d9c6e622604e6d8d090153f0
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Wei-Liu/fTPM-make-sure-TEE-is-initialized-before-fTPM/20210116-081538
git checkout b82b3e643532e299d9c6e622604e6d8d090153f0
# save the attached .config to linux build tree
make W=1 ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_dev_common_init':
drivers/char/tpm/tpm-dev-common.c:267: multiple definition of `tpm_dev_common_init'; drivers/char/tpm/tpm-dev-common.o:drivers/char/tpm/tpm-dev-common.c:267: first defined here
ld: drivers/char/tpm/tpm-dev-common.o: in function `tpm_dev_common_exit':
drivers/char/tpm/tpm-dev-common.c:275: multiple definition of `tpm_dev_common_exit'; drivers/char/tpm/tpm-dev-common.o:drivers/char/tpm/tpm-dev-common.c:275: first defined here
ld: drivers/char/tpm/tpm-dev.o:drivers/char/tpm/tpm-dev.c:60: multiple definition of `tpm_fops'; drivers/char/tpm/tpm-dev.o:drivers/char/tpm/tpm-dev.c:60: first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_calc_ordinal_duration':
drivers/char/tpm/tpm-interface.c:53: multiple definition of `tpm_calc_ordinal_duration'; drivers/char/tpm/tpm-interface.o:drivers/char/tpm/tpm-interface.c:53: first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_transmit_cmd':
drivers/char/tpm/tpm-interface.c:215: multiple definition of `tpm_transmit_cmd'; drivers/char/tpm/tpm-interface.o:drivers/char/tpm/tpm-interface.c:215: first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_get_timeouts':
drivers/char/tpm/tpm-interface.c:240: multiple definition of `tpm_get_timeouts'; drivers/char/tpm/tpm-interface.o:drivers/char/tpm/tpm-interface.c:240: first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_is_tpm2':
drivers/char/tpm/tpm-interface.c:261: multiple definition of `tpm_is_tpm2'; drivers/char/tpm/tpm-interface.o:drivers/char/tpm/tpm-interface.c:261: first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_pcr_read':
drivers/char/tpm/tpm-interface.c:286: multiple definition of `tpm_pcr_read'; drivers/char/tpm/tpm-interface.o:drivers/char/tpm/tpm-interface.c:286: first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_pcr_extend':
drivers/char/tpm/tpm-interface.c:316: multiple definition of `tpm_pcr_extend'; drivers/char/tpm/tpm-interface.o:drivers/char/tpm/tpm-interface.c:316: first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_send':
drivers/char/tpm/tpm-interface.c:354: multiple definition of `tpm_send'; drivers/char/tpm/tpm-interface.o:drivers/char/tpm/tpm-interface.c:354: first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_pm_suspend':
drivers/char/tpm/tpm-interface.c:390: multiple definition of `tpm_pm_suspend'; drivers/char/tpm/tpm-interface.o:drivers/char/tpm/tpm-interface.c:390: first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_pm_resume':
drivers/char/tpm/tpm-interface.c:423: multiple definition of `tpm_pm_resume'; drivers/char/tpm/tpm-interface.o:drivers/char/tpm/tpm-interface.c:423: first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_get_random':
drivers/char/tpm/tpm-interface.c:442: multiple definition of `tpm_get_random'; drivers/char/tpm/tpm-interface.o:drivers/char/tpm/tpm-interface.c:442: first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_transmit':
drivers/char/tpm/tpm-interface.c:154: multiple definition of `tpm_transmit'; drivers/char/tpm/tpm-interface.o:drivers/char/tpm/tpm-interface.c:154: first defined here
ld: drivers/char/tpm/tpm-interface.o: in function `tpm_auto_startup':
drivers/char/tpm/tpm-interface.c:371: multiple definition of `tpm_auto_startup'; drivers/char/tpm/tpm-interface.o:drivers/char/tpm/tpm-interface.c:371: first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_getcap':
drivers/char/tpm/tpm1-cmd.c:484: multiple definition of `tpm1_getcap'; drivers/char/tpm/tpm1-cmd.o:drivers/char/tpm/tpm1-cmd.c:484: first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_do_selftest':
drivers/char/tpm/tpm1-cmd.c:643: multiple definition of `tpm1_do_selftest'; drivers/char/tpm/tpm1-cmd.o:drivers/char/tpm/tpm1-cmd.c:643: first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_calc_ordinal_duration':
drivers/char/tpm/tpm1-cmd.c:292: multiple definition of `tpm1_calc_ordinal_duration'; drivers/char/tpm/tpm1-cmd.o:drivers/char/tpm/tpm1-cmd.c:292: first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_get_timeouts':
drivers/char/tpm/tpm1-cmd.c:343: multiple definition of `tpm1_get_timeouts'; drivers/char/tpm/tpm1-cmd.o:drivers/char/tpm/tpm1-cmd.c:343: first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_pcr_extend':
drivers/char/tpm/tpm1-cmd.c:465: multiple definition of `tpm1_pcr_extend'; drivers/char/tpm/tpm1-cmd.o:drivers/char/tpm/tpm1-cmd.c:465: first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_get_random':
drivers/char/tpm/tpm1-cmd.c:531: multiple definition of `tpm1_get_random'; drivers/char/tpm/tpm1-cmd.o:drivers/char/tpm/tpm1-cmd.c:531: first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_pcr_read':
drivers/char/tpm/tpm1-cmd.c:585: multiple definition of `tpm1_pcr_read'; drivers/char/tpm/tpm1-cmd.o:drivers/char/tpm/tpm1-cmd.c:585: first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_auto_startup':
drivers/char/tpm/tpm1-cmd.c:705: multiple definition of `tpm1_auto_startup'; drivers/char/tpm/tpm1-cmd.o:drivers/char/tpm/tpm1-cmd.c:705: first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_pm_suspend':
drivers/char/tpm/tpm1-cmd.c:738: multiple definition of `tpm1_pm_suspend'; drivers/char/tpm/tpm1-cmd.o:drivers/char/tpm/tpm1-cmd.c:738: first defined here
ld: drivers/char/tpm/tpm1-cmd.o: in function `tpm1_get_pcr_allocation':
drivers/char/tpm/tpm1-cmd.c:796: multiple definition of `tpm1_get_pcr_allocation'; drivers/char/tpm/tpm1-cmd.o:drivers/char/tpm/tpm1-cmd.c:796: first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_flush_context':
drivers/char/tpm/tpm2-cmd.c:349: multiple definition of `tpm2_flush_context'; drivers/char/tpm/tpm2-cmd.o:drivers/char/tpm/tpm2-cmd.c:349: first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_tpm_pt':
drivers/char/tpm/tpm2-cmd.c:388: multiple definition of `tpm2_get_tpm_pt'; drivers/char/tpm/tpm2-cmd.o:drivers/char/tpm/tpm2-cmd.c:388: first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_probe':
drivers/char/tpm/tpm2-cmd.c:484: multiple definition of `tpm2_probe'; drivers/char/tpm/tpm2-cmd.o:drivers/char/tpm/tpm2-cmd.c:484: first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_cc_attrs_tbl':
drivers/char/tpm/tpm2-cmd.c:619: multiple definition of `tpm2_get_cc_attrs_tbl'; drivers/char/tpm/tpm2-cmd.o:drivers/char/tpm/tpm2-cmd.c:619: first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_timeouts':
drivers/char/tpm/tpm2-cmd.c:26: multiple definition of `tpm2_get_timeouts'; drivers/char/tpm/tpm2-cmd.o:drivers/char/tpm/tpm2-cmd.c:26: first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_calc_ordinal_duration':
drivers/char/tpm/tpm2-cmd.c:129: multiple definition of `tpm2_calc_ordinal_duration'; drivers/char/tpm/tpm2-cmd.o:drivers/char/tpm/tpm2-cmd.c:129: first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_pcr_read':
drivers/char/tpm/tpm2-cmd.c:163: multiple definition of `tpm2_pcr_read'; drivers/char/tpm/tpm2-cmd.o:drivers/char/tpm/tpm2-cmd.c:163: first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_pcr_extend':
drivers/char/tpm/tpm2-cmd.c:237: multiple definition of `tpm2_pcr_extend'; drivers/char/tpm/tpm2-cmd.o:drivers/char/tpm/tpm2-cmd.c:237: first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_random':
drivers/char/tpm/tpm2-cmd.c:289: multiple definition of `tpm2_get_random'; drivers/char/tpm/tpm2-cmd.o:drivers/char/tpm/tpm2-cmd.c:289: first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_shutdown':
drivers/char/tpm/tpm2-cmd.c:421: multiple definition of `tpm2_shutdown'; drivers/char/tpm/tpm2-cmd.o:drivers/char/tpm/tpm2-cmd.c:421: first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_get_pcr_allocation':
drivers/char/tpm/tpm2-cmd.c:540: multiple definition of `tpm2_get_pcr_allocation'; drivers/char/tpm/tpm2-cmd.o:drivers/char/tpm/tpm2-cmd.c:540: first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_auto_startup':
drivers/char/tpm/tpm2-cmd.c:723: multiple definition of `tpm2_auto_startup'; drivers/char/tpm/tpm2-cmd.o:drivers/char/tpm/tpm2-cmd.c:723: first defined here
ld: drivers/char/tpm/tpm2-cmd.o: in function `tpm2_find_cc':
drivers/char/tpm/tpm2-cmd.c:753: multiple definition of `tpm2_find_cc'; drivers/char/tpm/tpm2-cmd.o:drivers/char/tpm/tpm2-cmd.c:753: first defined here
ld: drivers/char/tpm/tpmrm-dev.o:drivers/char/tpm/tpmrm-dev.c:47: multiple definition of `tpmrm_fops'; drivers/char/tpm/tpmrm-dev.o:drivers/char/tpm/tpmrm-dev.c:47: first defined here
ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_init_space':
drivers/char/tpm/tpm2-space.c:42: multiple definition of `tpm2_init_space'; drivers/char/tpm/tpm2-space.o:drivers/char/tpm/tpm2-space.c:42: first defined here
ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_del_space':
drivers/char/tpm/tpm2-space.c:60: multiple definition of `tpm2_del_space'; drivers/char/tpm/tpm2-space.o:drivers/char/tpm/tpm2-space.c:60: first defined here
ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_flush_space':
drivers/char/tpm/tpm2-space.c:165: multiple definition of `tpm2_flush_space'; drivers/char/tpm/tpm2-space.o:drivers/char/tpm/tpm2-space.c:165: first defined here
ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_prepare_space':
drivers/char/tpm/tpm2-space.c:302: multiple definition of `tpm2_prepare_space'; drivers/char/tpm/tpm2-space.o:drivers/char/tpm/tpm2-space.c:302: first defined here
ld: drivers/char/tpm/tpm2-space.o: in function `tpm2_commit_space':
drivers/char/tpm/tpm2-space.c:533: multiple definition of `tpm2_commit_space'; drivers/char/tpm/tpm2-space.o:drivers/char/tpm/tpm2-space.c:533: first defined here
ld: drivers/char/tpm/tpm-sysfs.o: in function `tpm_sysfs_add_device':
drivers/char/tpm/tpm-sysfs.c:341: multiple definition of `tpm_sysfs_add_device'; drivers/char/tpm/tpm-sysfs.o:drivers/char/tpm/tpm-sysfs.c:341: first defined here
ld: drivers/char/tpm/eventlog/common.o: in function `tpm_bios_log_teardown':
drivers/char/tpm/eventlog/common.c:167: multiple definition of `tpm_bios_log_teardown'; drivers/char/tpm/eventlog/common.o:drivers/char/tpm/eventlog/common.c:167: first defined here
ld: drivers/char/tpm/eventlog/common.o: in function `tpm_bios_log_setup':
drivers/char/tpm/eventlog/common.c:104: multiple definition of `tpm_bios_log_setup'; drivers/char/tpm/eventlog/common.o:drivers/char/tpm/eventlog/common.c:104: first defined here
ld: drivers/char/tpm/eventlog/tpm1.o:drivers/char/tpm/eventlog/tpm1.c:291: multiple definition of `tpm1_binary_b_measurements_seqops'; drivers/char/tpm/eventlog/tpm1.o:drivers/char/tpm/eventlog/tpm1.c:291: first defined here
ld: drivers/char/tpm/eventlog/tpm1.o:drivers/char/tpm/eventlog/tpm1.c:284: multiple definition of `tpm1_ascii_b_measurements_seqops'; drivers/char/tpm/eventlog/tpm1.o:drivers/char/tpm/eventlog/tpm1.c:284: first defined here
ld: drivers/char/tpm/eventlog/tpm2.o:drivers/char/tpm/eventlog/tpm2.c:154: multiple definition of `tpm2_binary_b_measurements_seqops'; drivers/char/tpm/eventlog/tpm2.o:drivers/char/tpm/eventlog/tpm2.c:154: first defined here
ld: drivers/char/tpm/tpm_ppi.o: in function `tpm_add_ppi':
drivers/char/tpm/tpm_ppi.c:367: multiple definition of `tpm_add_ppi'; drivers/char/tpm/tpm_ppi.o:drivers/char/tpm/tpm_ppi.c:367: first defined here
ld: drivers/char/tpm/eventlog/acpi.o: in function `tpm_read_log_acpi':
drivers/char/tpm/eventlog/acpi.c:46: multiple definition of `tpm_read_log_acpi'; drivers/char/tpm/eventlog/acpi.o:drivers/char/tpm/eventlog/acpi.c:46: first defined here
ld: drivers/char/tpm/eventlog/efi.o: in function `tpm_read_log_efi':
>> drivers/char/tpm/eventlog/efi.c:17: multiple definition of `tpm_read_log_efi'; drivers/char/tpm/eventlog/efi.o:drivers/char/tpm/eventlog/efi.c:17: first defined here
ld: drivers/char/tpm/tpm_tis_core.o: in function `tpm_tis_remove':
drivers/char/tpm/tpm_tis_core.c:857: multiple definition of `tpm_tis_remove'; drivers/char/tpm/tpm_tis_core.o:drivers/char/tpm/tpm_tis_core.c:857: first defined here
ld: drivers/char/tpm/tpm_tis_core.o: in function `tpm_tis_core_init':
drivers/char/tpm/tpm_tis_core.c:949: multiple definition of `tpm_tis_core_init'; drivers/char/tpm/tpm_tis_core.o:drivers/char/tpm/tpm_tis_core.c:949: first defined here
ld: drivers/char/tpm/tpm_tis_core.o: in function `tpm_tis_resume':
drivers/char/tpm/tpm_tis_core.c:1143: multiple definition of `tpm_tis_resume'; drivers/char/tpm/tpm_tis_core.o:drivers/char/tpm/tpm_tis_core.c:1143: first defined here


vim +17 drivers/char/tpm/eventlog/efi.c

58cc1e4faf10a73c drivers/char/tpm/tpm_eventlog_efi.c Thiebaud Weksteen 2017-09-20 14
58cc1e4faf10a73c drivers/char/tpm/tpm_eventlog_efi.c Thiebaud Weksteen 2017-09-20 15 /* read binary bios log from EFI configuration table */
58cc1e4faf10a73c drivers/char/tpm/tpm_eventlog_efi.c Thiebaud Weksteen 2017-09-20 16 int tpm_read_log_efi(struct tpm_chip *chip)
58cc1e4faf10a73c drivers/char/tpm/tpm_eventlog_efi.c Thiebaud Weksteen 2017-09-20 @17 {

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (14.69 kB)
.config.gz (44.86 kB)
Download all attachments

2021-01-16 11:57:28

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH] fTPM: make sure TEE is initialized before fTPM

On Fri, Jan 15, 2021 at 04:49:57PM -0800, Randy Dunlap wrote:
> Hi,
>
> On 1/15/21 4:12 PM, Wei Liu wrote:
> > For built-in drivers, the order of initialization function invocation is
> > determined by their link order.
> >
> > The original code linked TPM drivers before TEE driver when they were
> > both built in. That caused fTPM's initialization to be deferred to a
> > worker thread instead of running on PID 1.
> >
> > That is problematic because IMA's initialization routine, which runs on
> > PID 1 as a late initcall, needs to have access to the default TPM
> > instance. If fTPM's initialization is deferred, IMA will not be able to
> > get hold of a TPM instance in time.
> >
> > Fix this by modifying Makefile to make sure TEE is initialized before
> > fTPM when they are both built in.
> >
> > Signed-off-by: Wei Liu <[email protected]>
> > ---
> > drivers/Makefile | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/Makefile b/drivers/Makefile
> > index fd11b9ac4cc3..45ea5ec9d0fd 100644
> > --- a/drivers/Makefile
> > +++ b/drivers/Makefile
> > @@ -180,6 +180,11 @@ obj-$(CONFIG_NVMEM) += nvmem/
> > obj-$(CONFIG_FPGA) += fpga/
> > obj-$(CONFIG_FSI) += fsi/
> > obj-$(CONFIG_TEE) += tee/
> > +
> > +# TPM drivers must come after TEE, otherwise fTPM initialization will be
> > +# deferred, which causes IMA to not get a TPM device in time
> > +obj-$(CONFIG_TCG_TPM) += char/tpm/
> > +
> > obj-$(CONFIG_MULTIPLEXER) += mux/
> > obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/
> > obj-$(CONFIG_SIOX) += siox/
> >
>
> As I suspected and then tested, since you did not remove the other build
> of char/tpm/, this ends up with multiple definition linker errors (below).

Oops, I didn't commit the hunk that removed the line in char/Makefile.

But I will hold off sending out v2 until the following discussion is
settled.

>
> I would think that instead of depending on Makefile order you should use different
> initcall levels as needed. Depending on Makefile order is what we did 15 years ago.
>

No, not really. The same trick was used in 2014 (1bacc894c227).

Both TEE and TPM are just drivers. I think they belong to the same level
(at the moment device_initcall). Looking at the list of levels, I'm not
sure how I can move TEE to a different level.

Out of the seven levels, which one would you suggest I use for which
driver?

Wei.

2021-01-16 12:15:25

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH] fTPM: make sure TEE is initialized before fTPM

On Sat, Jan 16, 2021 at 11:55:29AM +0000, Wei Liu wrote:
> On Fri, Jan 15, 2021 at 04:49:57PM -0800, Randy Dunlap wrote:
> > Hi,
> >
> > On 1/15/21 4:12 PM, Wei Liu wrote:
> > > For built-in drivers, the order of initialization function invocation is
> > > determined by their link order.
> > >
> > > The original code linked TPM drivers before TEE driver when they were
> > > both built in. That caused fTPM's initialization to be deferred to a
> > > worker thread instead of running on PID 1.
> > >
> > > That is problematic because IMA's initialization routine, which runs on
> > > PID 1 as a late initcall, needs to have access to the default TPM
> > > instance. If fTPM's initialization is deferred, IMA will not be able to
> > > get hold of a TPM instance in time.
> > >
> > > Fix this by modifying Makefile to make sure TEE is initialized before
> > > fTPM when they are both built in.
> > >
> > > Signed-off-by: Wei Liu <[email protected]>
> > > ---
> > > drivers/Makefile | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/Makefile b/drivers/Makefile
> > > index fd11b9ac4cc3..45ea5ec9d0fd 100644
> > > --- a/drivers/Makefile
> > > +++ b/drivers/Makefile
> > > @@ -180,6 +180,11 @@ obj-$(CONFIG_NVMEM) += nvmem/
> > > obj-$(CONFIG_FPGA) += fpga/
> > > obj-$(CONFIG_FSI) += fsi/
> > > obj-$(CONFIG_TEE) += tee/
> > > +
> > > +# TPM drivers must come after TEE, otherwise fTPM initialization will be
> > > +# deferred, which causes IMA to not get a TPM device in time
> > > +obj-$(CONFIG_TCG_TPM) += char/tpm/
> > > +
> > > obj-$(CONFIG_MULTIPLEXER) += mux/
> > > obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/
> > > obj-$(CONFIG_SIOX) += siox/
> > >
> >
> > As I suspected and then tested, since you did not remove the other build
> > of char/tpm/, this ends up with multiple definition linker errors (below).
>
> Oops, I didn't commit the hunk that removed the line in char/Makefile.
>
> But I will hold off sending out v2 until the following discussion is
> settled.
>
> >
> > I would think that instead of depending on Makefile order you should use different
> > initcall levels as needed. Depending on Makefile order is what we did 15 years ago.
> >
>
> No, not really. The same trick was used in 2014 (1bacc894c227).
>
> Both TEE and TPM are just drivers. I think they belong to the same level
> (at the moment device_initcall). Looking at the list of levels, I'm not
> sure how I can move TEE to a different level.
>
> Out of the seven levels, which one would you suggest I use for which
> driver?

A bit more random thought.

Moving one driver to a different level is not the solution either. What
if there is a dependency chain in the future in which more than 2
drivers are involved? Do we invent more levels or abuse levels that
aren't supposed to be used by device drivers?

The proper solution to me is to somehow sort the initcalls with their
dependencies in mind. The requires quite a bit of engineering
(integrating depmod into kernel build?). Given that there are only a few
cases, I don't think effort would be worth it.

Wei.

>
> Wei.

2021-01-17 08:37:17

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] fTPM: make sure TEE is initialized before fTPM

On Sat, Jan 16, 2021 at 12:11:09PM +0000, Wei Liu wrote:
> On Sat, Jan 16, 2021 at 11:55:29AM +0000, Wei Liu wrote:
> > On Fri, Jan 15, 2021 at 04:49:57PM -0800, Randy Dunlap wrote:
> > > Hi,
> > >
> > > On 1/15/21 4:12 PM, Wei Liu wrote:
> > > > For built-in drivers, the order of initialization function invocation is
> > > > determined by their link order.
> > > >
> > > > The original code linked TPM drivers before TEE driver when they were
> > > > both built in. That caused fTPM's initialization to be deferred to a
> > > > worker thread instead of running on PID 1.
> > > >
> > > > That is problematic because IMA's initialization routine, which runs on
> > > > PID 1 as a late initcall, needs to have access to the default TPM
> > > > instance. If fTPM's initialization is deferred, IMA will not be able to
> > > > get hold of a TPM instance in time.
> > > >
> > > > Fix this by modifying Makefile to make sure TEE is initialized before
> > > > fTPM when they are both built in.
> > > >
> > > > Signed-off-by: Wei Liu <[email protected]>
> > > > ---
> > > > drivers/Makefile | 5 +++++
> > > > 1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/Makefile b/drivers/Makefile
> > > > index fd11b9ac4cc3..45ea5ec9d0fd 100644
> > > > --- a/drivers/Makefile
> > > > +++ b/drivers/Makefile
> > > > @@ -180,6 +180,11 @@ obj-$(CONFIG_NVMEM) += nvmem/
> > > > obj-$(CONFIG_FPGA) += fpga/
> > > > obj-$(CONFIG_FSI) += fsi/
> > > > obj-$(CONFIG_TEE) += tee/
> > > > +
> > > > +# TPM drivers must come after TEE, otherwise fTPM initialization will be
> > > > +# deferred, which causes IMA to not get a TPM device in time
> > > > +obj-$(CONFIG_TCG_TPM) += char/tpm/
> > > > +
> > > > obj-$(CONFIG_MULTIPLEXER) += mux/
> > > > obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/
> > > > obj-$(CONFIG_SIOX) += siox/
> > > >
> > >
> > > As I suspected and then tested, since you did not remove the other build
> > > of char/tpm/, this ends up with multiple definition linker errors (below).
> >
> > Oops, I didn't commit the hunk that removed the line in char/Makefile.
> >
> > But I will hold off sending out v2 until the following discussion is
> > settled.
> >
> > >
> > > I would think that instead of depending on Makefile order you should use different
> > > initcall levels as needed. Depending on Makefile order is what we did 15 years ago.
> > >
> >
> > No, not really. The same trick was used in 2014 (1bacc894c227).
> >
> > Both TEE and TPM are just drivers. I think they belong to the same level
> > (at the moment device_initcall). Looking at the list of levels, I'm not
> > sure how I can move TEE to a different level.
> >
> > Out of the seven levels, which one would you suggest I use for which
> > driver?
>
> A bit more random thought.
>
> Moving one driver to a different level is not the solution either. What
> if there is a dependency chain in the future in which more than 2
> drivers are involved? Do we invent more levels or abuse levels that
> aren't supposed to be used by device drivers?
>
> The proper solution to me is to somehow sort the initcalls with their
> dependencies in mind. The requires quite a bit of engineering
> (integrating depmod into kernel build?). Given that there are only a few
> cases, I don't think effort would be worth it.

Make it an explicit dependancy in the driver, and then things will be
loaded properly. You can always defer your probe if you do not have all
of the proper resources, which is how these types of things are handled,
instead of worrying about creating new init levels.

thanks,

greg k-h

2021-01-17 14:35:12

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH] fTPM: make sure TEE is initialized before fTPM

On Sun, Jan 17, 2021 at 09:29:42AM +0100, Greg Kroah-Hartman wrote:
> On Sat, Jan 16, 2021 at 12:11:09PM +0000, Wei Liu wrote:
> > On Sat, Jan 16, 2021 at 11:55:29AM +0000, Wei Liu wrote:
> > > On Fri, Jan 15, 2021 at 04:49:57PM -0800, Randy Dunlap wrote:
> > > > Hi,
> > > >
> > > > On 1/15/21 4:12 PM, Wei Liu wrote:
> > > > > For built-in drivers, the order of initialization function invocation is
> > > > > determined by their link order.
> > > > >
> > > > > The original code linked TPM drivers before TEE driver when they were
> > > > > both built in. That caused fTPM's initialization to be deferred to a
> > > > > worker thread instead of running on PID 1.
> > > > >
> > > > > That is problematic because IMA's initialization routine, which runs on
> > > > > PID 1 as a late initcall, needs to have access to the default TPM
> > > > > instance. If fTPM's initialization is deferred, IMA will not be able to
> > > > > get hold of a TPM instance in time.
> > > > >
> > > > > Fix this by modifying Makefile to make sure TEE is initialized before
> > > > > fTPM when they are both built in.
> > > > >
> > > > > Signed-off-by: Wei Liu <[email protected]>
> > > > > ---
> > > > > drivers/Makefile | 5 +++++
> > > > > 1 file changed, 5 insertions(+)
> > > > >
> > > > > diff --git a/drivers/Makefile b/drivers/Makefile
> > > > > index fd11b9ac4cc3..45ea5ec9d0fd 100644
> > > > > --- a/drivers/Makefile
> > > > > +++ b/drivers/Makefile
> > > > > @@ -180,6 +180,11 @@ obj-$(CONFIG_NVMEM) += nvmem/
> > > > > obj-$(CONFIG_FPGA) += fpga/
> > > > > obj-$(CONFIG_FSI) += fsi/
> > > > > obj-$(CONFIG_TEE) += tee/
> > > > > +
> > > > > +# TPM drivers must come after TEE, otherwise fTPM initialization will be
> > > > > +# deferred, which causes IMA to not get a TPM device in time
> > > > > +obj-$(CONFIG_TCG_TPM) += char/tpm/
> > > > > +
> > > > > obj-$(CONFIG_MULTIPLEXER) += mux/
> > > > > obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/
> > > > > obj-$(CONFIG_SIOX) += siox/
> > > > >
> > > >
> > > > As I suspected and then tested, since you did not remove the other build
> > > > of char/tpm/, this ends up with multiple definition linker errors (below).
> > >
> > > Oops, I didn't commit the hunk that removed the line in char/Makefile.
> > >
> > > But I will hold off sending out v2 until the following discussion is
> > > settled.
> > >
> > > >
> > > > I would think that instead of depending on Makefile order you should use different
> > > > initcall levels as needed. Depending on Makefile order is what we did 15 years ago.
> > > >
> > >
> > > No, not really. The same trick was used in 2014 (1bacc894c227).
> > >
> > > Both TEE and TPM are just drivers. I think they belong to the same level
> > > (at the moment device_initcall). Looking at the list of levels, I'm not
> > > sure how I can move TEE to a different level.
> > >
> > > Out of the seven levels, which one would you suggest I use for which
> > > driver?
> >
> > A bit more random thought.
> >
> > Moving one driver to a different level is not the solution either. What
> > if there is a dependency chain in the future in which more than 2
> > drivers are involved? Do we invent more levels or abuse levels that
> > aren't supposed to be used by device drivers?
> >
> > The proper solution to me is to somehow sort the initcalls with their
> > dependencies in mind. The requires quite a bit of engineering
> > (integrating depmod into kernel build?). Given that there are only a few
> > cases, I don't think effort would be worth it.
>
> Make it an explicit dependancy in the driver, and then things will be
> loaded properly.

I take it you mean using MODULE_SOFTDEP to do that?

> You can always defer your probe if you do not have all
> of the proper resources, which is how these types of things are handled,
> instead of worrying about creating new init levels.


fTPM's probe is already deferred in current Linux without this patch. It
will eventually show up in Linux but at that point it is too late for
Linux's Integrity Measurement Architecture to use it.

The probe getting deferred is exactly what I tried to avoid here. :-)

Wei.

>
> thanks,
>
> greg k-h

2021-01-17 14:36:53

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] fTPM: make sure TEE is initialized before fTPM

On Sun, Jan 17, 2021 at 02:21:27PM +0000, Wei Liu wrote:
> On Sun, Jan 17, 2021 at 09:29:42AM +0100, Greg Kroah-Hartman wrote:
> > On Sat, Jan 16, 2021 at 12:11:09PM +0000, Wei Liu wrote:
> > > On Sat, Jan 16, 2021 at 11:55:29AM +0000, Wei Liu wrote:
> > > > On Fri, Jan 15, 2021 at 04:49:57PM -0800, Randy Dunlap wrote:
> > > > > Hi,
> > > > >
> > > > > On 1/15/21 4:12 PM, Wei Liu wrote:
> > > > > > For built-in drivers, the order of initialization function invocation is
> > > > > > determined by their link order.
> > > > > >
> > > > > > The original code linked TPM drivers before TEE driver when they were
> > > > > > both built in. That caused fTPM's initialization to be deferred to a
> > > > > > worker thread instead of running on PID 1.
> > > > > >
> > > > > > That is problematic because IMA's initialization routine, which runs on
> > > > > > PID 1 as a late initcall, needs to have access to the default TPM
> > > > > > instance. If fTPM's initialization is deferred, IMA will not be able to
> > > > > > get hold of a TPM instance in time.
> > > > > >
> > > > > > Fix this by modifying Makefile to make sure TEE is initialized before
> > > > > > fTPM when they are both built in.
> > > > > >
> > > > > > Signed-off-by: Wei Liu <[email protected]>
> > > > > > ---
> > > > > > drivers/Makefile | 5 +++++
> > > > > > 1 file changed, 5 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/Makefile b/drivers/Makefile
> > > > > > index fd11b9ac4cc3..45ea5ec9d0fd 100644
> > > > > > --- a/drivers/Makefile
> > > > > > +++ b/drivers/Makefile
> > > > > > @@ -180,6 +180,11 @@ obj-$(CONFIG_NVMEM) += nvmem/
> > > > > > obj-$(CONFIG_FPGA) += fpga/
> > > > > > obj-$(CONFIG_FSI) += fsi/
> > > > > > obj-$(CONFIG_TEE) += tee/
> > > > > > +
> > > > > > +# TPM drivers must come after TEE, otherwise fTPM initialization will be
> > > > > > +# deferred, which causes IMA to not get a TPM device in time
> > > > > > +obj-$(CONFIG_TCG_TPM) += char/tpm/
> > > > > > +
> > > > > > obj-$(CONFIG_MULTIPLEXER) += mux/
> > > > > > obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/
> > > > > > obj-$(CONFIG_SIOX) += siox/
> > > > > >
> > > > >
> > > > > As I suspected and then tested, since you did not remove the other build
> > > > > of char/tpm/, this ends up with multiple definition linker errors (below).
> > > >
> > > > Oops, I didn't commit the hunk that removed the line in char/Makefile.
> > > >
> > > > But I will hold off sending out v2 until the following discussion is
> > > > settled.
> > > >
> > > > >
> > > > > I would think that instead of depending on Makefile order you should use different
> > > > > initcall levels as needed. Depending on Makefile order is what we did 15 years ago.
> > > > >
> > > >
> > > > No, not really. The same trick was used in 2014 (1bacc894c227).
> > > >
> > > > Both TEE and TPM are just drivers. I think they belong to the same level
> > > > (at the moment device_initcall). Looking at the list of levels, I'm not
> > > > sure how I can move TEE to a different level.
> > > >
> > > > Out of the seven levels, which one would you suggest I use for which
> > > > driver?
> > >
> > > A bit more random thought.
> > >
> > > Moving one driver to a different level is not the solution either. What
> > > if there is a dependency chain in the future in which more than 2
> > > drivers are involved? Do we invent more levels or abuse levels that
> > > aren't supposed to be used by device drivers?
> > >
> > > The proper solution to me is to somehow sort the initcalls with their
> > > dependencies in mind. The requires quite a bit of engineering
> > > (integrating depmod into kernel build?). Given that there are only a few
> > > cases, I don't think effort would be worth it.
> >
> > Make it an explicit dependancy in the driver, and then things will be
> > loaded properly.
>
> I take it you mean using MODULE_SOFTDEP to do that?

That's one way, or just explicitly depend on a symbol in the other
module.

> > You can always defer your probe if you do not have all
> > of the proper resources, which is how these types of things are handled,
> > instead of worrying about creating new init levels.
>
>
> fTPM's probe is already deferred in current Linux without this patch.

What patch?

> It
> will eventually show up in Linux but at that point it is too late for
> Linux's Integrity Measurement Architecture to use it.

How can it be "too late"?

> The probe getting deferred is exactly what I tried to avoid here. :-)

Then don't start up IMA without it?

I really don't know, but this feels like something is broken in your
module...

thanks,

greg k-h

2021-01-17 15:13:06

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH] fTPM: make sure TEE is initialized before fTPM

On Sun, Jan 17, 2021 at 03:32:17PM +0100, Greg Kroah-Hartman wrote:
> On Sun, Jan 17, 2021 at 02:21:27PM +0000, Wei Liu wrote:
> > On Sun, Jan 17, 2021 at 09:29:42AM +0100, Greg Kroah-Hartman wrote:
> > > On Sat, Jan 16, 2021 at 12:11:09PM +0000, Wei Liu wrote:
> > > > On Sat, Jan 16, 2021 at 11:55:29AM +0000, Wei Liu wrote:
> > > > > On Fri, Jan 15, 2021 at 04:49:57PM -0800, Randy Dunlap wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On 1/15/21 4:12 PM, Wei Liu wrote:
> > > > > > > For built-in drivers, the order of initialization function invocation is
> > > > > > > determined by their link order.
> > > > > > >
> > > > > > > The original code linked TPM drivers before TEE driver when they were
> > > > > > > both built in. That caused fTPM's initialization to be deferred to a
> > > > > > > worker thread instead of running on PID 1.
> > > > > > >
> > > > > > > That is problematic because IMA's initialization routine, which runs on
> > > > > > > PID 1 as a late initcall, needs to have access to the default TPM
> > > > > > > instance. If fTPM's initialization is deferred, IMA will not be able to
> > > > > > > get hold of a TPM instance in time.
> > > > > > >
> > > > > > > Fix this by modifying Makefile to make sure TEE is initialized before
> > > > > > > fTPM when they are both built in.
> > > > > > >
> > > > > > > Signed-off-by: Wei Liu <[email protected]>
> > > > > > > ---
> > > > > > > drivers/Makefile | 5 +++++
> > > > > > > 1 file changed, 5 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/Makefile b/drivers/Makefile
> > > > > > > index fd11b9ac4cc3..45ea5ec9d0fd 100644
> > > > > > > --- a/drivers/Makefile
> > > > > > > +++ b/drivers/Makefile
> > > > > > > @@ -180,6 +180,11 @@ obj-$(CONFIG_NVMEM) += nvmem/
> > > > > > > obj-$(CONFIG_FPGA) += fpga/
> > > > > > > obj-$(CONFIG_FSI) += fsi/
> > > > > > > obj-$(CONFIG_TEE) += tee/
> > > > > > > +
> > > > > > > +# TPM drivers must come after TEE, otherwise fTPM initialization will be
> > > > > > > +# deferred, which causes IMA to not get a TPM device in time
> > > > > > > +obj-$(CONFIG_TCG_TPM) += char/tpm/
> > > > > > > +
> > > > > > > obj-$(CONFIG_MULTIPLEXER) += mux/
> > > > > > > obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/
> > > > > > > obj-$(CONFIG_SIOX) += siox/
> > > > > > >
> > > > > >
> > > > > > As I suspected and then tested, since you did not remove the other build
> > > > > > of char/tpm/, this ends up with multiple definition linker errors (below).
> > > > >
> > > > > Oops, I didn't commit the hunk that removed the line in char/Makefile.
> > > > >
> > > > > But I will hold off sending out v2 until the following discussion is
> > > > > settled.
> > > > >
> > > > > >
> > > > > > I would think that instead of depending on Makefile order you should use different
> > > > > > initcall levels as needed. Depending on Makefile order is what we did 15 years ago.
> > > > > >
> > > > >
> > > > > No, not really. The same trick was used in 2014 (1bacc894c227).
> > > > >
> > > > > Both TEE and TPM are just drivers. I think they belong to the same level
> > > > > (at the moment device_initcall). Looking at the list of levels, I'm not
> > > > > sure how I can move TEE to a different level.
> > > > >
> > > > > Out of the seven levels, which one would you suggest I use for which
> > > > > driver?
> > > >
> > > > A bit more random thought.
> > > >
> > > > Moving one driver to a different level is not the solution either. What
> > > > if there is a dependency chain in the future in which more than 2
> > > > drivers are involved? Do we invent more levels or abuse levels that
> > > > aren't supposed to be used by device drivers?
> > > >
> > > > The proper solution to me is to somehow sort the initcalls with their
> > > > dependencies in mind. The requires quite a bit of engineering
> > > > (integrating depmod into kernel build?). Given that there are only a few
> > > > cases, I don't think effort would be worth it.
> > >
> > > Make it an explicit dependancy in the driver, and then things will be
> > > loaded properly.
> >
> > I take it you mean using MODULE_SOFTDEP to do that?
>
> That's one way, or just explicitly depend on a symbol in the other
> module.
>
> > > You can always defer your probe if you do not have all
> > > of the proper resources, which is how these types of things are handled,
> > > instead of worrying about creating new init levels.
> >
> >
> > fTPM's probe is already deferred in current Linux without this patch.
>
> What patch?

The one you're replying to right now.

>
> > It
> > will eventually show up in Linux but at that point it is too late for
> > Linux's Integrity Measurement Architecture to use it.
>
> How can it be "too late"?

See security/integrity/ima/ima_init.c.

int __init ima_init(void)
{
int rc;

ima_tpm_chip = tpm_default_chip();
if (!ima_tpm_chip)
pr_info("No TPM chip found, activating TPM-bypass!\n");
...
}

ima_init is called by init_ima which is a late initcall.

>
> > The probe getting deferred is exactly what I tried to avoid here. :-)
>
> Then don't start up IMA without it?

IMA can't be built as a module, so it seems to me you're suggesting
adding synchronization between IMA and TPM drivers.

I'm not sure IMA initialization can or should be deferred though. As the
whole point is to measure the system.

>
> I really don't know, but this feels like something is broken in your
> module...
>

I don't disagree. I think having some sort of synchronization between
IMA and TPM drivers may work too. But that's outside of my expertise
and I will leave that to IMA and TPM maintainers.

Wei.

> thanks,
>
> greg k-h