Re: [PATCH v2] thunderbolt: Fix to check for kmemdup failure

[Mika Westerberg]

On Wed, Mar 20, 2019 at 10:57:54AM -0500, Aditya Pakki wrote:
> Memory allocated via kmemdup might fail and return a NULL pointer.
> This patch adds a check on the return value of kmemdup and passes the
> error upstream.
> 
> Signed-off-by: Aditya Pakki 

Applied, thanks!

Re: [PATCH v2] thunderbolt: Fix to check for kmemdup failure

[Mukesh Ojha]

On 3/20/2019 9:27 PM, Aditya Pakki wrote:

Memory allocated via kmemdup might fail and return a NULL pointer.
This patch adds a check on the return value of kmemdup and passes the
error upstream.

Signed-off-by: Aditya Pakki 

---
v1: Missed check on tb_sw_read, suggested by Mukesh
---
  drivers/thunderbolt/switch.c | 22 --
  1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index cd96994dc094..504365d46827 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -1294,13 +1294,14 @@ int tb_switch_configure(struct tb_switch *sw)
return tb_plug_events_active(sw, true);
  }
  
-static void tb_switch_set_uuid(struct tb_switch *sw)

+static int tb_switch_set_uuid(struct tb_switch *sw)
  {
u32 uuid[4];
-   int cap;
+   int cap, ret;
  
+	ret = 0;

if (sw->uuid)
-   return;
+   return ret;
  y
/*
 * The newer controllers include fused UUID as part of link
@@ -1308,7 +1309,9 @@ static void tb_switch_set_uuid(struct tb_switch *sw)
 */
cap = tb_switch_find_vse_cap(sw, TB_VSE_CAP_LINK_CONTROLLER);
if (cap > 0) {
-   tb_sw_read(sw, uuid, TB_CFG_SWITCH, cap + 3, 4);
+   ret = tb_sw_read(sw, uuid, TB_CFG_SWITCH, cap + 3, 4);
+   if (ret)
+   return ret;
} else {
/*
 * ICM generates UUID based on UID and fills the upper
@@ -1323,6 +1326,9 @@ static void tb_switch_set_uuid(struct tb_switch *sw)
}
  
  	sw->uuid = kmemdup(uuid, sizeof(uuid), GFP_KERNEL);

+   if (!sw->uuid)
+   ret = -ENOMEM;
+   return ret;
  }



Thanks for doing the change.

Reviewed-by: Mukesh Ojha 


-Mukesh



  
  static int tb_switch_add_dma_port(struct tb_switch *sw)

@@ -1372,7 +1378,9 @@ static int tb_switch_add_dma_port(struct tb_switch *sw)
  
  	if (status) {

tb_sw_info(sw, "switch flash authentication failed\n");
-   tb_switch_set_uuid(sw);
+   ret = tb_switch_set_uuid(sw);
+   if (ret)
+   return ret;
nvm_set_auth_status(sw, status);
}
  
@@ -1422,7 +1430,9 @@ int tb_switch_add(struct tb_switch *sw)

}
tb_sw_dbg(sw, "uid: %#llx\n", sw->uid);
  
-		tb_switch_set_uuid(sw);

+   ret = tb_switch_set_uuid(sw);
+   if (ret)
+   return ret;
  
  		for (i = 0; i <= sw->config.max_port_number; i++) {

if (sw->ports[i].disabled) {

[PATCH v2] thunderbolt: Fix to check for kmemdup failure

[Aditya Pakki]

Memory allocated via kmemdup might fail and return a NULL pointer.
This patch adds a check on the return value of kmemdup and passes the
error upstream.

Signed-off-by: Aditya Pakki 

---
v1: Missed check on tb_sw_read, suggested by Mukesh
---
 drivers/thunderbolt/switch.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index cd96994dc094..504365d46827 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -1294,13 +1294,14 @@ int tb_switch_configure(struct tb_switch *sw)
return tb_plug_events_active(sw, true);
 }
 
-static void tb_switch_set_uuid(struct tb_switch *sw)
+static int tb_switch_set_uuid(struct tb_switch *sw)
 {
u32 uuid[4];
-   int cap;
+   int cap, ret;
 
+   ret = 0;
if (sw->uuid)
-   return;
+   return ret;
 
/*
 * The newer controllers include fused UUID as part of link
@@ -1308,7 +1309,9 @@ static void tb_switch_set_uuid(struct tb_switch *sw)
 */
cap = tb_switch_find_vse_cap(sw, TB_VSE_CAP_LINK_CONTROLLER);
if (cap > 0) {
-   tb_sw_read(sw, uuid, TB_CFG_SWITCH, cap + 3, 4);
+   ret = tb_sw_read(sw, uuid, TB_CFG_SWITCH, cap + 3, 4);
+   if (ret)
+   return ret;
} else {
/*
 * ICM generates UUID based on UID and fills the upper
@@ -1323,6 +1326,9 @@ static void tb_switch_set_uuid(struct tb_switch *sw)
}
 
sw->uuid = kmemdup(uuid, sizeof(uuid), GFP_KERNEL);
+   if (!sw->uuid)
+   ret = -ENOMEM;
+   return ret;
 }
 
 static int tb_switch_add_dma_port(struct tb_switch *sw)
@@ -1372,7 +1378,9 @@ static int tb_switch_add_dma_port(struct tb_switch *sw)
 
if (status) {
tb_sw_info(sw, "switch flash authentication failed\n");
-   tb_switch_set_uuid(sw);
+   ret = tb_switch_set_uuid(sw);
+   if (ret)
+   return ret;
nvm_set_auth_status(sw, status);
}
 
@@ -1422,7 +1430,9 @@ int tb_switch_add(struct tb_switch *sw)
}
tb_sw_dbg(sw, "uid: %#llx\n", sw->uid);
 
-   tb_switch_set_uuid(sw);
+   ret = tb_switch_set_uuid(sw);
+   if (ret)
+   return ret;
 
for (i = 0; i <= sw->config.max_port_number; i++) {
if (sw->ports[i].disabled) {
-- 
2.17.1