All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] clk: zynqmp: Misc fixes
@ 2021-08-18  6:59 Shubhrajyoti Datta
  2021-08-18  6:59 ` [PATCH 1/3] clk: zynqmp: Check the return type Shubhrajyoti Datta
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Shubhrajyoti Datta @ 2021-08-18  6:59 UTC (permalink / raw)
  To: linux-clk; +Cc: sboyd, mturquette, michal.simek, git-dev, Shubhrajyoti Datta


The series fixes
- Memory leak 
- A stack-out-of-bounds in strncpy
- Adds a check for return type

Ian Nam (1):
  clk: zynqmp: Fix stack-out-of-bounds in strncpy`

Shubhrajyoti Datta (2):
  clk: zynqmp: Check the return type
  clk: zynqmp: Fix a memory leak

 drivers/clk/zynqmp/clk-mux-zynqmp.c |  2 +-
 drivers/clk/zynqmp/clkc.c           | 11 ++++++++---
 2 files changed, 9 insertions(+), 4 deletions(-)

-- 
2.17.1


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

* [PATCH 1/3] clk: zynqmp: Check the return type
  2021-08-18  6:59 [PATCH 0/3] clk: zynqmp: Misc fixes Shubhrajyoti Datta
@ 2021-08-18  6:59 ` Shubhrajyoti Datta
  2021-08-29  4:26   ` Stephen Boyd
  2021-08-29  4:26   ` Stephen Boyd
  2021-08-18  6:59 ` [PATCH 2/3] clk: zynqmp: Fix a memory leak Shubhrajyoti Datta
  2021-08-18  6:59 ` [PATCH 3/3] clk: zynqmp: Fix stack-out-of-bounds in strncpy` Shubhrajyoti Datta
  2 siblings, 2 replies; 8+ messages in thread
From: Shubhrajyoti Datta @ 2021-08-18  6:59 UTC (permalink / raw)
  To: linux-clk; +Cc: sboyd, mturquette, michal.simek, git-dev, Shubhrajyoti Datta

Currently the return value of of_clk_add_hw_provider is ignored.
lets check and return value.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/clk/zynqmp/clkc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c
index 871184e406e1..eb25303eefed 100644
--- a/drivers/clk/zynqmp/clkc.c
+++ b/drivers/clk/zynqmp/clkc.c
@@ -762,9 +762,7 @@ static int zynqmp_clk_setup(struct device_node *np)
 	zynqmp_register_clocks(np);
 
 	zynqmp_data->num = clock_max_idx;
-	of_clk_add_hw_provider(np, of_clk_hw_onecell_get, zynqmp_data);
-
-	return 0;
+	return of_clk_add_hw_provider(np, of_clk_hw_onecell_get, zynqmp_data);
 }
 
 static int zynqmp_clock_probe(struct platform_device *pdev)
-- 
2.17.1


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

* [PATCH 2/3] clk: zynqmp: Fix a memory leak
  2021-08-18  6:59 [PATCH 0/3] clk: zynqmp: Misc fixes Shubhrajyoti Datta
  2021-08-18  6:59 ` [PATCH 1/3] clk: zynqmp: Check the return type Shubhrajyoti Datta
@ 2021-08-18  6:59 ` Shubhrajyoti Datta
  2021-08-29  4:26   ` Stephen Boyd
  2021-08-18  6:59 ` [PATCH 3/3] clk: zynqmp: Fix stack-out-of-bounds in strncpy` Shubhrajyoti Datta
  2 siblings, 1 reply; 8+ messages in thread
From: Shubhrajyoti Datta @ 2021-08-18  6:59 UTC (permalink / raw)
  To: linux-clk; +Cc: sboyd, mturquette, michal.simek, git-dev, Shubhrajyoti Datta

Fix a memory leak of mux.

Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/clk/zynqmp/clk-mux-zynqmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/zynqmp/clk-mux-zynqmp.c b/drivers/clk/zynqmp/clk-mux-zynqmp.c
index 157d4a960bf4..17afce594f28 100644
--- a/drivers/clk/zynqmp/clk-mux-zynqmp.c
+++ b/drivers/clk/zynqmp/clk-mux-zynqmp.c
@@ -159,7 +159,7 @@ struct clk_hw *zynqmp_clk_register_mux(const char *name, u32 clk_id,
 	hw = &mux->hw;
 	ret = clk_hw_register(NULL, hw);
 	if (ret) {
-		kfree(hw);
+		kfree(mux);
 		hw = ERR_PTR(ret);
 	}
 
-- 
2.17.1


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

* [PATCH 3/3] clk: zynqmp: Fix stack-out-of-bounds in strncpy`
  2021-08-18  6:59 [PATCH 0/3] clk: zynqmp: Misc fixes Shubhrajyoti Datta
  2021-08-18  6:59 ` [PATCH 1/3] clk: zynqmp: Check the return type Shubhrajyoti Datta
  2021-08-18  6:59 ` [PATCH 2/3] clk: zynqmp: Fix a memory leak Shubhrajyoti Datta
