| Message ID | 20240311132030.1103122-2-stefanb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Preserve TPM log across kexec | expand |
Le 11/03/2024 à 14:20, Stefan Berger a écrit : > linux,sml-base holds the address of a buffer with the TPM log. This > buffer may become invalid after a kexec. To avoid accessing an invalid > address or corrupted buffer, embed the whole TPM log in the device tree > property linux,sml-log. This helps to protect the log since it is > properly carried across a kexec soft reboot with both of the kexec > syscalls. > > Avoid having the firmware ingest the whole TPM log when calling > prom_setprop but only create the linux,sml-log property as a place holder. > Insert the actual TPM log during the tree flattening phase. > > Fixes: 4a727429abec ("PPC64: Add support for instantiating SML from Open Firmware") > Suggested-by: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > arch/powerpc/kernel/prom_init.c | 27 +++++++++++++++++++-------- > 1 file changed, 19 insertions(+), 8 deletions(-) > > diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c > index e67effdba85c..6f7ca72013c2 100644 > --- a/arch/powerpc/kernel/prom_init.c > +++ b/arch/powerpc/kernel/prom_init.c > @@ -211,6 +211,8 @@ static cell_t __prombss regbuf[1024]; > > static bool __prombss rtas_has_query_cpu_stopped; > > +static u64 __prombss sml_base; > +static u32 __prombss sml_size; > > /* > * Error results ... some OF calls will return "-1" on error, some > @@ -1954,17 +1956,15 @@ static void __init prom_instantiate_sml(void) > } > prom_printf(" done\n"); > > - reserve_mem(base, size); > - > - prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-base", > - &base, sizeof(base)); > - prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-size", > - &size, sizeof(size)); > - > - prom_debug("sml base = 0x%llx\n", base); > + /* Add property now, defer adding log to tree flattening phase */ > + prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-log", > + NULL, 0); > prom_debug("sml size = 0x%x\n", size); > > prom_debug("prom_instantiate_sml: end...\n"); > + > + sml_base = base; > + sml_size = size; > } > > /* > @@ -2645,6 +2645,17 @@ static void __init scan_dt_build_struct(phandle node, unsigned long *mem_start, > } > prev_name = sstart + soff; > > + if (!prom_strcmp("linux,sml-log", pname)) { > + /* push property head */ > + dt_push_token(OF_DT_PROP, mem_start, mem_end); > + dt_push_token(sml_size, mem_start, mem_end); > + dt_push_token(soff, mem_start, mem_end); > + /* push property content */ > + valp = make_room(mem_start, mem_end, sml_size, 1); > + memcpy(valp, (void *)sml_base, sml_size); You can't cast a u64 into a pointer. If sml_base is an address, it must be declared as an unsigned long. Build with pmac32_defconfig : CC arch/powerpc/kernel/prom_init.o arch/powerpc/kernel/prom_init.c: In function 'scan_dt_build_struct': arch/powerpc/kernel/prom_init.c:2663:38: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] 2663 | memcpy(valp, (void *)sml_base, sml_size); | ^ cc1: all warnings being treated as errors make[4]: *** [scripts/Makefile.build:243: arch/powerpc/kernel/prom_init.o] Error 1 > + *mem_start = ALIGN(*mem_start, 4); > + continue; > + } > /* get length */ > l = call_prom("getproplen", 2, 1, node, pname); >
On Mon, Mar 11, 2024 at 09:20:28AM -0400, Stefan Berger wrote: > linux,sml-base holds the address of a buffer with the TPM log. This > buffer may become invalid after a kexec. To avoid accessing an invalid > address or corrupted buffer, embed the whole TPM log in the device tree > property linux,sml-log. This helps to protect the log since it is > properly carried across a kexec soft reboot with both of the kexec > syscalls. > > Avoid having the firmware ingest the whole TPM log when calling > prom_setprop but only create the linux,sml-log property as a place holder. > Insert the actual TPM log during the tree flattening phase. > > Fixes: 4a727429abec ("PPC64: Add support for instantiating SML from Open Firmware") > Suggested-by: Michael Ellerman <mpe@ellerman.id.au> > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > arch/powerpc/kernel/prom_init.c | 27 +++++++++++++++++++-------- > 1 file changed, 19 insertions(+), 8 deletions(-) > > diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c > index e67effdba85c..6f7ca72013c2 100644 > --- a/arch/powerpc/kernel/prom_init.c > +++ b/arch/powerpc/kernel/prom_init.c > @@ -211,6 +211,8 @@ static cell_t __prombss regbuf[1024]; > > static bool __prombss rtas_has_query_cpu_stopped; > > +static u64 __prombss sml_base; > +static u32 __prombss sml_size; Should inside an #ifdef CONFIG_PPC64 since prom_instantiate_sml() is? > > /* > * Error results ... some OF calls will return "-1" on error, some > @@ -1954,17 +1956,15 @@ static void __init prom_instantiate_sml(void) > } > prom_printf(" done\n"); > > - reserve_mem(base, size); > - > - prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-base", > - &base, sizeof(base)); > - prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-size", > - &size, sizeof(size)); > - > - prom_debug("sml base = 0x%llx\n", base); > + /* Add property now, defer adding log to tree flattening phase */ > + prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-log", > + NULL, 0); > prom_debug("sml size = 0x%x\n", size); > > prom_debug("prom_instantiate_sml: end...\n"); > + > + sml_base = base; > + sml_size = size; > } > > /* > @@ -2645,6 +2645,17 @@ static void __init scan_dt_build_struct(phandle node, unsigned long *mem_start, > } > prev_name = sstart + soff; > > + if (!prom_strcmp("linux,sml-log", pname)) { > + /* push property head */ > + dt_push_token(OF_DT_PROP, mem_start, mem_end); > + dt_push_token(sml_size, mem_start, mem_end); > + dt_push_token(soff, mem_start, mem_end); > + /* push property content */ > + valp = make_room(mem_start, mem_end, sml_size, 1); > + memcpy(valp, (void *)sml_base, sml_size); > + *mem_start = ALIGN(*mem_start, 4); > + continue; > + } Same question as above. Regards, Jerry > /* get length */ > l = call_prom("getproplen", 2, 1, node, pname); > > -- > 2.43.0 >
On 3/11/24 13:24, Christophe Leroy wrote: > > > Le 11/03/2024 à 14:20, Stefan Berger a écrit : >> linux,sml-base holds the address of a buffer with the TPM log. This >> buffer may become invalid after a kexec. To avoid accessing an invalid >> address or corrupted buffer, embed the whole TPM log in the device tree >> property linux,sml-log. This helps to protect the log since it is >> properly carried across a kexec soft reboot with both of the kexec >> syscalls. >> >> Avoid having the firmware ingest the whole TPM log when calling >> prom_setprop but only create the linux,sml-log property as a place holder. >> Insert the actual TPM log during the tree flattening phase. >> >> Fixes: 4a727429abec ("PPC64: Add support for instantiating SML from Open Firmware") >> Suggested-by: Michael Ellerman <mpe@ellerman.id.au> >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> --- >> @@ -2645,6 +2645,17 @@ static void __init scan_dt_build_struct(phandle node, unsigned long *mem_start, >> } >> prev_name = sstart + soff; >> >> + if (!prom_strcmp("linux,sml-log", pname)) { >> + /* push property head */ >> + dt_push_token(OF_DT_PROP, mem_start, mem_end); >> + dt_push_token(sml_size, mem_start, mem_end); >> + dt_push_token(soff, mem_start, mem_end); >> + /* push property content */ >> + valp = make_room(mem_start, mem_end, sml_size, 1); >> + memcpy(valp, (void *)sml_base, sml_size); > > You can't cast a u64 into a pointer. If sml_base is an address, it must > be declared as an unsigned long. > > Build with pmac32_defconfig : > > CC arch/powerpc/kernel/prom_init.o > arch/powerpc/kernel/prom_init.c: In function 'scan_dt_build_struct': > arch/powerpc/kernel/prom_init.c:2663:38: error: cast to pointer from > integer of different size [-Werror=int-to-pointer-cast] > 2663 | memcpy(valp, (void *)sml_base, sml_size); > | ^ > cc1: all warnings being treated as errors > make[4]: *** [scripts/Makefile.build:243: > arch/powerpc/kernel/prom_init.o] Error 1 Next round will have this block under #ifdef CONFIG_PPC64. Thanks.
On Mon Mar 11, 2024 at 3:20 PM EET, Stefan Berger wrote: > linux,sml-base holds the address of a buffer with the TPM log. This > buffer may become invalid after a kexec. To avoid accessing an invalid > address or corrupted buffer, embed the whole TPM log in the device tree > property linux,sml-log. This helps to protect the log since it is > properly carried across a kexec soft reboot with both of the kexec > syscalls. - Describe the environment where TPM log gets corrupted. - Describe why TPM log gets corrupted on kexec. > > Avoid having the firmware ingest the whole TPM log when calling > prom_setprop but only create the linux,sml-log property as a place holder. > Insert the actual TPM log during the tree flattening phase. This commit message should shed some light about reasons of the corruption in order to conclude that it should be fixed up like this. I.e. why the "post-state" is a legit state where can be continued despite a log being corrupted. Especially in security features this is pretty essential information. BR, Jarkko
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c index e67effdba85c..6f7ca72013c2 100644 --- a/arch/powerpc/kernel/prom_init.c +++ b/arch/powerpc/kernel/prom_init.c @@ -211,6 +211,8 @@ static cell_t __prombss regbuf[1024]; static bool __prombss rtas_has_query_cpu_stopped; +static u64 __prombss sml_base; +static u32 __prombss sml_size; /* * Error results ... some OF calls will return "-1" on error, some @@ -1954,17 +1956,15 @@ static void __init prom_instantiate_sml(void) } prom_printf(" done\n"); - reserve_mem(base, size); - - prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-base", - &base, sizeof(base)); - prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-size", - &size, sizeof(size)); - - prom_debug("sml base = 0x%llx\n", base); + /* Add property now, defer adding log to tree flattening phase */ + prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-log", + NULL, 0); prom_debug("sml size = 0x%x\n", size); prom_debug("prom_instantiate_sml: end...\n"); + + sml_base = base; + sml_size = size; } /* @@ -2645,6 +2645,17 @@ static void __init scan_dt_build_struct(phandle node, unsigned long *mem_start, } prev_name = sstart + soff; + if (!prom_strcmp("linux,sml-log", pname)) { + /* push property head */ + dt_push_token(OF_DT_PROP, mem_start, mem_end); + dt_push_token(sml_size, mem_start, mem_end); + dt_push_token(soff, mem_start, mem_end); + /* push property content */ + valp = make_room(mem_start, mem_end, sml_size, 1); + memcpy(valp, (void *)sml_base, sml_size); + *mem_start = ALIGN(*mem_start, 4); + continue; + } /* get length */ l = call_prom("getproplen", 2, 1, node, pname);
linux,sml-base holds the address of a buffer with the TPM log. This buffer may become invalid after a kexec. To avoid accessing an invalid address or corrupted buffer, embed the whole TPM log in the device tree property linux,sml-log. This helps to protect the log since it is properly carried across a kexec soft reboot with both of the kexec syscalls. Avoid having the firmware ingest the whole TPM log when calling prom_setprop but only create the linux,sml-log property as a place holder. Insert the actual TPM log during the tree flattening phase. Fixes: 4a727429abec ("PPC64: Add support for instantiating SML from Open Firmware") Suggested-by: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- arch/powerpc/kernel/prom_init.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)