All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
@ 2021-04-07 20:51 frowand.list
  2021-04-07 20:59 ` Frank Rowand
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: frowand.list @ 2021-04-07 20:51 UTC (permalink / raw)
  To: Rob Herring, Guenter Roeck
  Cc: Pantelis Antoniou, devicetree, Geert Uytterhoeven, linux-kernel

From: Frank Rowand <frank.rowand@sony.com>

The Devicetree standard specifies an 8 byte alignment of the FDT.
Code in libfdt expects this alignment for an FDT image in memory.
kmemdup() returns 4 byte alignment on openrisc.  Replace kmemdup()
with kmalloc(), align pointer, memcpy() to get proper alignment.

The 4 byte alignment exposed a related bug which triggered a crash
on openrisc with:
commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9")
as reported in:
https://lore.kernel.org/lkml/20210327224116.69309-1-linux@roeck-us.net/

Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
 drivers/of/of_private.h | 2 ++
 drivers/of/overlay.c    | 8 ++++++--
 drivers/of/unittest.c   | 9 +++++++--
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index d9e6a324de0a..d717efbd637d 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -8,6 +8,8 @@
  * Copyright (C) 1996-2005 Paul Mackerras.
  */
 
+#define FDT_ALIGN_SIZE 8
+
 /**
  * struct alias_prop - Alias property in 'aliases' node
  * @link:	List node to link the structure in aliases_lookup list
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 50bbe0edf538..8b40711ed202 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -1014,7 +1014,7 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
 int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
 			 int *ovcs_id)
 {
-	const void *new_fdt;
+	void *new_fdt;
 	int ret;
 	u32 size;
 	struct device_node *overlay_root;
@@ -1036,10 +1036,14 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
 	 * Must create permanent copy of FDT because of_fdt_unflatten_tree()
 	 * will create pointers to the passed in FDT in the unflattened tree.
 	 */
-	new_fdt = kmemdup(overlay_fdt, size, GFP_KERNEL);
+	size += FDT_ALIGN_SIZE;
+	new_fdt = kmalloc(size, GFP_KERNEL);
 	if (!new_fdt)
 		return -ENOMEM;
 
+	new_fdt = PTR_ALIGN(new_fdt, FDT_ALIGN_SIZE);
+	memcpy(new_fdt, overlay_fdt, size);
+
 	of_fdt_unflatten_tree(new_fdt, NULL, &overlay_root);
 	if (!overlay_root) {
 		pr_err("unable to unflatten overlay_fdt\n");
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index eb100627c186..edd6ce807691 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/device.h>
 #include <linux/platform_device.h>
+#include <linux/kernel.h>
 
 #include <linux/i2c.h>
 #include <linux/i2c-mux.h>
@@ -1415,7 +1416,7 @@ static int __init unittest_data_add(void)
 	 */
 	extern uint8_t __dtb_testcases_begin[];
 	extern uint8_t __dtb_testcases_end[];
-	const int size = __dtb_testcases_end - __dtb_testcases_begin;
+	u32 size = __dtb_testcases_end - __dtb_testcases_begin;
 	int rc;
 
 	if (!size) {
@@ -1425,10 +1426,14 @@ static int __init unittest_data_add(void)
 	}
 
 	/* creating copy */
-	unittest_data = kmemdup(__dtb_testcases_begin, size, GFP_KERNEL);
+	size += FDT_ALIGN_SIZE;
+	unittest_data = kmalloc(size, GFP_KERNEL);
 	if (!unittest_data)
 		return -ENOMEM;
 
+	unittest_data = PTR_ALIGN(unittest_data, FDT_ALIGN_SIZE);
+	memcpy(unittest_data, __dtb_testcases_begin, size);
+
 	of_fdt_unflatten_tree(unittest_data, NULL, &unittest_data_node);
 	if (!unittest_data_node) {
 		pr_warn("%s: No tree to attach; not running tests\n", __func__);
-- 
Frank Rowand <frank.rowand@sony.com>


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
  2021-04-07 20:51 [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT frowand.list
@ 2021-04-07 20:59 ` Frank Rowand
  2021-04-07 22:01   ` Guenter Roeck
  2021-04-07 21:34 ` Rob Herring
  2021-04-09  9:52   ` Dan Carpenter
  2 siblings, 1 reply; 11+ messages in thread
From: Frank Rowand @ 2021-04-07 20:59 UTC (permalink / raw)
  To: Rob Herring, Guenter Roeck
  Cc: Pantelis Antoniou, devicetree, Geert Uytterhoeven, linux-kernel

Hi Guenter,

On 4/7/21 3:51 PM, frowand.list@gmail.com wrote:
> From: Frank Rowand <frank.rowand@sony.com>
> 
> The Devicetree standard specifies an 8 byte alignment of the FDT.
> Code in libfdt expects this alignment for an FDT image in memory.
> kmemdup() returns 4 byte alignment on openrisc.  Replace kmemdup()
> with kmalloc(), align pointer, memcpy() to get proper alignment.
> 
> The 4 byte alignment exposed a related bug which triggered a crash
> on openrisc with:
> commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9")
> as reported in:
> https://lore.kernel.org/lkml/20210327224116.69309-1-linux@roeck-us.net/

Can you please test this patch?

The most complete coverage will be a config with:

 OF_UNITTEST=y
 OF_OVERLAY=y
 OF_DYNAMIC=y

One path will be missed if you test with:

 #OF_OVERLAY=n
 #OF_DYNAMIC=n

Thanks!

-Frank

> 
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
> ---
>  drivers/of/of_private.h | 2 ++
>  drivers/of/overlay.c    | 8 ++++++--
>  drivers/of/unittest.c   | 9 +++++++--
>  3 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index d9e6a324de0a..d717efbd637d 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -8,6 +8,8 @@
>   * Copyright (C) 1996-2005 Paul Mackerras.
>   */
>  
> +#define FDT_ALIGN_SIZE 8
> +
>  /**
>   * struct alias_prop - Alias property in 'aliases' node
>   * @link:	List node to link the structure in aliases_lookup list
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 50bbe0edf538..8b40711ed202 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -1014,7 +1014,7 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
>  int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
>  			 int *ovcs_id)
>  {
> -	const void *new_fdt;
> +	void *new_fdt;
>  	int ret;
>  	u32 size;
>  	struct device_node *overlay_root;
> @@ -1036,10 +1036,14 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
>  	 * Must create permanent copy of FDT because of_fdt_unflatten_tree()
>  	 * will create pointers to the passed in FDT in the unflattened tree.
>  	 */
> -	new_fdt = kmemdup(overlay_fdt, size, GFP_KERNEL);
> +	size += FDT_ALIGN_SIZE;
> +	new_fdt = kmalloc(size, GFP_KERNEL);
>  	if (!new_fdt)
>  		return -ENOMEM;
>  
> +	new_fdt = PTR_ALIGN(new_fdt, FDT_ALIGN_SIZE);
> +	memcpy(new_fdt, overlay_fdt, size);
> +
>  	of_fdt_unflatten_tree(new_fdt, NULL, &overlay_root);
>  	if (!overlay_root) {
>  		pr_err("unable to unflatten overlay_fdt\n");
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index eb100627c186..edd6ce807691 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -22,6 +22,7 @@
>  #include <linux/slab.h>
>  #include <linux/device.h>
>  #include <linux/platform_device.h>
> +#include <linux/kernel.h>
>  
>  #include <linux/i2c.h>
>  #include <linux/i2c-mux.h>
> @@ -1415,7 +1416,7 @@ static int __init unittest_data_add(void)
>  	 */
>  	extern uint8_t __dtb_testcases_begin[];
>  	extern uint8_t __dtb_testcases_end[];
> -	const int size = __dtb_testcases_end - __dtb_testcases_begin;
> +	u32 size = __dtb_testcases_end - __dtb_testcases_begin;
>  	int rc;
>  
>  	if (!size) {
> @@ -1425,10 +1426,14 @@ static int __init unittest_data_add(void)
>  	}
>  
>  	/* creating copy */
> -	unittest_data = kmemdup(__dtb_testcases_begin, size, GFP_KERNEL);
> +	size += FDT_ALIGN_SIZE;
> +	unittest_data = kmalloc(size, GFP_KERNEL);
>  	if (!unittest_data)
>  		return -ENOMEM;
>  
> +	unittest_data = PTR_ALIGN(unittest_data, FDT_ALIGN_SIZE);
> +	memcpy(unittest_data, __dtb_testcases_begin, size);
> +
>  	of_fdt_unflatten_tree(unittest_data, NULL, &unittest_data_node);
>  	if (!unittest_data_node) {
>  		pr_warn("%s: No tree to attach; not running tests\n", __func__);
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
  2021-04-07 20:51 [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT frowand.list
  2021-04-07 20:59 ` Frank Rowand
