| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <768fc5f1-3919-477e-a8e6-16a7e8536add@linux.ibm.com>
Date: Thu, 7 Mar 2024 10:11:03 -0500
From: Stefan Berger <stefanb@...ux.ibm.com>
To: Michael Ellerman <mpe@...erman.id.au>, linux-integrity@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org, conor.dooley@...rochip.com,
nayna@...ux.ibm.com, Lukas Wunner <lukas@...ner.de>
Cc: linux-kernel@...r.kernel.org, jarkko@...nel.org, rnsastry@...ux.ibm.com,
peterhuewe@....de, viparash@...ibm.com
Subject: Re: [PATCH 1/2] powerpc/prom_init: Replace linux,sml-base/sml-size
with linux,sml-log
On 3/7/24 05:41, Michael Ellerman wrote:
> Stefan Berger <stefanb@...ux.ibm.com> writes:
>> linux,sml-base holds the address of a buffer with the TPM log. This
>> buffer may become invalid after a kexec and therefore embed the whole TPM
>> log in linux,sml-log. This helps to protect the log since it is properly
>> carried across a kexec with both of the kexec syscalls.
>>
>> Signed-off-by: Stefan Berger <stefanb@...ux.ibm.com>
>> ---
>> arch/powerpc/kernel/prom_init.c | 8 ++------
>> 1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
>> index e67effdba85c..41268c30de4c 100644
>> --- a/arch/powerpc/kernel/prom_init.c
>> +++ b/arch/powerpc/kernel/prom_init.c
>> @@ -1956,12 +1956,8 @@ static void __init prom_instantiate_sml(void)
>>
>> 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);
>> + prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-log",
>> + (void *)base, size);
>
> As we discussed via chat, doing it this way sucks the full content of
> the log back into Open Firmware.
>
> That relies on OF handling such big properties, and also means more
> memory will be consumed, which can cause problems early in boot.
>
> A better solution is to explicitly add the log to the FDT in the
> flattening phase.
Done.
>
> Also adding the new linux,sml-log property should be accompanied by a
> change to the device tree binding.
See my proposal below.
>
> The syntax is not very obvious to me, but possibly something like?
>
> diff --git a/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml b/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml
> index 50a3fd31241c..cd75037948bc 100644
> --- a/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml
> +++ b/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml
> @@ -74,8 +74,6 @@ required:
> - ibm,my-dma-window
> - ibm,my-drc-index
> - ibm,loc-code
> - - linux,sml-base
> - - linux,sml-size
>
> allOf:
> - $ref: tpm-common.yaml#
> diff --git a/Documentation/devicetree/bindings/tpm/tpm-common.yaml b/Documentation/devicetree/bindings/tpm/tpm-common.yaml
> index 3c1241b2a43f..616604707c95 100644
> --- a/Documentation/devicetree/bindings/tpm/tpm-common.yaml
> +++ b/Documentation/devicetree/bindings/tpm/tpm-common.yaml
> @@ -25,6 +25,11 @@ properties:
> base address of reserved memory allocated for firmware event log
> $ref: /schemas/types.yaml#/definitions/uint64
>
> + linux,sml-log:
> + description:
> + Content of firmware event log
> + $ref: /schemas/types.yaml#/definitions/uint8-array
> +
> linux,sml-size:
> description:
> size of reserved memory allocated for firmware event log
> @@ -53,15 +58,22 @@ dependentRequired:
> linux,sml-base: ['linux,sml-size']
> linux,sml-size: ['linux,sml-base']
>
> -# must only have either memory-region or linux,sml-base
> +# must only have either memory-region or linux,sml-base/size or linux,sml-log
> # as well as either resets or reset-gpios
> dependentSchemas:
> memory-region:
> properties:
> linux,sml-base: false
> + linux,sml-log: false
> linux,sml-base:
> properties:
> memory-region: false
> + linux,sml-log: false
> + linux,sml-log:
> + properties:
> + memory-region: false
> + linux,sml-base: false
> + linux,sml-size: false
> resets:
> properties:
> reset-gpios: false
>
>
I have been working with this patch here now and it passes the following
test:
make dt_binding_check dtbs_check DT_SCHEMA_FILES=tpm/ibm,vtpm.yaml
diff --git a/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml
b/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml
index 50a3fd31241c..cacf6c3082de 100644
--- a/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml
+++ b/Documentation/devicetree/bindings/tpm/ibm,vtpm.yaml
@@ -74,8 +74,12 @@ required:
- ibm,my-dma-window
- ibm,my-drc-index
- ibm,loc-code
- - linux,sml-base
- - linux,sml-size
+oneOf:
+ - required:
+ - linux,sml-base
+ - linux,sml-size
+ - required:
+ - linux,sml-log
allOf:
- $ref: tpm-common.yaml#
@@ -102,3 +106,21 @@ examples:
linux,sml-size = <0xbce10200>;
};
};
+ - |
+ soc {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ tpm@...00003 {
+ compatible = "IBM,vtpm";
+ device_type = "IBM,vtpm";
+ reg = <0x30000003>;
+ interrupts = <0xa0003 0x0>;
+ ibm,#dma-address-cells = <0x2>;
+ ibm,#dma-size-cells = <0x2>;
+ ibm,my-dma-window = <0x10000003 0x0 0x0 0x0 0x10000000>;
+ ibm,my-drc-index = <0x30000003>;
+ ibm,loc-code = "U8286.41A.10082DV-V3-C3";
+ linux,sml-log = <00 00 00 00 03 00 00>;
+ };
+ };
diff --git a/Documentation/devicetree/bindings/tpm/tpm-common.yaml
b/Documentation/devicetree/bindings/tpm/tpm-common.yaml
index 3c1241b2a43f..591c48f8cb74 100644
--- a/Documentation/devicetree/bindings/tpm/tpm-common.yaml
+++ b/Documentation/devicetree/bindings/tpm/tpm-common.yaml
@@ -30,6 +30,11 @@ properties:
size of reserved memory allocated for firmware event log
$ref: /schemas/types.yaml#/definitions/uint32
+ linux,sml-log:
+ description:
+ firmware event log
+ $ref: /schemas/types.yaml#/definitions/uint8-array
+
memory-region:
description: reserved memory allocated for firmware event log
maxItems: 1
Is my patch missing something?
Powered by blists - more mailing lists