XDP fixes for socionext driver

PATCH In review Lorenzo Bianconi 4 participants 8 messages Jan 25, 2020 – Jan 27, 2020

Subsystems (3)

Networking Network drivers Socionext (SNI) netsec network driver

lore

Share & tools

lore

Lorenzo Bianconi

author

2020-01-25 11:50 UTC #0 lore raw

cover XDP fixes for socionext driver

Fix possible user-after-in XDP rx path
Fix rx statistics accounting if no bpf program is attached

Lorenzo Bianconi (2):
net: socionext: fix possible user-after-free in netsec_process_rx
net: socionext: fix xdp_result initialization in netsec_process_rx

 drivers/net/ethernet/socionext/netsec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Signature · 1 line

2.21.1

Lorenzo Bianconi

author

2020-01-25 11:50 UTC #1 lore raw

PATCH 1/2 net: socionext: fix possible user-after-free in netsec_process_rx

Fix possible use-after-free in in netsec_process_rx that can occurs if the first packet is sent to the normal networking stack and the following one is dropped by the bpf program attached to the xdp hook. Fix the issue defining the skb pointer in the 'budget' loop

Fixes ba2b232108d3c ("net: netsec: add XDP support")

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 drivers/net/ethernet/socionext/netsec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ethernet/socionext/netsec.c +1 -1

index 869a498e3b5e..0e12a9856aea 100644
@@ -929,7 +929,6 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
 	struct netsec_rx_pkt_info rx_info;
 	enum dma_data_direction dma_dir;
 	struct bpf_prog *xdp_prog;
-	struct sk_buff *skb = NULL;
 	u16 xdp_xmit = 0;
 	u32 xdp_act = 0;
 	int done = 0;

@@ -943,6 +942,7 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
 		struct netsec_de *de = dring->vaddr + (DESC_SZ * idx);
 		struct netsec_desc *desc = &dring->desc[idx];
 		struct page *page = virt_to_page(desc->addr);
+		struct sk_buff *skb = NULL;
 		u32 xdp_result = XDP_PASS;
 		u16 pkt_len, desc_len;
 		dma_addr_t dma_handle;

Signature · 1 line

2.21.1

Jesper Dangaard Brouer

Acked-by on v1 1/2

2020-01-25 12:49 UTC #2 lore raw

Re: [PATCH net 1/2] net: socionext: fix possible user-after-free in netsec_process_rx

Quoted 8 lines · On Sat, 25 Jan 2020 12:48:50 +0100 Lorenzo Bianconi <[email protected]> wrote:

Fix possible use-after-free in in netsec_process_rx that can occurs if the first packet is sent to the normal networking stack and the following one is dropped by the bpf program attached to the xdp hook. Fix the issue defining the skb pointer in the 'budget' loop
Fixes ba2b232108d3c ("net: netsec: add XDP support")
Signed-off-by Lorenzo Bianconi <[email protected]>
---

Acked-by Jesper Dangaard Brouer <[email protected]>

Quoted 16 lines

 drivers/net/ethernet/socionext/netsec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ethernet/socionext/netsec.c +0 -1

index 869a498e3b5e..0e12a9856aea 100644
@@ -929,7 +929,6 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
 	struct netsec_rx_pkt_info rx_info;
 	enum dma_data_direction dma_dir;
 	struct bpf_prog *xdp_prog;
-	struct sk_buff *skb = NULL;
 	u16 xdp_xmit = 0;
 	u32 xdp_act = 0;
 	int done = 0;

drivers/net/ethernet/socionext/netsec.c static int netsec_process_rx(struct netsec_priv *priv, int budget) · line 948

Quoted 7 lines

drivers/net/ethernet/socionext/netsec.c +1 -0

@@ -943,6 +942,7 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
 		struct netsec_de *de = dring->vaddr + (DESC_SZ * idx);
 		struct netsec_desc *desc = &dring->desc[idx];
 		struct page *page = virt_to_page(desc->addr);
+		struct sk_buff *skb = NULL;
 		u32 xdp_result = XDP_PASS;
 		u16 pkt_len, desc_len;
 		dma_addr_t dma_handle;

Yes, this is needed in case an xdp_prog jumps to label 'next:', skipping the skb = build_skb() assignment.

Signature · 4 lines

Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Ilias Apalodimas

maintainer Acked-by on v1 1/2

2020-01-27 08:01 UTC #3 lore raw

Re: [PATCH net 1/2] net: socionext: fix possible user-after-free in netsec_process_rx

In reply to Lorenzo Bianconi

Quoted 34 lines · Lorenzo Bianconi

Fix possible use-after-free in in netsec_process_rx that can occurs if the first packet is sent to the normal networking stack and the following one is dropped by the bpf program attached to the xdp hook. Fix the issue defining the skb pointer in the 'budget' loop
Fixes ba2b232108d3c ("net: netsec: add XDP support")
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 drivers/net/ethernet/socionext/netsec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/socionext/netsec.c +1 -1
index 869a498e3b5e..0e12a9856aea 100644
@@ -929,7 +929,6 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
 	struct netsec_rx_pkt_info rx_info;
 	enum dma_data_direction dma_dir;
 	struct bpf_prog *xdp_prog;
-	struct sk_buff *skb = NULL;
 	u16 xdp_xmit = 0;
 	u32 xdp_act = 0;
 	int done = 0;