@ 2021-08-18  6:59 ` Shubhrajyoti Datta
  2021-08-29  4:25   ` Stephen Boyd
  2 siblings, 1 reply; 8+ messages in thread
From: Shubhrajyoti Datta @ 2021-08-18  6:59 UTC (permalink / raw)
  To: linux-clk
  Cc: sboyd, mturquette, michal.simek, git-dev, Ian Nam, Rajan Vaja,
	Shubhrajyoti Datta

From: Ian Nam <young.kwan.nam@xilinx.com>

"BUG: KASAN: stack-out-of-bounds in strncpy+0x30/0x68"

Linux-ATF interface is using 16 bytes of SMC payload. In case clock name is
longer than 15 bytes, string terminated NULL character will not be received
by Linux. Add explicit NULL character at last byte to fix issues when clock
name is longer.

This fixes below bug reported by KASAN:

[    7.522474] ==================================================================
[    7.529795] BUG: KASAN: stack-out-of-bounds in strncpy+0x30/0x68
[    7.535871] Read of size 1 at addr ffff0008c89a7410 by task swapper/0/1
[    7.542557]
[    7.544065] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.4.0-00396-g81ef9e7-dirty #3
[    7.551809] Hardware name: Xilinx Versal vck190 Eval board revA (QSPI) (DT)
[    7.558847] Call trace:
[    7.561321]  dump_backtrace+0x0/0x1e8
[    7.565023]  show_stack+0x14/0x20
[    7.568374]  dump_stack+0xd4/0x108
[    7.571817]  print_address_description.isra.0+0xbc/0x37c
[    7.577189]  __kasan_report+0x144/0x198
[    7.581068]  kasan_report+0xc/0x18
[    7.584507]  __asan_load1+0x5c/0x68
[    7.588032]  strncpy+0x30/0x68
[    7.591120]  zynqmp_clock_probe+0x238/0x7b8
[    7.595350]  platform_drv_probe+0x6c/0xc8
[    7.599405]  really_probe+0x14c/0x418
[    7.603108]  driver_probe_device+0x74/0x130
[    7.607339]  __device_attach_driver+0xc4/0xe8
[    7.611744]  bus_for_each_drv+0xec/0x150
[    7.615711]  __device_attach+0x160/0x1d8
[    7.619678]  device_initial_probe+0x10/0x18
[    7.623907]  bus_probe_device+0xe0/0xf0
[    7.627785]  device_add+0x528/0x950
[    7.631312]  of_device_add+0x5c/0x80
[    7.634926]  of_platform_device_create_pdata+0x120/0x168
[    7.640299]  of_platform_bus_create+0x244/0x4e0
[    7.644880]  of_platform_populate+0x50/0xe8
[    7.649110]  zynqmp_firmware_probe+0x370/0x3a8
[    7.653602]  platform_drv_probe+0x6c/0xc8
[    7.657656]  really_probe+0x14c/0x418
[    7.661359]  driver_probe_device+0x74/0x130
[    7.665589]  device_driver_attach+0x94/0xa0
[    7.669820]  __driver_attach+0x70/0x108
[    7.673698]  bus_for_each_dev+0xe4/0x158
[    7.677664]  driver_attach+0x30/0x40
[    7.681278]  bus_add_driver+0x21c/0x2b8
[    7.685157]  driver_register+0xbc/0x1d0
[    7.689035]  __platform_driver_register+0x7c/0x88
[    7.693793]  zynqmp_firmware_driver_init+0x1c/0x24
[    7.698637]  do_one_initcall+0xa4/0x234
[    7.702518]  kernel_init_freeable+0x1b0/0x24c
[    7.706924]  kernel_init+0x10/0x110
[    7.710450]  ret_from_fork+0x10/0x18
[    7.714058]
[    7.715559] The buggy address belongs to the page:
[    7.720405] page:ffff0008f9be1c88 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0
[    7.728772] raw: 0008d00000000000 ffff0008f9be1c90 ffff0008f9be1c90 0000000000000000
[    7.736606] raw: 0000000000000000 0000000000000000 00000000ffffffff
[    7.742942] page dumped because: kasan: bad access detected
[    7.748572]
[    7.750076] addr ffff0008c89a7410 is located in stack of task swapper/0/1 at offset 112 in frame:
[    7.759052]  zynqmp_clock_probe+0x0/0x7b8
[    7.763103]
[    7.764604] this frame has 3 objects:
[    7.768306]  [32, 44) 'response'
[    7.768312]  [64, 80) 'ret_payload'
[    7.771573]  [96, 112) 'name'
[    7.775095]
[    7.779585] Memory state around the buggy address:
[    7.784430]  ffff0008c89a7300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[    7.791735]  ffff0008c89a7380: 00 00 00 00 f1 f1 f1 f1 00 04 f2 f2 00 00 f2 f2
[    7.799040] >ffff0008c89a7400: 00 00 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
[    7.806342]                          ^
[    7.810132]  ffff0008c89a7480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[    7.817437]  ffff0008c89a7500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[    7.824738] ==================================================================

