diff mbox series

[V2,2/3] dt-bindings: chosen: Document ima-kexec-buffer

Message ID 20200618071045.471131-3-prsriva@linux.microsoft.com
State Changes Requested
Headers show
Series Adding support for carrying IMA measurement logs | expand

Checks

Context Check Description
robh/checkpatch warning total: 1 errors, 0 warnings, 20 lines checked

Commit Message

Prakhar Srivastava June 18, 2020, 7:10 a.m. UTC
Integrity measurement architecture(IMA) validates if files
have been accidentally or maliciously altered, both remotely and
locally, appraise a file's measurement against a "good" value stored
as an extended attribute, and enforce local file integrity.

IMA also measures singatures of kernel and initrd during kexec along with
the command line used for kexec.
These measurements are critical to verify the seccurity posture of the OS.

Resering memory and adding the memory information to a device tree node
acts as the mechanism to carry over IMA measurement logs.

Update devicetree documentation to reflect the addition of new property
under the chosen node. 

---
 Documentation/devicetree/bindings/chosen.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Thiago Jung Bauermann June 20, 2020, 12:41 a.m. UTC | #1
Prakhar Srivastava <prsriva@linux.microsoft.com> writes:

> Integrity measurement architecture(IMA) validates if files
> have been accidentally or maliciously altered, both remotely and
> locally, appraise a file's measurement against a "good" value stored
> as an extended attribute, and enforce local file integrity.
>
> IMA also measures singatures of kernel and initrd during kexec along with
> the command line used for kexec.
> These measurements are critical to verify the seccurity posture of the OS.
>
> Resering memory and adding the memory information to a device tree node
> acts as the mechanism to carry over IMA measurement logs.
>
> Update devicetree documentation to reflect the addition of new property
> under the chosen node.

Thank you for writing this documentation patch. It's something I should
have done when I added the powerpc IMA kexec support.

You addressed Rob Herring's comments regarding the commit message, but
not the ones regarding the patch contents.

When posting a new version of the patches, make sure to address all
comments made so far. Addressing a comment doesn't necessarily mean
implementing the requested change. If you don't then you should at least
explain why you chose a different path.

I mention it because this has occurred before with this patch series,
and it's hard to make forward progress if review comments get ignored.

> ---
>  Documentation/devicetree/bindings/chosen.txt | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
> index 45e79172a646..a15f70c007ef 100644
> --- a/Documentation/devicetree/bindings/chosen.txt
> +++ b/Documentation/devicetree/bindings/chosen.txt
> @@ -135,3 +135,20 @@ e.g.
>  		linux,initrd-end = <0x82800000>;
>  	};
>  };
> +
> +linux,ima-kexec-buffer
> +----------------------
> +
> +This property(currently used by powerpc, arm64) holds the memory range,

space before the parenthesis.

> +the address and the size, of the IMA measurement logs that are being carried

Maybe it's because English isn't my first language, but IMHO it's
clearer if "the address and the size" is between parentheses rather than
commas.

> +over to the kexec session.

I don't think there's a "kexec session", but I'm not sure what a good
term would be. "linux,booted-from-kexec" uses "new kernel" so perhaps
that's a good option to use instead of "kexec session".

> +
> +/ {
> +	chosen {
> +		linux,ima-kexec-buffer = <0x9 0x82000000 0x0 0x00008000>;
> +	};
> +};
> +
> +This porperty does not represent real hardware, but the memory allocated for
> +carrying the IMA measurement logs. The address and the suze are expressed in
> +#address-cells and #size-cells, respectively of the root node.


--
Thiago Jung Bauermann
IBM Linux Technology Center
Prakhar Srivastava July 13, 2020, 8:32 p.m. UTC | #2
On 6/19/20 5:41 PM, Thiago Jung Bauermann wrote:
> 
> Prakhar Srivastava <prsriva@linux.microsoft.com> writes:
> 
>> Integrity measurement architecture(IMA) validates if files
>> have been accidentally or maliciously altered, both remotely and
>> locally, appraise a file's measurement against a "good" value stored
>> as an extended attribute, and enforce local file integrity.
>>
>> IMA also measures singatures of kernel and initrd during kexec along with
>> the command line used for kexec.
>> These measurements are critical to verify the seccurity posture of the OS.
>>
>> Resering memory and adding the memory information to a device tree node
>> acts as the mechanism to carry over IMA measurement logs.
>>
>> Update devicetree documentation to reflect the addition of new property
>> under the chosen node.
> 
> Thank you for writing this documentation patch. It's something I should
> have done when I added the powerpc IMA kexec support.
> 
> You addressed Rob Herring's comments regarding the commit message, but
> not the ones regarding the patch contents.
> 
> When posting a new version of the patches, make sure to address all
> comments made so far. Addressing a comment doesn't necessarily mean
> implementing the requested change. If you don't then you should at least
> explain why you chose a different path.
> 
> I mention it because this has occurred before with this patch series,
> and it's hard to make forward progress if review comments get ignored.
> 
>> ---
>>   Documentation/devicetree/bindings/chosen.txt | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
>> index 45e79172a646..a15f70c007ef 100644
>> --- a/Documentation/devicetree/bindings/chosen.txt
>> +++ b/Documentation/devicetree/bindings/chosen.txt
>> @@ -135,3 +135,20 @@ e.g.
>>   		linux,initrd-end = <0x82800000>;
>>   	};
>>   };
>> +
>> +linux,ima-kexec-buffer
>> +----------------------
>> +
>> +This property(currently used by powerpc, arm64) holds the memory range,
> 
> space before the parenthesis.
> 
>> +the address and the size, of the IMA measurement logs that are being carried
> 
> Maybe it's because English isn't my first language, but IMHO it's
> clearer if "the address and the size" is between parentheses rather than
> commas.
> 
>> +over to the kexec session.
> 
> I don't think there's a "kexec session", but I'm not sure what a good
> term would be. "linux,booted-from-kexec" uses "new kernel" so perhaps
> that's a good option to use instead of "kexec session".
> 
>> +
>> +/ {
>> +	chosen {
>> +		linux,ima-kexec-buffer = <0x9 0x82000000 0x0 0x00008000>;
>> +	};
>> +};
>> +
>> +This porperty does not represent real hardware, but the memory allocated for
>> +carrying the IMA measurement logs. The address and the suze are expressed in
>> +#address-cells and #size-cells, respectively of the root node.
> 
> 
I will update the descriptions and ack the comments/changes in the 
patches as well.

Thankyou,
Prakhar Srivastava
> --
> Thiago Jung Bauermann
> IBM Linux Technology Center
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
index 45e79172a646..a15f70c007ef 100644
--- a/Documentation/devicetree/bindings/chosen.txt
+++ b/Documentation/devicetree/bindings/chosen.txt
@@ -135,3 +135,20 @@  e.g.
 		linux,initrd-end = <0x82800000>;
 	};
 };
+
+linux,ima-kexec-buffer
+----------------------
+
+This property(currently used by powerpc, arm64) holds the memory range,
+the address and the size, of the IMA measurement logs that are being carried
+over to the kexec session.
+
+/ {
+	chosen {
+		linux,ima-kexec-buffer = <0x9 0x82000000 0x0 0x00008000>;
+	};
+};
+
+This porperty does not represent real hardware, but the memory allocated for
+carrying the IMA measurement logs. The address and the suze are expressed in
+#address-cells and #size-cells, respectively of the root node.