@ 2021-04-07 21:34 ` Rob Herring
  2021-04-08 14:09   ` Frank Rowand
  2021-04-09  9:52   ` Dan Carpenter
  2 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2021-04-07 21:34 UTC (permalink / raw)
  To: Frank Rowand
  Cc: Guenter Roeck, Pantelis Antoniou, devicetree, Geert Uytterhoeven,
	linux-kernel

On Wed, Apr 7, 2021 at 3:51 PM <frowand.list@gmail.com> wrote:
>
> From: Frank Rowand <frank.rowand@sony.com>
>
> The Devicetree standard specifies an 8 byte alignment of the FDT.
> Code in libfdt expects this alignment for an FDT image in memory.
> kmemdup() returns 4 byte alignment on openrisc.  Replace kmemdup()
> with kmalloc(), align pointer, memcpy() to get proper alignment.
>
> The 4 byte alignment exposed a related bug which triggered a crash
> on openrisc with:
> commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9")
> as reported in:
> https://lore.kernel.org/lkml/20210327224116.69309-1-linux@roeck-us.net/
>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
> ---
>  drivers/of/of_private.h | 2 ++
>  drivers/of/overlay.c    | 8 ++++++--
>  drivers/of/unittest.c   | 9 +++++++--
>  3 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index d9e6a324de0a..d717efbd637d 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -8,6 +8,8 @@
>   * Copyright (C) 1996-2005 Paul Mackerras.
>   */
>
> +#define FDT_ALIGN_SIZE 8
> +
>  /**
>   * struct alias_prop - Alias property in 'aliases' node
>   * @link:      List node to link the structure in aliases_lookup list
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 50bbe0edf538..8b40711ed202 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -1014,7 +1014,7 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
>  int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
>                          int *ovcs_id)
>  {
> -       const void *new_fdt;
> +       void *new_fdt;
>         int ret;
>         u32 size;
>         struct device_node *overlay_root;
> @@ -1036,10 +1036,14 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
>          * Must create permanent copy of FDT because of_fdt_unflatten_tree()
>          * will create pointers to the passed in FDT in the unflattened tree.
>          */
> -       new_fdt = kmemdup(overlay_fdt, size, GFP_KERNEL);
> +       size += FDT_ALIGN_SIZE;
> +       new_fdt = kmalloc(size, GFP_KERNEL);
>         if (!new_fdt)
>                 return -ENOMEM;
>
> +       new_fdt = PTR_ALIGN(new_fdt, FDT_ALIGN_SIZE);
> +       memcpy(new_fdt, overlay_fdt, size);
> +
>         of_fdt_unflatten_tree(new_fdt, NULL, &overlay_root);
>         if (!overlay_root) {
>                 pr_err("unable to unflatten overlay_fdt\n");
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index eb100627c186..edd6ce807691 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -22,6 +22,7 @@
>  #include <linux/slab.h>
>  #include <linux/device.h>
>  #include <linux/platform_device.h>
> +#include <linux/kernel.h>
>
>  #include <linux/i2c.h>
>  #include <linux/i2c-mux.h>
> @@ -1415,7 +1416,7 @@ static int __init unittest_data_add(void)
>          */
>         extern uint8_t __dtb_testcases_begin[];
>         extern uint8_t __dtb_testcases_end[];
> -       const int size = __dtb_testcases_end - __dtb_testcases_begin;
> +       u32 size = __dtb_testcases_end - __dtb_testcases_begin;
>         int rc;
>
>         if (!size) {
> @@ -1425,10 +1426,14 @@ static int __init unittest_data_add(void)
>         }
>
>         /* creating copy */
> -       unittest_data = kmemdup(__dtb_testcases_begin, size, GFP_KERNEL);
> +       size += FDT_ALIGN_SIZE;
> +       unittest_data = kmalloc(size, GFP_KERNEL);
>         if (!unittest_data)
>                 return -ENOMEM;
>
> +       unittest_data = PTR_ALIGN(unittest_data, FDT_ALIGN_SIZE);
> +       memcpy(unittest_data, __dtb_testcases_begin, size);
> +
>         of_fdt_unflatten_tree(unittest_data, NULL, &unittest_data_node);
>         if (!unittest_data_node) {
>                 pr_warn("%s: No tree to attach; not running tests\n", __func__);

The next line here is a kfree(unittest_data) which I assume will fail
if the ptr address changed. Same issue in the overlay code.

The error path is easy to fix. Freeing the memory later on, not so
much... One solution is always alloc a power of 2 size, that's
guaranteed to be 'size' aligned:

 * The allocated object address is aligned to at least ARCH_KMALLOC_MINALIGN
 * bytes. For @size of power of two bytes, the alignment is also guaranteed
 * to be at least to the size.

Rob

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
  2021-04-07 20:59 ` Frank Rowand
@ 2021-04-07 22:01   ` Guenter Roeck
  2021-04-08 14:48     ` Frank Rowand
  0 siblings, 1 reply; 11+ messages in thread
From: Guenter Roeck @ 2021-04-07 22:01 UTC (permalink / raw)
  To: Frank Rowand, Rob Herring
  Cc: Pantelis Antoniou, devicetree, Geert Uytterhoeven, linux-kernel

On 4/7/21 1:59 PM, Frank Rowand wrote:
> Hi Guenter,
> 
> On 4/7/21 3:51 PM, frowand.list@gmail.com wrote:
>> From: Frank Rowand <frank.rowand@sony.com>
>>
>> The Devicetree standard specifies an 8 byte alignment of the FDT.
>> Code in libfdt expects this alignment for an FDT image in memory.
>> kmemdup() returns 4 byte alignment on openrisc.  Replace kmemdup()
>> with kmalloc(), align pointer, memcpy() to get proper alignment.
>>
>> The 4 byte alignment exposed a related bug which triggered a crash
>> on openrisc with:
>> commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9")
>> as reported in:
>> https://lore.kernel.org/lkml/20210327224116.69309-1-linux@roeck-us.net/
> 
> Can you please test this patch?
> 

Sure, will do, after you fixed the problem pointed out by Rob.

Sorry, I should have mentioned it - that problem was the reason
why I didn't propose a fix myself.

Guenter

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
  2021-04-07 21:34 ` Rob Herring
@ 2021-04-08 14:09   ` Frank Rowand
  0 siblings, 0 replies; 11+ messages in thread
From: Frank Rowand @ 2021-04-08 14:09 UTC (permalink / raw)
  To: Rob Herring
  Cc: Guenter Roeck, Pantelis Antoniou, devicetree, Geert Uytterhoeven,
	linux-kernel