Signed-off-by: Ian Nam <young.kwan.nam@xilinx.com>
Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
---
 drivers/clk/zynqmp/clkc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c
index eb25303eefed..2c9da6623b84 100644
--- a/drivers/clk/zynqmp/clkc.c
+++ b/drivers/clk/zynqmp/clkc.c
@@ -710,6 +710,13 @@ static void zynqmp_get_clock_info(void)
 				  FIELD_PREP(CLK_ATTR_NODE_INDEX, i);
 
 		zynqmp_pm_clock_get_name(clock[i].clk_id, &name);
+
+		/*
+		 * Terminate with NULL character in case name provided by firmware
+		 * is longer and truncated due to size limit.
+		 */
+		name.name[sizeof(name.name) - 1] = '\0';
+
 		if (!strcmp(name.name, RESERVED_CLK_NAME))
 			continue;
 		strncpy(clock[i].clk_name, name.name, MAX_NAME_LEN);
-- 
2.17.1


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

* Re: [PATCH 3/3] clk: zynqmp: Fix stack-out-of-bounds in strncpy`
  2021-08-18  6:59 ` [PATCH 3/3] clk: zynqmp: Fix stack-out-of-bounds in strncpy` Shubhrajyoti Datta
@ 2021-08-29  4:25   ` Stephen Boyd
  0 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2021-08-29  4:25 UTC (permalink / raw)
  To: Shubhrajyoti Datta, linux-clk
  Cc: mturquette, michal.simek, git-dev, Ian Nam, Rajan Vaja,
	Shubhrajyoti Datta

