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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 12 Mar 2024 12:11:09 +0100
From: Lukas Wunner <lukas@...ner.de>
To: Stefan Berger <stefanb@...ux.ibm.com>
Cc: mpe@...erman.id.au, linux-integrity@...r.kernel.org,
	linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
	jarkko@...nel.org, rnsastry@...ux.ibm.com, peterhuewe@....de,
	viparash@...ibm.com, devicetree@...r.kernel.org,
	jsnitsel@...hat.com, Nayna Jain <nayna@...ux.ibm.com>
Subject: Re: [RFC PATCH v2 2/3] dt-bindings: tpm: Add linux,sml-log to
 ibm,vtpm.yaml

On Mon, Mar 11, 2024 at 09:20:29AM -0400, Stefan Berger wrote:
> Add linux,sml-log, which carries the firmware TPM log in a uint8-array, to
> the properties. Either this property is required or both linux,sml-base and
> linux,sml-size are required. Add a test case for verification.
> 
> Fixes: 82003e0487fb ("Documentation: tpm: add the IBM Virtual TPM device tree binding documentation")

The Fixes tag is confusing.  The patch won't even apply cleanly to the
v4.10 commit referenced here as the conversion to yaml happened only
recently with v6.8.

Why is the Fixes tag necessary in the first place?  Same question for
the other patches in the series.  This looks like feature work rather
than a fix.  Not sure whether it satisfies the "obviously correct"
rule per Documentation/process/stable-kernel-rules.rst.


> --- 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

I assume that either these two or the new "linux,sml-log" property
are (still) required?  If so, a quick grep through the bindings
(e.g. auxdisplay/img,ascii-lcd.yaml) shows that the following
might work:

required:
  - ...

oneOf:
  - required:
      - linux,sml-base
  - required:
      - linux,sml-log


> --- 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:
> +      Content of firmware event log

Please add one or two sentences of context so that readers don't
need to use git blame + git log to find out what this is for.
(Mention at least that the property may be used to pass the log
to a kexec kernel.)


> -# 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

Could you add "linux,sml-size: false" to "memory-region" as well
while at it for consistency?

Thanks,

Lukas

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