diff mbox series

[v4,2/5] powerpc: Use libfdt functions to fetch IMA buffer properties

Message ID 20200819172134.11243-3-nramas@linux.microsoft.com (mailing list archive)
State New, archived
Headers show
Series Carry forward IMA measurement log on kexec on ARM64 | expand

Commit Message

Lakshmi Ramasubramanian Aug. 19, 2020, 5:21 p.m. UTC 
remove_ima_buffer() uses custom code to handle properties of
the IMA buffer, such as the buffer's address, size, etc., in 
the device tree. Flat Device Tree (FDT) library (libfdt) provides
helper functions for handling device tree node properties and
they should be used instead.

Use libfdt functions for handling IMA buffer properties in
the device tree node for powerpc.

Co-developed-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
 arch/powerpc/kexec/ima.c | 63 ++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 38 deletions(-)

Comments

Thiago Jung Bauermann Aug. 27, 2020, 11:50 p.m. UTC | #1 
Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:

> @@ -63,7 +29,22 @@ void remove_ima_buffer(void *fdt, int chosen_node)
>  	if (!prop)
>  		return;
>  
> -	ret = do_get_kexec_buffer(prop, len, &addr, &size);
> +	ret = fdt_address_cells(fdt, chosen_node);

This change was already present in the previous version of the patch but
it was just today that I noticed a problem: there's no #address-cells
property in /chosen. This code will still work though because if there's
no property this function returns 2 which is the correct value for
ppc64. But it's conceptually wrong. You need to pass the node offset for
/ so that it gets the #address-cells property from there.

> +	if (ret < 0)
> +		return;
> +	addr_cells = ret;
> +
> +	ret = fdt_size_cells(fdt, chosen_node);

Here we're not so lucky. The default value returned when no #size-cells
property is present is 1, which is wrong for ppc64 so this change
introduces a bug. You also need to pass the node offset for / here.

> +	if (ret < 0)
> +		return;
> +	size_cells = ret;
> +
> +	if (len < 4 * (addr_cells + size_cells))
> +		return;
> +
> +	addr = of_read_number(prop, addr_cells);
> +	size = of_read_number(prop + 4 * addr_cells, size_cells);
> +
>  	fdt_delprop(fdt, chosen_node, FDT_PROP_IMA_KEXEC_BUFFER);
>  	if (ret)
>  		return;
> @@ -129,9 +110,15 @@ int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node)
>  	if (!image->arch.ima_buffer_size)
>  		return 0;
>  
> -	ret = get_addr_size_cells(&addr_cells, &size_cells);
> -	if (ret)
> +	ret = fdt_address_cells(fdt, chosen_node);
> +	if (ret < 0)
> +		return ret;
> +	addr_cells = ret;
> +
> +	ret = fdt_size_cells(fdt, chosen_node);
> +	if (ret < 0)
>  		return ret;
> +	size_cells = ret;
>  
>  	entry_size = 4 * (addr_cells + size_cells);

Ditto here.
Lakshmi Ramasubramanian Aug. 28, 2020, 5:46 p.m. UTC | #2 
On 8/27/20 4:50 PM, Thiago Jung Bauermann wrote:
> 
> Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
> 
>> @@ -63,7 +29,22 @@ void remove_ima_buffer(void *fdt, int chosen_node)
>>   	if (!prop)
>>   		return;
>>   
>> -	ret = do_get_kexec_buffer(prop, len, &addr, &size);
>> +	ret = fdt_address_cells(fdt, chosen_node);
> 
> This change was already present in the previous version of the patch but
> it was just today that I noticed a problem: there's no #address-cells
> property in /chosen. This code will still work though because if there's
> no property this function returns 2 which is the correct value for
> ppc64. But it's conceptually wrong. You need to pass the node offset for
> / so that it gets the #address-cells property from there.

Thanks for the info.
Will fix this.

> 
>> +	if (ret < 0)
>> +		return;
>> +	addr_cells = ret;
>> +
>> +	ret = fdt_size_cells(fdt, chosen_node);
> 
> Here we're not so lucky. The default value returned when no #size-cells
> property is present is 1, which is wrong for ppc64 so this change
> introduces a bug. You also need to pass the node offset for / here.

Will fix this.