Quoting Shubhrajyoti Datta (2021-08-17 23:59:29)
> From: Ian Nam <young.kwan.nam@xilinx.com>
> 
> "BUG: KASAN: stack-out-of-bounds in strncpy+0x30/0x68"
> 
> Linux-ATF interface is using 16 bytes of SMC payload. In case clock name is
> longer than 15 bytes, string terminated NULL character will not be received
> by Linux. Add explicit NULL character at last byte to fix issues when clock
> name is longer.
> 
> This fixes below bug reported by KASAN:
> 
> [    7.522474] ==================================================================
> [    7.529795] BUG: KASAN: stack-out-of-bounds in strncpy+0x30/0x68
> [    7.535871] Read of size 1 at addr ffff0008c89a7410 by task swapper/0/1
> [    7.542557]
> [    7.544065] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.4.0-00396-g81ef9e7-dirty #3
> [    7.551809] Hardware name: Xilinx Versal vck190 Eval board revA (QSPI) (DT)
> [    7.558847] Call trace:
> [    7.561321]  dump_backtrace+0x0/0x1e8
> [    7.565023]  show_stack+0x14/0x20
> [    7.568374]  dump_stack+0xd4/0x108
> [    7.571817]  print_address_description.isra.0+0xbc/0x37c
> [    7.577189]  __kasan_report+0x144/0x198
> [    7.581068]  kasan_report+0xc/0x18
> [    7.584507]  __asan_load1+0x5c/0x68
> [    7.588032]  strncpy+0x30/0x68
> [    7.591120]  zynqmp_clock_probe+0x238/0x7b8
> [    7.595350]  platform_drv_probe+0x6c/0xc8
> [    7.599405]  really_probe+0x14c/0x418
> [    7.603108]  driver_probe_device+0x74/0x130
> [    7.607339]  __device_attach_driver+0xc4/0xe8
> [    7.611744]  bus_for_each_drv+0xec/0x150
> [    7.615711]  __device_attach+0x160/0x1d8
> [    7.619678]  device_initial_probe+0x10/0x18
> [    7.623907]  bus_probe_device+0xe0/0xf0
> [    7.627785]  device_add+0x528/0x950
> [    7.631312]  of_device_add+0x5c/0x80
> [    7.634926]  of_platform_device_create_pdata+0x120/0x168
> [    7.640299]  of_platform_bus_create+0x244/0x4e0
> [    7.644880]  of_platform_populate+0x50/0xe8
> [    7.649110]  zynqmp_firmware_probe+0x370/0x3a8
> [    7.653602]  platform_drv_probe+0x6c/0xc8
> [    7.657656]  really_probe+0x14c/0x418
> [    7.661359]  driver_probe_device+0x74/0x130
> [    7.665589]  device_driver_attach+0x94/0xa0
> [    7.669820]  __driver_attach+0x70/0x108
> [    7.673698]  bus_for_each_dev+0xe4/0x158
> [    7.677664]  driver_attach+0x30/0x40
> [    7.681278]  bus_add_driver+0x21c/0x2b8
> [    7.685157]  driver_register+0xbc/0x1d0
> [    7.689035]  __platform_driver_register+0x7c/0x88
> [    7.693793]  zynqmp_firmware_driver_init+0x1c/0x24
> [    7.698637]  do_one_initcall+0xa4/0x234
> [    7.702518]  kernel_init_freeable+0x1b0/0x24c
> [    7.706924]  kernel_init+0x10/0x110
> [    7.710450]  ret_from_fork+0x10/0x18
> [    7.714058]
> [    7.715559] The buggy address belongs to the page:
> [    7.720405] page:ffff0008f9be1c88 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0
> [    7.728772] raw: 0008d00000000000 ffff0008f9be1c90 ffff0008f9be1c90 0000000000000000
> [    7.736606] raw: 0000000000000000 0000000000000000 00000000ffffffff
> [    7.742942] page dumped because: kasan: bad access detected
> [    7.748572]
> [    7.750076] addr ffff0008c89a7410 is located in stack of task swapper/0/1 at offset 112 in frame:
> [    7.759052]  zynqmp_clock_probe+0x0/0x7b8
> [    7.763103]
> [    7.764604] this frame has 3 objects:
> [    7.768306]  [32, 44) 'response'
> [    7.768312]  [64, 80) 'ret_payload'
> [    7.771573]  [96, 112) 'name'
> [    7.775095]
> [    7.779585] Memory state around the buggy address:
> [    7.784430]  ffff0008c89a7300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [    7.791735]  ffff0008c89a7380: 00 00 00 00 f1 f1 f1 f1 00 04 f2 f2 00 00 f2 f2
> [    7.799040] >ffff0008c89a7400: 00 00 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
> [    7.806342]                          ^
> [    7.810132]  ffff0008c89a7480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [    7.817437]  ffff0008c89a7500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [    7.824738] ==================================================================
> 
> Signed-off-by: Ian Nam <young.kwan.nam@xilinx.com>
> Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---
>  drivers/clk/zynqmp/clkc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c
> index eb25303eefed..2c9da6623b84 100644
> --- a/drivers/clk/zynqmp/clkc.c
> +++ b/drivers/clk/zynqmp/clkc.c
> @@ -710,6 +710,13 @@ static void zynqmp_get_clock_info(void)
>                                   FIELD_PREP(CLK_ATTR_NODE_INDEX, i);
>  
>                 zynqmp_pm_clock_get_name(clock[i].clk_id, &name);
> +
> +               /*
> +                * Terminate with NULL character in case name provided by firmware
> +                * is longer and truncated due to size limit.
> +                */
> +               name.name[sizeof(name.name) - 1] = '\0';
> +
>                 if (!strcmp(name.name, RESERVED_CLK_NAME))
>                         continue;
>                 strncpy(clock[i].clk_name, name.name, MAX_NAME_LEN);

