[PATCH] stmmac: Fix type of local variable in stmmac_xmit

Subsystems: networking drivers, stmmac ethernet driver, the rest

STALE3840d

2 messages, 2 authors, 2016-03-04 · open the first message on its own page

[PATCH] stmmac: Fix type of local variable in stmmac_xmit

From: Andrzej Hajda <hidden>
Date: 2016-03-04 07:19:13

Variable entry holds result of jumbo_frm callback. It can be negative,
so the variable should be signed. The patch changes also type of related
first_entry variable to make code compact and coherent.

The problem has been detected using patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci.

Signed-off-by: Andrzej Hajda <redacted>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4c5ce98..cd31a3c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1955,7 +1955,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 	unsigned int nopaged_len = skb_headlen(skb);
 	int i, csum_insertion = 0, is_jumbo = 0;
 	int nfrags = skb_shinfo(skb)->nr_frags;
-	unsigned int entry, first_entry;
+	int entry, first_entry;
 	struct dma_desc *desc, *first;
 	unsigned int enh_desc;
 
-- 
1.9.1

Re: [PATCH] stmmac: Fix type of local variable in stmmac_xmit

From: David Miller <davem@davemloft.net>
Date: 2016-03-04 19:31:13

From: Andrzej Hajda <redacted>
Date: Fri, 04 Mar 2016 08:19:04 +0100
Variable entry holds result of jumbo_frm callback. It can be negative,
so the variable should be signed. The patch changes also type of related
first_entry variable to make code compact and coherent.

The problem has been detected using patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci.

Signed-off-by: Andrzej Hajda <redacted>
Actually, this only papers over a much deeper problem.

You cannot mix continually incrementing indexes with negative
return values.

You simply can't.

Because after enough traffic the legitimate indexes will be negative
integer values, so this jumbo index test would alway trigger.

A better fix is needed for this.  ->jumbo_frm() cannot return a value
that is interpreted both as an index as well as an error code.  The
dual signalling between these two values is simply impossible because
one the whole value space of a 32-bit integer is also a valid index.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help