> 
>> +	if (ret < 0)
>> +		return;
>> +	size_cells = ret;
>> +
>> +	if (len < 4 * (addr_cells + size_cells))
>> +		return;
>> +
>> +	addr = of_read_number(prop, addr_cells);
>> +	size = of_read_number(prop + 4 * addr_cells, size_cells);
>> +
>>   	fdt_delprop(fdt, chosen_node, FDT_PROP_IMA_KEXEC_BUFFER);
>>   	if (ret)
>>   		return;
>> @@ -129,9 +110,15 @@ int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node)
>>   	if (!image->arch.ima_buffer_size)
>>   		return 0;
>>   
>> -	ret = get_addr_size_cells(&addr_cells, &size_cells);
>> -	if (ret)
>> +	ret = fdt_address_cells(fdt, chosen_node);
>> +	if (ret < 0)
>> +		return ret;
>> +	addr_cells = ret;
>> +
>> +	ret = fdt_size_cells(fdt, chosen_node);
>> +	if (ret < 0)
>>   		return ret;
>> +	size_cells = ret;
>>   
>>   	entry_size = 4 * (addr_cells + size_cells);
> 
> Ditto here.
> 

Will fix this.

thanks,
  -lakshmi
diff mbox series

Patch

diff --git a/arch/powerpc/kexec/ima.c b/arch/powerpc/kexec/ima.c
index f5112ee4bb0b..573ff708d700 100644
--- a/arch/powerpc/kexec/ima.c
+++ b/arch/powerpc/kexec/ima.c
@@ -12,40 +12,6 @@ 
 #include <linux/memblock.h>
 #include <linux/libfdt.h>
 
-static int get_addr_size_cells(int *addr_cells, int *size_cells)
-{
-	struct device_node *root;
-
-	root = of_find_node_by_path("/");
-	if (!root)
-		return -EINVAL;
-
-	*addr_cells = of_n_addr_cells(root);
-	*size_cells = of_n_size_cells(root);
-
-	of_node_put(root);
-
-	return 0;
-}
-
-static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
-			       size_t *size)
-{
-	int ret, addr_cells, size_cells;
-
-	ret = get_addr_size_cells(&addr_cells, &size_cells);
-	if (ret)
-		return ret;
-
-	if (len < 4 * (addr_cells + size_cells))
-		return -ENOENT;
-
-	*addr = of_read_number(prop, addr_cells);
-	*size = of_read_number(prop + 4 * addr_cells, size_cells);
-
-	return 0;
-}
-
 /**
  * remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
  *
@@ -54,7 +20,7 @@  static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
  */
 void remove_ima_buffer(void *fdt, int chosen_node)
 {
-	int ret, len;
+	int ret, len, addr_cells, size_cells;
 	unsigned long addr;
 	size_t size;
 	const void *prop;
@@ -63,7 +29,22 @@  void remove_ima_buffer(void *fdt, int chosen_node)
 	if (!prop)
 		return;
 
-	ret = do_get_kexec_buffer(prop, len, &addr, &size);
+	ret = fdt_address_cells(fdt, chosen_node);
+	if (ret < 0)
+		return;
+	addr_cells = ret;
+
+	ret = fdt_size_cells(fdt, chosen_node);
+	if (ret < 0)
+		return;
+	size_cells = ret;
+
+	if (len < 4 * (addr_cells + size_cells))
+		return;
+
+	addr = of_read_number(prop, addr_cells);
+	size = of_read_number(prop + 4 * addr_cells, size_cells);
+
 	fdt_delprop(fdt, chosen_node, FDT_PROP_IMA_KEXEC_BUFFER);
 	if (ret)
 		return;
@@ -129,9 +110,15 @@  int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node)
 	if (!image->arch.ima_buffer_size)
 		return 0;
 
-	ret = get_addr_size_cells(&addr_cells, &size_cells);
-	if (ret)
+	ret = fdt_address_cells(fdt, chosen_node);
+	if (ret < 0)
+		return ret;
+	addr_cells = ret;
+
+	ret = fdt_size_cells(fdt, chosen_node);
+	if (ret < 0)
 		return ret;
+	size_cells = ret;
 
 	entry_size = 4 * (addr_cells + size_cells);