Skip to contentSkip to conversation

net: thunderx: nicvf_main: Fix potential NULL pointer dereferences

PATCH In review Gustavo A. R. Silva 3 participants 4 messages Apr 3, 2018 – Apr 5, 2018

Subsystems (3)
Networking Network drivers Arm/cavium thunder network driver
lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH v2 1/1 net: thunderx: nicvf_main: Fix potential NULL pointer dereferences

Add null check on kmalloc() return value in order to prevent a null pointer dereference.

Addresses-Coverity-ID: 1467429 ("Dereference null return value")

Fixes 37c3347eb247 ("net: thunderx: add ndo_set_rx_mode callback implementation for VF")
Signed-off-by Gustavo A. R. Silva <[email protected]>

---
Changes in v2:
- Add a null check on a second kmalloc a few lines below. Thanks to Eric Dumazet for pointing this out.

 drivers/net/ethernet/cavium/thunder/nicvf_main.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/net/ethernet/cavium/thunder/nicvf_main.c +4 -0
index 1e9a31f..f7b5ca5 100644
@@ -1999,10 +1999,14 @@ static void nicvf_set_rx_mode(struct net_device *netdev)
 				struct xcast_addr *xaddr;
 
 				mc_list = kmalloc(sizeof(*mc_list), GFP_ATOMIC);
+				if (unlikely(!mc_list))
+					return;
 				INIT_LIST_HEAD(&mc_list->list);
 				netdev_hw_addr_list_for_each(ha, &netdev->mc) {
 					xaddr = kmalloc(sizeof(*xaddr),
 							GFP_ATOMIC);
+					if (unlikely(!xaddr))
+						return;
 					xaddr->addr =
 						ether_addr_to_u64(ha->addr);
 					list_add_tail(&xaddr->list,
Signature · 1 line
2.7.4
#1 lore raw
Quoted 20 lines · Gustavo A. R. Silva

Add null check on kmalloc() return value in order to prevent a null pointer dereference.

Addresses-Coverity-ID: 1467429 ("Dereference null return value")

Fixes 37c3347eb247 ("net: thunderx: add ndo_set_rx_mode callback implementation for VF")
Signed-off-by Gustavo A. R. Silva <[email protected]>

---
Changes in v2:
- Add a null check on a second kmalloc a few lines below. Thanks to Eric Dumazet for pointing this out.

 drivers/net/ethernet/cavium/thunder/nicvf_main.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/net/ethernet/cavium/thunder/nicvf_main.c +0 -0
index 1e9a31f..f7b5ca5 100644
@@ -1999,10 +1999,14 @@ static void nicvf_set_rx_mode(struct net_device *netdev)
 				struct xcast_addr *xaddr;

drivers/net/ethernet/cavium/thunder/nicvf_main.c static void nicvf_set_rx_mode(struct net_device *netdev) · line 2009

Quoted 10 lines
drivers/net/ethernet/cavium/thunder/nicvf_main.c +4 -0
 
 				mc_list = kmalloc(sizeof(*mc_list), GFP_ATOMIC);
+				if (unlikely(!mc_list))
+					return;
 				INIT_LIST_HEAD(&mc_list->list);
 				netdev_hw_addr_list_for_each(ha, &netdev->mc) {
 					xaddr = kmalloc(sizeof(*xaddr),
 							GFP_ATOMIC);
+					if (unlikely(!xaddr))
+						return;

So now you leak memory :/

If you fix bugs, please do not add new ones.

Quoted 4 lines
drivers/net/ethernet/cavium/thunder/nicvf_main.c +0 -0
 					xaddr->addr =
 						ether_addr_to_u64(ha->addr);
 					list_add_tail(&xaddr->list,
author
#2 lore raw
Quoted 24 lines · Eric Dumazet
Quoted 21 lines · Gustavo A. R. Silva

Add null check on kmalloc() return value in order to prevent a null pointer dereference.

Addresses-Coverity-ID: 1467429 ("Dereference null return value")

Fixes 37c3347eb247 ("net: thunderx: add ndo_set_rx_mode callback implementation for VF")
Signed-off-by Gustavo A. R. Silva <[email protected]>

---
Changes in v2:
- Add a null check on a second kmalloc a few lines below. Thanks to Eric Dumazet for pointing this out.

  drivers/net/ethernet/cavium/thunder/nicvf_main.c | 4 ++++
  1 file changed, 4 insertions(+)
drivers/net/ethernet/cavium/thunder/nicvf_main.c +0 -0
index 1e9a31f..f7b5ca5 100644
@@ -1999,10 +1999,14 @@ static void nicvf_set_rx_mode(struct net_device *netdev)
  				struct xcast_addr *xaddr;
Quoted 12 lines

drivers/net/ethernet/cavium/thunder/nicvf_main.c static void nicvf_set_rx_mode(struct net_device *netdev) · line 2009

Quoted 9 lines
drivers/net/ethernet/cavium/thunder/nicvf_main.c +4 -0
  				mc_list = kmalloc(sizeof(*mc_list), GFP_ATOMIC);
+				if (unlikely(!mc_list))
+					return;
  				INIT_LIST_HEAD(&mc_list->list);
  				netdev_hw_addr_list_for_each(ha, &netdev->mc) {
  					xaddr = kmalloc(sizeof(*xaddr),
  							GFP_ATOMIC);
+					if (unlikely(!xaddr))
+						return;

So now you leak memory :/

Oh god. I'm sorry.

I had problems with this file since the beginning.

I'll send v3 shortly.

Thanks
--
Gustavo

#3 lore raw
In reply to Gustavo A. R. Silva

Hi guys,

Please give me couple days to workout solution for this. I'll post patch for this as soon as I done with my testing.

WBR,
Vadim

Quoted 33 lines · Gustavo A. R. Silva

Add null check on kmalloc() return value in order to prevent a null pointer dereference.

Addresses-Coverity-ID: 1467429 ("Dereference null return value")

Fixes 37c3347eb247 ("net: thunderx: add ndo_set_rx_mode callback implementation for VF")
Signed-off-by Gustavo A. R. Silva <[email protected]>

---
Changes in v2:
- Add a null check on a second kmalloc a few lines below. Thanks to Eric Dumazet for pointing this out.

 drivers/net/ethernet/cavium/thunder/nicvf_main.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/net/ethernet/cavium/thunder/nicvf_main.c +4 -0
index 1e9a31f..f7b5ca5 100644
@@ -1999,10 +1999,14 @@ static void nicvf_set_rx_mode(struct net_device *netdev)
 				struct xcast_addr *xaddr;
 
 				mc_list = kmalloc(sizeof(*mc_list), GFP_ATOMIC);
+				if (unlikely(!mc_list))
+					return;
 				INIT_LIST_HEAD(&mc_list->list);
 				netdev_hw_addr_list_for_each(ha, &netdev->mc) {
 					xaddr = kmalloc(sizeof(*xaddr),
 							GFP_ATOMIC);
+					if (unlikely(!xaddr))
+						return;
 					xaddr->addr =
 						ether_addr_to_u64(ha->addr);
 					list_add_tail(&xaddr->list,