@@ -943,6 +942,7 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
 		struct netsec_de *de = dring->vaddr + (DESC_SZ * idx);
 		struct netsec_desc *desc = &dring->desc[idx];
 		struct page *page = virt_to_page(desc->addr);
+		struct sk_buff *skb = NULL;
 		u32 xdp_result = XDP_PASS;
 		u16 pkt_len, desc_len;
 		dma_addr_t dma_handle;
Signature · 1 line
2.21.1

Acked-by Ilias Apalodimas <[email protected]>

Lorenzo Bianconi

author

2020-01-25 11:50 UTC #4 lore raw

PATCH 2/2 net: socionext: fix xdp_result initialization in netsec_process_rx

In reply to Lorenzo Bianconi

Fix xdp_result initialization in netsec_process_rx in order to not increase rx counters if there is no bpf program attached to the xdp hook and napi_gro_receive returns GRO_DROP

Fixes ba2b232108d3c ("net: netsec: add XDP support")

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 drivers/net/ethernet/socionext/netsec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ethernet/socionext/netsec.c +1 -1

index 0e12a9856aea..56c0e643f430 100644
@@ -942,8 +942,8 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
 		struct netsec_de *de = dring->vaddr + (DESC_SZ * idx);
 		struct netsec_desc *desc = &dring->desc[idx];
 		struct page *page = virt_to_page(desc->addr);
+		u32 xdp_result = NETSEC_XDP_PASS;
 		struct sk_buff *skb = NULL;
-		u32 xdp_result = XDP_PASS;
 		u16 pkt_len, desc_len;
 		dma_addr_t dma_handle;
 		struct xdp_buff xdp;

Signature · 1 line

2.21.1

Jesper Dangaard Brouer

Acked-by on v1 2/2

2020-01-25 12:51 UTC #5 lore raw

Re: [PATCH net 2/2] net: socionext: fix xdp_result initialization in netsec_process_rx

Quoted 7 lines · On Sat, 25 Jan 2020 12:48:51 +0100 Lorenzo Bianconi <[email protected]> wrote:

Fix xdp_result initialization in netsec_process_rx in order to not increase rx counters if there is no bpf program attached to the xdp hook and napi_gro_receive returns GRO_DROP
Fixes ba2b232108d3c ("net: netsec: add XDP support")
Signed-off-by Lorenzo Bianconi <[email protected]>
---

Acked-by Jesper Dangaard Brouer <[email protected]>

drivers/net/ethernet/socionext/netsec.c static int netsec_process_rx(struct netsec_priv *priv, int budget) · line 945

Quoted 12 lines

 drivers/net/ethernet/socionext/netsec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/ethernet/socionext/netsec.c +1 -0

index 0e12a9856aea..56c0e643f430 100644
@@ -942,8 +942,8 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
 		struct netsec_de *de = dring->vaddr + (DESC_SZ * idx);
 		struct netsec_desc *desc = &dring->desc[idx];
 		struct page *page = virt_to_page(desc->addr);
+		u32 xdp_result = NETSEC_XDP_PASS;

As a help to reviewers value is:
#define NETSEC_XDP_PASS 0

drivers/net/ethernet/socionext/netsec.c static int netsec_process_rx(struct netsec_priv *priv, int budget) · line 946

Quoted 2 lines

drivers/net/ethernet/socionext/netsec.c +0 -1
 		struct sk_buff *skb = NULL;
-		u32 xdp_result = XDP_PASS;

XDP_PASS == 2

Quoted 3 lines

drivers/net/ethernet/socionext/netsec.c +0 -0

 		u16 pkt_len, desc_len;
 		dma_addr_t dma_handle;
 		struct xdp_buff xdp;

Signature · 4 lines

Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Ilias Apalodimas

maintainer Acked-by on v1 2/2

2020-01-27 08:01 UTC #6 lore raw

Re: [PATCH net 2/2] net: socionext: fix xdp_result initialization in netsec_process_rx

In reply to Lorenzo Bianconi

Quoted 27 lines · Lorenzo Bianconi

Fix xdp_result initialization in netsec_process_rx in order to not increase rx counters if there is no bpf program attached to the xdp hook and napi_gro_receive returns GRO_DROP
Fixes ba2b232108d3c ("net: netsec: add XDP support")
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 drivers/net/ethernet/socionext/netsec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/socionext/netsec.c +1 -1
index 0e12a9856aea..56c0e643f430 100644
@@ -942,8 +942,8 @@ static int netsec_process_rx(struct netsec_priv *priv, int budget)
 		struct netsec_de *de = dring->vaddr + (DESC_SZ * idx);
 		struct netsec_desc *desc = &dring->desc[idx];
 		struct page *page = virt_to_page(desc->addr);
+		u32 xdp_result = NETSEC_XDP_PASS;
 		struct sk_buff *skb = NULL;
-		u32 xdp_result = XDP_PASS;
 		u16 pkt_len, desc_len;
 		dma_addr_t dma_handle;
 		struct xdp_buff xdp;
Signature · 1 line
2.21.1

Acked-by Ilias Apalodimas <[email protected]>

David Miller

maintainer

2020-01-27 10:08 UTC #7 lore raw

In reply to Lorenzo Bianconi

From: Lorenzo Bianconi <[email protected]>
Date: Sat, 25 Jan 2020 12:48:49 +0100

Quoted 2 lines

Fix possible user-after-in XDP rx path
Fix rx statistics accounting if no bpf program is attached

Series applied and queued up for -stable, thank you.