On 4/7/21 4:34 PM, Rob Herring wrote:
> On Wed, Apr 7, 2021 at 3:51 PM <frowand.list@gmail.com> wrote:
>>
>> From: Frank Rowand <frank.rowand@sony.com>
>>
>> The Devicetree standard specifies an 8 byte alignment of the FDT.
>> Code in libfdt expects this alignment for an FDT image in memory.
>> kmemdup() returns 4 byte alignment on openrisc.  Replace kmemdup()
>> with kmalloc(), align pointer, memcpy() to get proper alignment.
>>
>> The 4 byte alignment exposed a related bug which triggered a crash
>> on openrisc with:
>> commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9")
>> as reported in:
>> https://lore.kernel.org/lkml/20210327224116.69309-1-linux@roeck-us.net/
>>
>> Reported-by: Guenter Roeck <linux@roeck-us.net>
>> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
>> ---
>>  drivers/of/of_private.h | 2 ++
>>  drivers/of/overlay.c    | 8 ++++++--
>>  drivers/of/unittest.c   | 9 +++++++--
>>  3 files changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
>> index d9e6a324de0a..d717efbd637d 100644
>> --- a/drivers/of/of_private.h
>> +++ b/drivers/of/of_private.h
>> @@ -8,6 +8,8 @@
>>   * Copyright (C) 1996-2005 Paul Mackerras.
>>   */
>>
>> +#define FDT_ALIGN_SIZE 8
>> +
>>  /**
>>   * struct alias_prop - Alias property in 'aliases' node
>>   * @link:      List node to link the structure in aliases_lookup list
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index 50bbe0edf538..8b40711ed202 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
>> @@ -1014,7 +1014,7 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
>>  int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
>>                          int *ovcs_id)
>>  {
>> -       const void *new_fdt;
>> +       void *new_fdt;
>>         int ret;
>>         u32 size;
>>         struct device_node *overlay_root;
>> @@ -1036,10 +1036,14 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
>>          * Must create permanent copy of FDT because of_fdt_unflatten_tree()
>>          * will create pointers to the passed in FDT in the unflattened tree.
>>          */
>> -       new_fdt = kmemdup(overlay_fdt, size, GFP_KERNEL);
>> +       size += FDT_ALIGN_SIZE;
>> +       new_fdt = kmalloc(size, GFP_KERNEL);
>>         if (!new_fdt)
>>                 return -ENOMEM;
>>
>> +       new_fdt = PTR_ALIGN(new_fdt, FDT_ALIGN_SIZE);
>> +       memcpy(new_fdt, overlay_fdt, size);
>> +
>>         of_fdt_unflatten_tree(new_fdt, NULL, &overlay_root);
>>         if (!overlay_root) {
>>                 pr_err("unable to unflatten overlay_fdt\n");
>> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
>> index eb100627c186..edd6ce807691 100644
>> --- a/drivers/of/unittest.c
>> +++ b/drivers/of/unittest.c
>> @@ -22,6 +22,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/device.h>
>>  #include <linux/platform_device.h>
>> +#include <linux/kernel.h>
>>
>>  #include <linux/i2c.h>
>>  #include <linux/i2c-mux.h>
>> @@ -1415,7 +1416,7 @@ static int __init unittest_data_add(void)
>>          */
>>         extern uint8_t __dtb_testcases_begin[];
>>         extern uint8_t __dtb_testcases_end[];
>> -       const int size = __dtb_testcases_end - __dtb_testcases_begin;
>> +       u32 size = __dtb_testcases_end - __dtb_testcases_begin;
>>         int rc;
>>
>>         if (!size) {
>> @@ -1425,10 +1426,14 @@ static int __init unittest_data_add(void)
>>         }
>>
>>         /* creating copy */
>> -       unittest_data = kmemdup(__dtb_testcases_begin, size, GFP_KERNEL);
>> +       size += FDT_ALIGN_SIZE;
>> +       unittest_data = kmalloc(size, GFP_KERNEL);
>>         if (!unittest_data)
>>                 return -ENOMEM;
>>
>> +       unittest_data = PTR_ALIGN(unittest_data, FDT_ALIGN_SIZE);
>> +       memcpy(unittest_data, __dtb_testcases_begin, size);
>> +
>>         of_fdt_unflatten_tree(unittest_data, NULL, &unittest_data_node);
>>         if (!unittest_data_node) {
>>                 pr_warn("%s: No tree to attach; not running tests\n", __func__);
> 
> The next line here is a kfree(unittest_data) which I assume will fail
> if the ptr address changed. Same issue in the overlay code.

Thanks for catching this.

> 
> The error path is easy to fix. Freeing the memory later on, not so
> much... 

The overlay subsystem retains ownership of the allocated memory and
responsibility for any subsequent kfree(), so actually not very
difficult.

New version of the patch should be out this morning.

-Frank

> One solution is always alloc a power of 2 size, that's
> guaranteed to be 'size' aligned:
> 
>  * The allocated object address is aligned to at least ARCH_KMALLOC_MINALIGN
>  * bytes. For @size of power of two bytes, the alignment is also guaranteed
>  * to be at least to the size.
> 
> Rob
> .
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
  2021-04-07 22:01   ` Guenter Roeck
@ 2021-04-08 14:48     ` Frank Rowand
  0 siblings, 0 replies; 11+ messages in thread
From: Frank Rowand @ 2021-04-08 14:48 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring
  Cc: Pantelis Antoniou, devicetree, Geert Uytterhoeven, linux-kernel

On 4/7/21 5:01 PM, Guenter Roeck wrote:
> On 4/7/21 1:59 PM, Frank Rowand wrote:
>> Hi Guenter,
>>
>> On 4/7/21 3:51 PM, frowand.list@gmail.com wrote:
>>> From: Frank Rowand <frank.rowand@sony.com>
>>>
>>> The Devicetree standard specifies an 8 byte alignment of the FDT.
>>> Code in libfdt expects this alignment for an FDT image in memory.
>>> kmemdup() returns 4 byte alignment on openrisc.  Replace kmemdup()
>>> with kmalloc(), align pointer, memcpy() to get proper alignment.
>>>
>>> The 4 byte alignment exposed a related bug which triggered a crash
>>> on openrisc with:
>>> commit 79edff12060f ("scripts/dtc: Update to upstream version v1.6.0-51-g183df9e9c2b9")
>>> as reported in:
>>> https://lore.kernel.org/lkml/20210327224116.69309-1-linux@roeck-us.net/
>>
>> Can you please test this patch?
>>
> 
> Sure, will do, after you fixed the problem pointed out by Rob.
> 
> Sorry, I should have mentioned it - that problem was the reason
> why I didn't propose a fix myself.

No problem, I was aware of the issue but then spaced on actually
dealing with it.

- Space Cadet Frank

> 
> Guenter
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [kbuild] Re: [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
  2021-04-07 20:51 [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT frowand.list
  2021-04-07 20:59 ` Frank Rowand
@ 2021-04-09  9:52   ` Dan Carpenter
  2021-04-09  9:52   ` Dan Carpenter
  2 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2021-04-09  9:52 UTC (permalink / raw)
  To: kbuild, frowand.list, Rob Herring, Guenter Roeck
  Cc: lkp, kbuild-all, Pantelis Antoniou, devicetree,
	Geert Uytterhoeven, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 5125 bytes --]

Hi,

url:    https://github.com/0day-ci/linux/commits/frowand-list-gmail-com/of-unittest-overlay-ensure-proper-alignment-of-copied-FDT/20210408-045317 
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git  for-next
config: i386-randconfig-m021-20210407 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/of/overlay.c:1045 of_overlay_fdt_apply() warn: overwrite may leak 'new_fdt'

vim +/new_fdt +1045 drivers/of/overlay.c

39a751a4cb7e47 Frank Rowand      2018-02-12  1015  int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
39a751a4cb7e47 Frank Rowand      2018-02-12  1016  			 int *ovcs_id)
39a751a4cb7e47 Frank Rowand      2018-02-12  1017  {
7a18fbf9013a19 Frank Rowand      2021-04-07  1018  	void *new_fdt;
39a751a4cb7e47 Frank Rowand      2018-02-12  1019  	int ret;
39a751a4cb7e47 Frank Rowand      2018-02-12  1020  	u32 size;
39a751a4cb7e47 Frank Rowand      2018-02-12  1021  	struct device_node *overlay_root;
39a751a4cb7e47 Frank Rowand      2018-02-12  1022  
39a751a4cb7e47 Frank Rowand      2018-02-12  1023  	*ovcs_id = 0;
39a751a4cb7e47 Frank Rowand      2018-02-12  1024  	ret = 0;
39a751a4cb7e47 Frank Rowand      2018-02-12  1025  
39a751a4cb7e47 Frank Rowand      2018-02-12  1026  	if (overlay_fdt_size < sizeof(struct fdt_header) ||
39a751a4cb7e47 Frank Rowand      2018-02-12  1027  	    fdt_check_header(overlay_fdt)) {
39a751a4cb7e47 Frank Rowand      2018-02-12  1028  		pr_err("Invalid overlay_fdt header\n");
39a751a4cb7e47 Frank Rowand      2018-02-12  1029  		return -EINVAL;
39a751a4cb7e47 Frank Rowand      2018-02-12  1030  	}
39a751a4cb7e47 Frank Rowand      2018-02-12  1031  
39a751a4cb7e47 Frank Rowand      2018-02-12  1032  	size = fdt_totalsize(overlay_fdt);
39a751a4cb7e47 Frank Rowand      2018-02-12  1033  	if (overlay_fdt_size < size)
39a751a4cb7e47 Frank Rowand      2018-02-12  1034  		return -EINVAL;
39a751a4cb7e47 Frank Rowand      2018-02-12  1035  
39a751a4cb7e47 Frank Rowand      2018-02-12  1036  	/*
39a751a4cb7e47 Frank Rowand      2018-02-12  1037  	 * Must create permanent copy of FDT because of_fdt_unflatten_tree()
39a751a4cb7e47 Frank Rowand      2018-02-12  1038  	 * will create pointers to the passed in FDT in the unflattened tree.
39a751a4cb7e47 Frank Rowand      2018-02-12  1039  	 */
7a18fbf9013a19 Frank Rowand      2021-04-07  1040  	size += FDT_ALIGN_SIZE;
7a18fbf9013a19 Frank Rowand      2021-04-07  1041  	new_fdt = kmalloc(size, GFP_KERNEL);
39a751a4cb7e47 Frank Rowand      2018-02-12  1042  	if (!new_fdt)
39a751a4cb7e47 Frank Rowand      2018-02-12  1043  		return -ENOMEM;
39a751a4cb7e47 Frank Rowand      2018-02-12  1044  
7a18fbf9013a19 Frank Rowand      2021-04-07 @1045  	new_fdt = PTR_ALIGN(new_fdt, FDT_ALIGN_SIZE);
                                                        ^^^^^^^
We're not freeing the exact same pointer that we allocated.

7a18fbf9013a19 Frank Rowand      2021-04-07  1046  	memcpy(new_fdt, overlay_fdt, size);
7a18fbf9013a19 Frank Rowand      2021-04-07  1047  
39a751a4cb7e47 Frank Rowand      2018-02-12  1048  	of_fdt_unflatten_tree(new_fdt, NULL, &overlay_root);
39a751a4cb7e47 Frank Rowand      2018-02-12  1049  	if (!overlay_root) {
39a751a4cb7e47 Frank Rowand      2018-02-12  1050  		pr_err("unable to unflatten overlay_fdt\n");
39a751a4cb7e47 Frank Rowand      2018-02-12  1051  		ret = -EINVAL;
39a751a4cb7e47 Frank Rowand      2018-02-12  1052  		goto out_free_new_fdt;
39a751a4cb7e47 Frank Rowand      2018-02-12  1053  	}
39a751a4cb7e47 Frank Rowand      2018-02-12  1054  
39a751a4cb7e47 Frank Rowand      2018-02-12  1055  	ret = of_overlay_apply(new_fdt, overlay_root, ovcs_id);
39a751a4cb7e47 Frank Rowand      2018-02-12  1056  	if (ret < 0) {
39a751a4cb7e47 Frank Rowand      2018-02-12  1057  		/*
39a751a4cb7e47 Frank Rowand      2018-02-12  1058  		 * new_fdt and overlay_root now belong to the overlay
39a751a4cb7e47 Frank Rowand      2018-02-12  1059  		 * changeset.
39a751a4cb7e47 Frank Rowand      2018-02-12  1060  		 * overlay changeset code is responsible for freeing them.
39a751a4cb7e47 Frank Rowand      2018-02-12  1061  		 */
39a751a4cb7e47 Frank Rowand      2018-02-12  1062  		goto out;
39a751a4cb7e47 Frank Rowand      2018-02-12  1063  	}
39a751a4cb7e47 Frank Rowand      2018-02-12  1064  
39a751a4cb7e47 Frank Rowand      2018-02-12  1065  	return 0;
39a751a4cb7e47 Frank Rowand      2018-02-12  1066  
39a751a4cb7e47 Frank Rowand      2018-02-12  1067  
39a751a4cb7e47 Frank Rowand      2018-02-12  1068  out_free_new_fdt:
39a751a4cb7e47 Frank Rowand      2018-02-12  1069  	kfree(new_fdt);
39a751a4cb7e47 Frank Rowand      2018-02-12  1070  
39a751a4cb7e47 Frank Rowand      2018-02-12  1071  out:
39a751a4cb7e47 Frank Rowand      2018-02-12  1072  	return ret;
39a751a4cb7e47 Frank Rowand      2018-02-12  1073  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org 

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34729 bytes --]

[-- Attachment #3: Type: text/plain, Size: 149 bytes --]

_______________________________________________
kbuild mailing list -- kbuild@lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
@ 2021-04-09  9:52   ` Dan Carpenter
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2021-04-09  9:52 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 5365 bytes --]

Hi,

url:    https://github.com/0day-ci/linux/commits/frowand-list-gmail-com/of-unittest-overlay-ensure-proper-alignment-of-copied-FDT/20210408-045317 
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git  for-next
config: i386-randconfig-m021-20210407 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/of/overlay.c:1045 of_overlay_fdt_apply() warn: overwrite may leak 'new_fdt'

vim +/new_fdt +1045 drivers/of/overlay.c

39a751a4cb7e47 Frank Rowand      2018-02-12  1015  int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
39a751a4cb7e47 Frank Rowand      2018-02-12  1016  			 int *ovcs_id)
39a751a4cb7e47 Frank Rowand      2018-02-12  1017  {
7a18fbf9013a19 Frank Rowand      2021-04-07  1018  	void *new_fdt;
39a751a4cb7e47 Frank Rowand      2018-02-12  1019  	int ret;
39a751a4cb7e47 Frank Rowand      2018-02-12  1020  	u32 size;
39a751a4cb7e47 Frank Rowand      2018-02-12  1021  	struct device_node *overlay_root;
39a751a4cb7e47 Frank Rowand      2018-02-12  1022  
39a751a4cb7e47 Frank Rowand      2018-02-12  1023  	*ovcs_id = 0;
39a751a4cb7e47 Frank Rowand      2018-02-12  1024  	ret = 0;
39a751a4cb7e47 Frank Rowand      2018-02-12  1025  
39a751a4cb7e47 Frank Rowand      2018-02-12  1026  	if (overlay_fdt_size < sizeof(struct fdt_header) ||
39a751a4cb7e47 Frank Rowand      2018-02-12  1027  	    fdt_check_header(overlay_fdt)) {
39a751a4cb7e47 Frank Rowand      2018-02-12  1028  		pr_err("Invalid overlay_fdt header\n");
39a751a4cb7e47 Frank Rowand      2018-02-12  1029  		return -EINVAL;
39a751a4cb7e47 Frank Rowand      2018-02-12  1030  	}
39a751a4cb7e47 Frank Rowand      2018-02-12  1031  
39a751a4cb7e47 Frank Rowand      2018-02-12  1032  	size = fdt_totalsize(overlay_fdt);
39a751a4cb7e47 Frank Rowand      2018-02-12  1033  	if (overlay_fdt_size < size)
39a751a4cb7e47 Frank Rowand      2018-02-12  1034  		return -EINVAL;
39a751a4cb7e47 Frank Rowand      2018-02-12  1035  
39a751a4cb7e47 Frank Rowand      2018-02-12  1036  	/*
39a751a4cb7e47 Frank Rowand      2018-02-12  1037  	 * Must create permanent copy of FDT because of_fdt_unflatten_tree()
39a751a4cb7e47 Frank Rowand      2018-02-12  1038  	 * will create pointers to the passed in FDT in the unflattened tree.
39a751a4cb7e47 Frank Rowand      2018-02-12  1039  	 */
7a18fbf9013a19 Frank Rowand      2021-04-07  1040  	size += FDT_ALIGN_SIZE;
7a18fbf9013a19 Frank Rowand      2021-04-07  1041  	new_fdt = kmalloc(size, GFP_KERNEL);
39a751a4cb7e47 Frank Rowand      2018-02-12  1042  	if (!new_fdt)
39a751a4cb7e47 Frank Rowand      2018-02-12  1043  		return -ENOMEM;
39a751a4cb7e47 Frank Rowand      2018-02-12  1044  
7a18fbf9013a19 Frank Rowand      2021-04-07 @1045  	new_fdt = PTR_ALIGN(new_fdt, FDT_ALIGN_SIZE);
                                                        ^^^^^^^
We're not freeing the exact same pointer that we allocated.

7a18fbf9013a19 Frank Rowand      2021-04-07  1046  	memcpy(new_fdt, overlay_fdt, size);
7a18fbf9013a19 Frank Rowand      2021-04-07  1047  
39a751a4cb7e47 Frank Rowand      2018-02-12  1048  	of_fdt_unflatten_tree(new_fdt, NULL, &overlay_root);
39a751a4cb7e47 Frank Rowand      2018-02-12  1049  	if (!overlay_root) {
39a751a4cb7e47 Frank Rowand      2018-02-12  1050  		pr_err("unable to unflatten overlay_fdt\n");
39a751a4cb7e47 Frank Rowand      2018-02-12  1051  		ret = -EINVAL;
39a751a4cb7e47 Frank Rowand      2018-02-12  1052  		goto out_free_new_fdt;
39a751a4cb7e47 Frank Rowand      2018-02-12  1053  	}
39a751a4cb7e47 Frank Rowand      2018-02-12  1054  
39a751a4cb7e47 Frank Rowand      2018-02-12  1055  	ret = of_overlay_apply(new_fdt, overlay_root, ovcs_id);
39a751a4cb7e47 Frank Rowand      2018-02-12  1056  	if (ret < 0) {
39a751a4cb7e47 Frank Rowand      2018-02-12  1057  		/*
39a751a4cb7e47 Frank Rowand      2018-02-12  1058  		 * new_fdt and overlay_root now belong to the overlay
39a751a4cb7e47 Frank Rowand      2018-02-12  1059  		 * changeset.
39a751a4cb7e47 Frank Rowand      2018-02-12  1060  		 * overlay changeset code is responsible for freeing them.
39a751a4cb7e47 Frank Rowand      2018-02-12  1061  		 */
39a751a4cb7e47 Frank Rowand      2018-02-12  1062  		goto out;
39a751a4cb7e47 Frank Rowand      2018-02-12  1063  	}
39a751a4cb7e47 Frank Rowand      2018-02-12  1064  
39a751a4cb7e47 Frank Rowand      2018-02-12  1065  	return 0;
39a751a4cb7e47 Frank Rowand      2018-02-12  1066  
39a751a4cb7e47 Frank Rowand      2018-02-12  1067  
39a751a4cb7e47 Frank Rowand      2018-02-12  1068  out_free_new_fdt:
39a751a4cb7e47 Frank Rowand      2018-02-12  1069  	kfree(new_fdt);
39a751a4cb7e47 Frank Rowand      2018-02-12  1070  
39a751a4cb7e47 Frank Rowand      2018-02-12  1071  out:
39a751a4cb7e47 Frank Rowand      2018-02-12  1072  	return ret;
39a751a4cb7e47 Frank Rowand      2018-02-12  1073  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org 

_______________________________________________
kbuild mailing list -- kbuild(a)lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 34729 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [kbuild] Re: [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
@ 2021-04-09  9:52   ` Dan Carpenter
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2021-04-09  9:52 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 5365 bytes --]

Hi,

url:    https://github.com/0day-ci/linux/commits/frowand-list-gmail-com/of-unittest-overlay-ensure-proper-alignment-of-copied-FDT/20210408-045317 
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git  for-next
config: i386-randconfig-m021-20210407 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/of/overlay.c:1045 of_overlay_fdt_apply() warn: overwrite may leak 'new_fdt'

vim +/new_fdt +1045 drivers/of/overlay.c

39a751a4cb7e47 Frank Rowand      2018-02-12  1015  int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
39a751a4cb7e47 Frank Rowand      2018-02-12  1016  			 int *ovcs_id)
39a751a4cb7e47 Frank Rowand      2018-02-12  1017  {
7a18fbf9013a19 Frank Rowand      2021-04-07  1018  	void *new_fdt;
39a751a4cb7e47 Frank Rowand      2018-02-12  1019  	int ret;
39a751a4cb7e47 Frank Rowand      2018-02-12  1020  	u32 size;
39a751a4cb7e47 Frank Rowand      2018-02-12  1021  	struct device_node *overlay_root;
39a751a4cb7e47 Frank Rowand      2018-02-12  1022  
39a751a4cb7e47 Frank Rowand      2018-02-12  1023  	*ovcs_id = 0;
39a751a4cb7e47 Frank Rowand      2018-02-12  1024  	ret = 0;
39a751a4cb7e47 Frank Rowand      2018-02-12  1025  
39a751a4cb7e47 Frank Rowand      2018-02-12  1026  	if (overlay_fdt_size < sizeof(struct fdt_header) ||
39a751a4cb7e47 Frank Rowand      2018-02-12  1027  	    fdt_check_header(overlay_fdt)) {
39a751a4cb7e47 Frank Rowand      2018-02-12  1028  		pr_err("Invalid overlay_fdt header\n");
39a751a4cb7e47 Frank Rowand      2018-02-12  1029  		return -EINVAL;
39a751a4cb7e47 Frank Rowand      2018-02-12  1030  	}
39a751a4cb7e47 Frank Rowand      2018-02-12  1031  
39a751a4cb7e47 Frank Rowand      2018-02-12  1032  	size = fdt_totalsize(overlay_fdt);
39a751a4cb7e47 Frank Rowand      2018-02-12  1033  	if (overlay_fdt_size < size)
39a751a4cb7e47 Frank Rowand      2018-02-12  1034  		return -EINVAL;
39a751a4cb7e47 Frank Rowand      2018-02-12  1035  
39a751a4cb7e47 Frank Rowand      2018-02-12  1036  	/*
39a751a4cb7e47 Frank Rowand      2018-02-12  1037  	 * Must create permanent copy of FDT because of_fdt_unflatten_tree()
39a751a4cb7e47 Frank Rowand      2018-02-12  1038  	 * will create pointers to the passed in FDT in the unflattened tree.
39a751a4cb7e47 Frank Rowand      2018-02-12  1039  	 */
7a18fbf9013a19 Frank Rowand      2021-04-07  1040  	size += FDT_ALIGN_SIZE;
7a18fbf9013a19 Frank Rowand      2021-04-07  1041  	new_fdt = kmalloc(size, GFP_KERNEL);
39a751a4cb7e47 Frank Rowand      2018-02-12  1042  	if (!new_fdt)
39a751a4cb7e47 Frank Rowand      2018-02-12  1043  		return -ENOMEM;
39a751a4cb7e47 Frank Rowand      2018-02-12  1044  
7a18fbf9013a19 Frank Rowand      2021-04-07 @1045  	new_fdt = PTR_ALIGN(new_fdt, FDT_ALIGN_SIZE);
                                                        ^^^^^^^
We're not freeing the exact same pointer that we allocated.

7a18fbf9013a19 Frank Rowand      2021-04-07  1046  	memcpy(new_fdt, overlay_fdt, size);
7a18fbf9013a19 Frank Rowand      2021-04-07  1047  
39a751a4cb7e47 Frank Rowand      2018-02-12  1048  	of_fdt_unflatten_tree(new_fdt, NULL, &overlay_root);
39a751a4cb7e47 Frank Rowand      2018-02-12  1049  	if (!overlay_root) {
39a751a4cb7e47 Frank Rowand      2018-02-12  1050  		pr_err("unable to unflatten overlay_fdt\n");
39a751a4cb7e47 Frank Rowand      2018-02-12  1051  		ret = -EINVAL;
39a751a4cb7e47 Frank Rowand      2018-02-12  1052  		goto out_free_new_fdt;
39a751a4cb7e47 Frank Rowand      2018-02-12  1053  	}
39a751a4cb7e47 Frank Rowand      2018-02-12  1054  
39a751a4cb7e47 Frank Rowand      2018-02-12  1055  	ret = of_overlay_apply(new_fdt, overlay_root, ovcs_id);
39a751a4cb7e47 Frank Rowand      2018-02-12  1056  	if (ret < 0) {
39a751a4cb7e47 Frank Rowand      2018-02-12  1057  		/*
39a751a4cb7e47 Frank Rowand      2018-02-12  1058  		 * new_fdt and overlay_root now belong to the overlay
39a751a4cb7e47 Frank Rowand      2018-02-12  1059  		 * changeset.
39a751a4cb7e47 Frank Rowand      2018-02-12  1060  		 * overlay changeset code is responsible for freeing them.
39a751a4cb7e47 Frank Rowand      2018-02-12  1061  		 */
39a751a4cb7e47 Frank Rowand      2018-02-12  1062  		goto out;
39a751a4cb7e47 Frank Rowand      2018-02-12  1063  	}
39a751a4cb7e47 Frank Rowand      2018-02-12  1064  
39a751a4cb7e47 Frank Rowand      2018-02-12  1065  	return 0;
39a751a4cb7e47 Frank Rowand      2018-02-12  1066  
39a751a4cb7e47 Frank Rowand      2018-02-12  1067  
39a751a4cb7e47 Frank Rowand      2018-02-12  1068  out_free_new_fdt:
39a751a4cb7e47 Frank Rowand      2018-02-12  1069  	kfree(new_fdt);
39a751a4cb7e47 Frank Rowand      2018-02-12  1070  
39a751a4cb7e47 Frank Rowand      2018-02-12  1071  out:
39a751a4cb7e47 Frank Rowand      2018-02-12  1072  	return ret;
39a751a4cb7e47 Frank Rowand      2018-02-12  1073  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org 

_______________________________________________
kbuild mailing list -- kbuild(a)lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 34729 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
@ 2021-04-08  5:38 kernel test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2021-04-08  5:38 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 10418 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210407205110.2173976-1-frowand.list@gmail.com>
References: <20210407205110.2173976-1-frowand.list@gmail.com>
TO: frowand.list(a)gmail.com
TO: Rob Herring <robh+dt@kernel.org>
TO: Guenter Roeck <linux@roeck-us.net>
CC: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
CC: devicetree(a)vger.kernel.org
CC: Geert Uytterhoeven <geert+renesas@glider.be>
CC: linux-kernel(a)vger.kernel.org

Hi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on v5.12-rc6 next-20210407]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/frowand-list-gmail-com/of-unittest-overlay-ensure-proper-alignment-of-copied-FDT/20210408-045317
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
:::::: branch date: 9 hours ago
:::::: commit date: 9 hours ago
config: microblaze-randconfig-m031-20210407 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/of/unittest.c:1434 unittest_data_add() warn: overwrite may leak 'unittest_data'

Old smatch warnings:
drivers/of/unittest.c:2964 unittest_unflatten_overlay_base() warn: should '1 << (((__builtin_constant_p((((((7 * 4) + 4) + 4) + 4)) - 1)) ?((((((((7 * 4) + 4) + 4) + 4)) - 1) < 2) ?0:63 - __builtin_clzll((((((7 * 4) + 4) + 4) + 4)) - 1)):((4 <= 4)) ?__ilog2_u32((((((7 * 4) + 4) + 4) + 4)) - 1):__ilog2_u64((((((7 * 4) + 4) + 4) + 4)) - 1)) + 1)' be a 64 bit type?
drivers/of/unittest.c:3127 of_unittest_overlay_high_level() error: potentially dereferencing uninitialized 'overlay_base_symbols'.

vim +/unittest_data +1434 drivers/of/unittest.c

ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1404  
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1405  /**
9697a5595ece52 drivers/of/unittest.c Wang Long          2015-03-11  1406   *	unittest_data_add - Reads, copies data from
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1407   *	linked tree and attaches it to the live tree
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1408   */
9697a5595ece52 drivers/of/unittest.c Wang Long          2015-03-11  1409  static int __init unittest_data_add(void)
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1410  {
9697a5595ece52 drivers/of/unittest.c Wang Long          2015-03-11  1411  	void *unittest_data;
9697a5595ece52 drivers/of/unittest.c Wang Long          2015-03-11  1412  	struct device_node *unittest_data_node, *np;
c8547119ce54ef drivers/of/unittest.c Frank Rowand       2015-03-14  1413  	/*
c8547119ce54ef drivers/of/unittest.c Frank Rowand       2015-03-14  1414  	 * __dtb_testcases_begin[] and __dtb_testcases_end[] are magically
c8547119ce54ef drivers/of/unittest.c Frank Rowand       2015-03-14  1415  	 * created by cmd_dt_S_dtb in scripts/Makefile.lib
c8547119ce54ef drivers/of/unittest.c Frank Rowand       2015-03-14  1416  	 */
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1417  	extern uint8_t __dtb_testcases_begin[];
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1418  	extern uint8_t __dtb_testcases_end[];
7a18fbf9013a19 drivers/of/unittest.c Frank Rowand       2021-04-07  1419  	u32 size = __dtb_testcases_end - __dtb_testcases_begin;
2eb46da2a760e5 drivers/of/selftest.c Grant Likely       2014-10-02  1420  	int rc;
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1421  
b951f9dc7f25fc drivers/of/selftest.c Gaurav Minocha     2014-07-26  1422  	if (!size) {
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1423  		pr_warn("%s: No testcase data to attach; not running tests\n",
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1424  			__func__);
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1425  		return -ENODATA;
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1426  	}
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1427  
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1428  	/* creating copy */
7a18fbf9013a19 drivers/of/unittest.c Frank Rowand       2021-04-07  1429  	size += FDT_ALIGN_SIZE;
7a18fbf9013a19 drivers/of/unittest.c Frank Rowand       2021-04-07  1430  	unittest_data = kmalloc(size, GFP_KERNEL);
2a656cb5a4a347 drivers/of/unittest.c Geert Uytterhoeven 2019-05-02  1431  	if (!unittest_data)
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1432  		return -ENOMEM;
2a656cb5a4a347 drivers/of/unittest.c Geert Uytterhoeven 2019-05-02  1433  
7a18fbf9013a19 drivers/of/unittest.c Frank Rowand       2021-04-07 @1434  	unittest_data = PTR_ALIGN(unittest_data, FDT_ALIGN_SIZE);
7a18fbf9013a19 drivers/of/unittest.c Frank Rowand       2021-04-07  1435  	memcpy(unittest_data, __dtb_testcases_begin, size);
7a18fbf9013a19 drivers/of/unittest.c Frank Rowand       2021-04-07  1436  
c4263233f30e72 drivers/of/unittest.c Gavin Shan         2016-05-03  1437  	of_fdt_unflatten_tree(unittest_data, NULL, &unittest_data_node);
9697a5595ece52 drivers/of/unittest.c Wang Long          2015-03-11  1438  	if (!unittest_data_node) {
b951f9dc7f25fc drivers/of/selftest.c Gaurav Minocha     2014-07-26  1439  		pr_warn("%s: No tree to attach; not running tests\n", __func__);
e13de8fe0d6a51 drivers/of/unittest.c Navid Emamdoost    2019-10-04  1440  		kfree(unittest_data);
b951f9dc7f25fc drivers/of/selftest.c Gaurav Minocha     2014-07-26  1441  		return -ENODATA;
b951f9dc7f25fc drivers/of/selftest.c Gaurav Minocha     2014-07-26  1442  	}
f948d6d8b792bb drivers/of/unittest.c Frank Rowand       2017-10-17  1443  
f948d6d8b792bb drivers/of/unittest.c Frank Rowand       2017-10-17  1444  	/*
39a751a4cb7e47 drivers/of/unittest.c Frank Rowand       2018-02-12  1445  	 * This lock normally encloses of_resolve_phandles()
f948d6d8b792bb drivers/of/unittest.c Frank Rowand       2017-10-17  1446  	 */
f948d6d8b792bb drivers/of/unittest.c Frank Rowand       2017-10-17  1447  	of_overlay_mutex_lock();
f948d6d8b792bb drivers/of/unittest.c Frank Rowand       2017-10-17  1448  
9697a5595ece52 drivers/of/unittest.c Wang Long          2015-03-11  1449  	rc = of_resolve_phandles(unittest_data_node);
2eb46da2a760e5 drivers/of/selftest.c Grant Likely       2014-10-02  1450  	if (rc) {
2eb46da2a760e5 drivers/of/selftest.c Grant Likely       2014-10-02  1451  		pr_err("%s: Failed to resolve phandles (rc=%i)\n", __func__, rc);
f948d6d8b792bb drivers/of/unittest.c Frank Rowand       2017-10-17  1452  		of_overlay_mutex_unlock();
2eb46da2a760e5 drivers/of/selftest.c Grant Likely       2014-10-02  1453  		return -EINVAL;
2eb46da2a760e5 drivers/of/selftest.c Grant Likely       2014-10-02  1454  	}
b951f9dc7f25fc drivers/of/selftest.c Gaurav Minocha     2014-07-26  1455  
5063e25a302e6a drivers/of/selftest.c Grant Likely       2014-10-03  1456  	if (!of_root) {
9697a5595ece52 drivers/of/unittest.c Wang Long          2015-03-11  1457  		of_root = unittest_data_node;
b951f9dc7f25fc drivers/of/selftest.c Gaurav Minocha     2014-07-26  1458  		for_each_of_allnodes(np)
b951f9dc7f25fc drivers/of/selftest.c Gaurav Minocha     2014-07-26  1459  			__of_attach_node_sysfs(np);
b951f9dc7f25fc drivers/of/selftest.c Gaurav Minocha     2014-07-26  1460  		of_aliases = of_find_node_by_path("/aliases");
b951f9dc7f25fc drivers/of/selftest.c Gaurav Minocha     2014-07-26  1461  		of_chosen = of_find_node_by_path("/chosen");
f948d6d8b792bb drivers/of/unittest.c Frank Rowand       2017-10-17  1462  		of_overlay_mutex_unlock();
b951f9dc7f25fc drivers/of/selftest.c Gaurav Minocha     2014-07-26  1463  		return 0;
b951f9dc7f25fc drivers/of/selftest.c Gaurav Minocha     2014-07-26  1464  	}
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1465  
0ac1743979408a drivers/of/unittest.c Frank Rowand       2020-02-20  1466  	EXPECT_BEGIN(KERN_INFO,
0ac1743979408a drivers/of/unittest.c Frank Rowand       2020-02-20  1467  		     "Duplicate name in testcase-data, renamed to \"duplicate-name#1\"");
0ac1743979408a drivers/of/unittest.c Frank Rowand       2020-02-20  1468  
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1469  	/* attach the sub-tree to live tree */
9697a5595ece52 drivers/of/unittest.c Wang Long          2015-03-11  1470  	np = unittest_data_node->child;
5063e25a302e6a drivers/of/selftest.c Grant Likely       2014-10-03  1471  	while (np) {
5063e25a302e6a drivers/of/selftest.c Grant Likely       2014-10-03  1472  		struct device_node *next = np->sibling;
3db316d00bfa60 drivers/of/unittest.c Frank Rowand       2015-03-14  1473  
5063e25a302e6a drivers/of/selftest.c Grant Likely       2014-10-03  1474  		np->parent = of_root;
5063e25a302e6a drivers/of/selftest.c Grant Likely       2014-10-03  1475  		attach_node_and_children(np);
5063e25a302e6a drivers/of/selftest.c Grant Likely       2014-10-03  1476  		np = next;
5063e25a302e6a drivers/of/selftest.c Grant Likely       2014-10-03  1477  	}
f948d6d8b792bb drivers/of/unittest.c Frank Rowand       2017-10-17  1478  
0ac1743979408a drivers/of/unittest.c Frank Rowand       2020-02-20  1479  	EXPECT_END(KERN_INFO,
0ac1743979408a drivers/of/unittest.c Frank Rowand       2020-02-20  1480  		   "Duplicate name in testcase-data, renamed to \"duplicate-name#1\"");
0ac1743979408a drivers/of/unittest.c Frank Rowand       2020-02-20  1481  
f948d6d8b792bb drivers/of/unittest.c Frank Rowand       2017-10-17  1482  	of_overlay_mutex_unlock();
f948d6d8b792bb drivers/of/unittest.c Frank Rowand       2017-10-17  1483  
5063e25a302e6a drivers/of/selftest.c Grant Likely       2014-10-03  1484  	return 0;
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1485  }
ae9304c9d31117 drivers/of/selftest.c Gaurav Minocha     2014-07-16  1486  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 31530 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT
@ 2021-04-08  0:44 kernel test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2021-04-08  0:44 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 6126 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210407205110.2173976-1-frowand.list@gmail.com>
References: <20210407205110.2173976-1-frowand.list@gmail.com>
TO: frowand.list(a)gmail.com
TO: Rob Herring <robh+dt@kernel.org>
TO: Guenter Roeck <linux@roeck-us.net>
CC: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
CC: devicetree(a)vger.kernel.org
CC: Geert Uytterhoeven <geert+renesas@glider.be>
CC: linux-kernel(a)vger.kernel.org

Hi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on v5.12-rc6 next-20210407]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/frowand-list-gmail-com/of-unittest-overlay-ensure-proper-alignment-of-copied-FDT/20210408-045317
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
:::::: branch date: 4 hours ago
:::::: commit date: 4 hours ago
config: i386-randconfig-m021-20210407 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/of/overlay.c:1045 of_overlay_fdt_apply() warn: overwrite may leak 'new_fdt'

vim +/new_fdt +1045 drivers/of/overlay.c

39a751a4cb7e47 Frank Rowand      2018-02-12  1014  
39a751a4cb7e47 Frank Rowand      2018-02-12  1015  int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
39a751a4cb7e47 Frank Rowand      2018-02-12  1016  			 int *ovcs_id)
39a751a4cb7e47 Frank Rowand      2018-02-12  1017  {
7a18fbf9013a19 Frank Rowand      2021-04-07  1018  	void *new_fdt;
39a751a4cb7e47 Frank Rowand      2018-02-12  1019  	int ret;
39a751a4cb7e47 Frank Rowand      2018-02-12  1020  	u32 size;
39a751a4cb7e47 Frank Rowand      2018-02-12  1021  	struct device_node *overlay_root;
39a751a4cb7e47 Frank Rowand      2018-02-12  1022  
39a751a4cb7e47 Frank Rowand      2018-02-12  1023  	*ovcs_id = 0;
39a751a4cb7e47 Frank Rowand      2018-02-12  1024  	ret = 0;
39a751a4cb7e47 Frank Rowand      2018-02-12  1025  
39a751a4cb7e47 Frank Rowand      2018-02-12  1026  	if (overlay_fdt_size < sizeof(struct fdt_header) ||
39a751a4cb7e47 Frank Rowand      2018-02-12  1027  	    fdt_check_header(overlay_fdt)) {
39a751a4cb7e47 Frank Rowand      2018-02-12  1028  		pr_err("Invalid overlay_fdt header\n");
39a751a4cb7e47 Frank Rowand      2018-02-12  1029  		return -EINVAL;
39a751a4cb7e47 Frank Rowand      2018-02-12  1030  	}
39a751a4cb7e47 Frank Rowand      2018-02-12  1031  
39a751a4cb7e47 Frank Rowand      2018-02-12  1032  	size = fdt_totalsize(overlay_fdt);
39a751a4cb7e47 Frank Rowand      2018-02-12  1033  	if (overlay_fdt_size < size)
39a751a4cb7e47 Frank Rowand      2018-02-12  1034  		return -EINVAL;
39a751a4cb7e47 Frank Rowand      2018-02-12  1035  
39a751a4cb7e47 Frank Rowand      2018-02-12  1036  	/*
39a751a4cb7e47 Frank Rowand      2018-02-12  1037  	 * Must create permanent copy of FDT because of_fdt_unflatten_tree()
39a751a4cb7e47 Frank Rowand      2018-02-12  1038  	 * will create pointers to the passed in FDT in the unflattened tree.
39a751a4cb7e47 Frank Rowand      2018-02-12  1039  	 */
7a18fbf9013a19 Frank Rowand      2021-04-07  1040  	size += FDT_ALIGN_SIZE;
7a18fbf9013a19 Frank Rowand      2021-04-07  1041  	new_fdt = kmalloc(size, GFP_KERNEL);
39a751a4cb7e47 Frank Rowand      2018-02-12  1042  	if (!new_fdt)
39a751a4cb7e47 Frank Rowand      2018-02-12  1043  		return -ENOMEM;
39a751a4cb7e47 Frank Rowand      2018-02-12  1044  
7a18fbf9013a19 Frank Rowand      2021-04-07 @1045  	new_fdt = PTR_ALIGN(new_fdt, FDT_ALIGN_SIZE);
7a18fbf9013a19 Frank Rowand      2021-04-07  1046  	memcpy(new_fdt, overlay_fdt, size);
7a18fbf9013a19 Frank Rowand      2021-04-07  1047  
39a751a4cb7e47 Frank Rowand      2018-02-12  1048  	of_fdt_unflatten_tree(new_fdt, NULL, &overlay_root);
39a751a4cb7e47 Frank Rowand      2018-02-12  1049  	if (!overlay_root) {
39a751a4cb7e47 Frank Rowand      2018-02-12  1050  		pr_err("unable to unflatten overlay_fdt\n");
39a751a4cb7e47 Frank Rowand      2018-02-12  1051  		ret = -EINVAL;
39a751a4cb7e47 Frank Rowand      2018-02-12  1052  		goto out_free_new_fdt;
39a751a4cb7e47 Frank Rowand      2018-02-12  1053  	}
39a751a4cb7e47 Frank Rowand      2018-02-12  1054  
39a751a4cb7e47 Frank Rowand      2018-02-12  1055  	ret = of_overlay_apply(new_fdt, overlay_root, ovcs_id);
39a751a4cb7e47 Frank Rowand      2018-02-12  1056  	if (ret < 0) {
39a751a4cb7e47 Frank Rowand      2018-02-12  1057  		/*
39a751a4cb7e47 Frank Rowand      2018-02-12  1058  		 * new_fdt and overlay_root now belong to the overlay
39a751a4cb7e47 Frank Rowand      2018-02-12  1059  		 * changeset.
39a751a4cb7e47 Frank Rowand      2018-02-12  1060  		 * overlay changeset code is responsible for freeing them.
39a751a4cb7e47 Frank Rowand      2018-02-12  1061  		 */
39a751a4cb7e47 Frank Rowand      2018-02-12  1062  		goto out;
39a751a4cb7e47 Frank Rowand      2018-02-12  1063  	}
39a751a4cb7e47 Frank Rowand      2018-02-12  1064  
39a751a4cb7e47 Frank Rowand      2018-02-12  1065  	return 0;
39a751a4cb7e47 Frank Rowand      2018-02-12  1066  
39a751a4cb7e47 Frank Rowand      2018-02-12  1067  
39a751a4cb7e47 Frank Rowand      2018-02-12  1068  out_free_new_fdt:
39a751a4cb7e47 Frank Rowand      2018-02-12  1069  	kfree(new_fdt);
39a751a4cb7e47 Frank Rowand      2018-02-12  1070  
39a751a4cb7e47 Frank Rowand      2018-02-12  1071  out:
39a751a4cb7e47 Frank Rowand      2018-02-12  1072  	return ret;
39a751a4cb7e47 Frank Rowand      2018-02-12  1073  }
39a751a4cb7e47 Frank Rowand      2018-02-12  1074  EXPORT_SYMBOL_GPL(of_overlay_fdt_apply);
7518b5890d8ac3 Pantelis Antoniou 2014-10-28  1075  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 34729 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-04-09  9:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-07 20:51 [PATCH 1/1] of: unittest: overlay: ensure proper alignment of copied FDT frowand.list
2021-04-07 20:59 ` Frank Rowand
2021-04-07 22:01   ` Guenter Roeck
2021-04-08 14:48     ` Frank Rowand
2021-04-07 21:34 ` Rob Herring
2021-04-08 14:09   ` Frank Rowand
2021-04-09  9:52 ` [kbuild] " Dan Carpenter
2021-04-09  9:52   ` Dan Carpenter
2021-04-09  9:52   ` Dan Carpenter
2021-04-08  0:44 kernel test robot
2021-04-08  5:38 kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.