Why not use strscpy()?

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

* Re: [PATCH 1/3] clk: zynqmp: Check the return type
  2021-08-18  6:59 ` [PATCH 1/3] clk: zynqmp: Check the return type Shubhrajyoti Datta
@ 2021-08-29  4:26   ` Stephen Boyd
  2021-08-29  4:26   ` Stephen Boyd
  1 sibling, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2021-08-29  4:26 UTC (permalink / raw)
  To: Shubhrajyoti Datta, linux-clk
  Cc: mturquette, michal.simek, git-dev, Shubhrajyoti Datta

Quoting Shubhrajyoti Datta (2021-08-17 23:59:27)
> Currently the return value of of_clk_add_hw_provider is ignored.
> lets check and return value.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---

Applied to clk-next

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

* Re: [PATCH 1/3] clk: zynqmp: Check the return type
  2021-08-18  6:59 ` [PATCH 1/3] clk: zynqmp: Check the return type Shubhrajyoti Datta
  2021-08-29  4:26   ` Stephen Boyd
@ 2021-08-29  4:26   ` Stephen Boyd
  1 sibling, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2021-08-29  4:26 UTC (permalink / raw)
  To: Shubhrajyoti Datta, linux-clk
  Cc: mturquette, michal.simek, git-dev, Shubhrajyoti Datta

Quoting Shubhrajyoti Datta (2021-08-17 23:59:27)
> Currently the return value of of_clk_add_hw_provider is ignored.
> lets check and return value.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---

Applied to clk-next

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

* Re: [PATCH 2/3] clk: zynqmp: Fix a memory leak
  2021-08-18  6:59 ` [PATCH 2/3] clk: zynqmp: Fix a memory leak Shubhrajyoti Datta
@ 2021-08-29  4:26   ` Stephen Boyd
  0 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2021-08-29  4:26 UTC (permalink / raw)
  To: Shubhrajyoti Datta, linux-clk
  Cc: mturquette, michal.simek, git-dev, Shubhrajyoti Datta

Quoting Shubhrajyoti Datta (2021-08-17 23:59:28)
> Fix a memory leak of mux.
> 
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
> ---

Applied to clk-next

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

end of thread, other threads:[~2021-08-29  4:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-18  6:59 [PATCH 0/3] clk: zynqmp: Misc fixes Shubhrajyoti Datta
2021-08-18  6:59 ` [PATCH 1/3] clk: zynqmp: Check the return type Shubhrajyoti Datta
2021-08-29  4:26   ` Stephen Boyd
2021-08-29  4:26   ` Stephen Boyd
2021-08-18  6:59 ` [PATCH 2/3] clk: zynqmp: Fix a memory leak Shubhrajyoti Datta
2021-08-29  4:26   ` Stephen Boyd
2021-08-18  6:59 ` [PATCH 3/3] clk: zynqmp: Fix stack-out-of-bounds in strncpy` Shubhrajyoti Datta
2021-08-29  4:25   ` Stephen Boyd

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.