* [PATCH 2/5] staging/slicoss: Remove always true if statement
2013-02-19 4:13 [PATCH 1/5] staging/slicoss: Check pointer before dereferencing Peter Huewe
@ 2013-02-19 4:18 ` Peter Huewe
2013-02-19 4:18 ` [PATCH 3/5] staging/slicoss: Fix operation may be undefined warning Peter Huewe
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Peter Huewe @ 2013-02-19 4:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Devendra Naga, Peter Huewe, Bill Pemberton, Cruz Julian Bishop,
devel, linux-kernel
skbtype is assigned once to NORMAL_ETHFRAME and then checked if it is
NORMAL_ETHFRAME -> remove the checks.
This also gets rid of the (false positive) smatch warning:
slicoss.c:2829 slic_xmit_start() error: potential NULL dereference
'hcmd'.
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
drivers/staging/slicoss/slicoss.c | 24 ++++++++++--------------
1 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index 753993c..048a341 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -2786,7 +2786,6 @@ static netdev_tx_t slic_xmit_start(struct sk_buff *skb, struct net_device *dev)
struct adapter *adapter = netdev_priv(dev);
struct slic_hostcmd *hcmd = NULL;
u32 status = 0;
- u32 skbtype = NORMAL_ETHFRAME;
void *offloadcmd = NULL;
card = adapter->card;
@@ -2800,19 +2799,16 @@ static netdev_tx_t slic_xmit_start(struct sk_buff *skb, struct net_device *dev)
goto xmit_fail;
}
- if (skbtype == NORMAL_ETHFRAME) {
- hcmd = slic_cmdq_getfree(adapter);
- if (!hcmd) {
- adapter->xmitq_full = 1;
- status = XMIT_FAIL_HOSTCMD_FAIL;
- goto xmit_fail;
- }
- hcmd->skb = skb;
- hcmd->busy = 1;
- hcmd->type = SLIC_CMD_DUMB;
- if (skbtype == NORMAL_ETHFRAME)
- slic_xmit_build_request(adapter, hcmd, skb);
+ hcmd = slic_cmdq_getfree(adapter);
+ if (!hcmd) {
+ adapter->xmitq_full = 1;
+ status = XMIT_FAIL_HOSTCMD_FAIL;
+ goto xmit_fail;
}
+ hcmd->skb = skb;
+ hcmd->busy = 1;
+ hcmd->type = SLIC_CMD_DUMB;
+ slic_xmit_build_request(adapter, hcmd, skb);
dev->stats.tx_packets++;
dev->stats.tx_bytes += skb->len;
@@ -2838,7 +2834,7 @@ static netdev_tx_t slic_xmit_start(struct sk_buff *skb, struct net_device *dev)
xmit_done:
return NETDEV_TX_OK;
xmit_fail:
- slic_xmit_fail(adapter, skb, offloadcmd, skbtype, status);
+ slic_xmit_fail(adapter, skb, offloadcmd, NORMAL_ETHFRAME, status);
goto xmit_done;
}
--
1.7.8.6
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 3/5] staging/slicoss: Fix operation may be undefined warning
2013-02-19 4:13 [PATCH 1/5] staging/slicoss: Check pointer before dereferencing Peter Huewe
2013-02-19 4:18 ` [PATCH 2/5] staging/slicoss: Remove always true if statement Peter Huewe
@ 2013-02-19 4:18 ` Peter Huewe
2013-02-19 4:18 ` [PATCH 4/5] staging/slicoss: Fix buffer possible overflow in slic_card_locate Peter Huewe
2013-02-19 4:18 ` [PATCH 5/5] staging/slicoss: Use ether_crc for mac hash calculation Peter Huewe
3 siblings, 0 replies; 5+ messages in thread
From: Peter Huewe @ 2013-02-19 4:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Devendra Naga, Peter Huewe, Bill Pemberton, Cruz Julian Bishop,
devel, linux-kernel
gcc complains about an undefined operation:
slicoss.c:1417:19: warning: operation on 'rspq->pageindex' may be
undefined [-Wsequence-point]
The intended operation was (probably) to retrieve the pageindex + 1 and let
it wrap around if it reaches the num_pages.
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
drivers/staging/slicoss/slicoss.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index 048a341..fc08585 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -1414,7 +1414,7 @@ static struct slic_rspbuf *slic_rspqueue_getnext(struct adapter *adapter)
slic_reg64_write(adapter, &adapter->slic_regs->slic_rbar64,
(rspq->paddr[rspq->pageindex] | SLIC_RSPQ_BUFSINPAGE),
&adapter->slic_regs->slic_addr_upper, 0, DONT_FLUSH);
- rspq->pageindex = (++rspq->pageindex) % rspq->num_pages;
+ rspq->pageindex = (rspq->pageindex + 1) % rspq->num_pages;
rspq->offset = 0;
rspq->rspbuf = (struct slic_rspbuf *)
rspq->vaddr[rspq->pageindex];
--
1.7.8.6
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 4/5] staging/slicoss: Fix buffer possible overflow in slic_card_locate
2013-02-19 4:13 [PATCH 1/5] staging/slicoss: Check pointer before dereferencing Peter Huewe
2013-02-19 4:18 ` [PATCH 2/5] staging/slicoss: Remove always true if statement Peter Huewe
2013-02-19 4:18 ` [PATCH 3/5] staging/slicoss: Fix operation may be undefined warning Peter Huewe
@ 2013-02-19 4:18 ` Peter Huewe
2013-02-19 4:18 ` [PATCH 5/5] staging/slicoss: Use ether_crc for mac hash calculation Peter Huewe
3 siblings, 0 replies; 5+ messages in thread
From: Peter Huewe @ 2013-02-19 4:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Devendra Naga, Peter Huewe, Bill Pemberton, Cruz Julian Bishop,
devel, linux-kernel
smatch complains about a possible buffer overflow
slicoss.c:3651 slic_card_locate() error: buffer overflow
'physcard->adapter' 4 <= 4
If the for loop is not exited prematurely i++ is executed after the last
iteration and thus i can be 4, which is out of bounds for
physcard->adapter.
-> Add check for this condition and simplify the if statement by
inverting the condition.
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
drivers/staging/slicoss/slicoss.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index fc08585..48056bf 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -3643,11 +3643,12 @@ static u32 slic_card_locate(struct adapter *adapter)
while (physcard) {
for (i = 0; i < SLIC_MAX_PORTS; i++) {
- if (!physcard->adapter[i])
- continue;
- else
+ if (physcard->adapter[i])
break;
}
+ if (i == SLIC_MAX_PORTS)
+ break;
+
if (physcard->adapter[i]->slotnumber == adapter->slotnumber)
break;
physcard = physcard->next;
--
1.7.8.6
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 5/5] staging/slicoss: Use ether_crc for mac hash calculation
2013-02-19 4:13 [PATCH 1/5] staging/slicoss: Check pointer before dereferencing Peter Huewe
` (2 preceding siblings ...)
2013-02-19 4:18 ` [PATCH 4/5] staging/slicoss: Fix buffer possible overflow in slic_card_locate Peter Huewe
@ 2013-02-19 4:18 ` Peter Huewe
3 siblings, 0 replies; 5+ messages in thread
From: Peter Huewe @ 2013-02-19 4:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Devendra Naga, Peter Huewe, Bill Pemberton, Cruz Julian Bishop,
devel, linux-kernel
Instead of performing the hash calculation for the mac address by ourself, we
can simply reuse ether_crc and shift only the result according to our
needs.
The code was tested against the previous implementation by verifying both
implementations against each other in userspace for 16200000000 different
mac addresses, changing the vendor bits of the mac address first.
Signed-off-by: Peter Huewe <peterhuewe@gmx.de>
---
drivers/staging/slicoss/slicoss.c | 69 ++----------------------------------
1 files changed, 4 insertions(+), 65 deletions(-)
diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index 48056bf..58a00c2 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -76,6 +76,7 @@
#include <linux/bitops.h>
#include <linux/io.h>
#include <linux/netdevice.h>
+#include <linux/crc32.h>
#include <linux/etherdevice.h>
#include <linux/skbuff.h>
#include <linux/delay.h>
@@ -189,76 +190,14 @@ static inline void slic_reg64_write(struct adapter *adapter, void __iomem *reg,
adapter->bit64reglock.flags);
}
-/*
- * Functions to obtain the CRC corresponding to the destination mac address.
- * This is a standard ethernet CRC in that it is a 32-bit, reflected CRC using
- * the polynomial:
- * x^32 + x^26 + x^23 + x^22 + x^16 + x^12 + x^11 + x^10 + x^8 + x^7 + x^5 +
- * x^4 + x^2 + x^1.
- *
- * After the CRC for the 6 bytes is generated (but before the value is
- * complemented),
- * we must then transpose the value and return bits 30-23.
- *
- */
-static u32 slic_crc_table[256]; /* Table of CRCs for all possible byte values */
-static u32 slic_crc_init; /* Is table initialized */
-
-/*
- * Contruct the CRC32 table
- */
-static void slic_mcast_init_crc32(void)
-{
- u32 c; /* CRC reg */
- u32 e = 0; /* Poly X-or pattern */
- int i; /* counter */
- int k; /* byte being shifted into crc */
-
- static int p[] = { 0, 1, 2, 4, 5, 7, 8, 10, 11, 12, 16, 22, 23, 26 };
-
- for (i = 0; i < ARRAY_SIZE(p); i++)
- e |= 1L << (31 - p[i]);
-
- for (i = 1; i < 256; i++) {
- c = i;
- for (k = 8; k; k--)
- c = c & 1 ? (c >> 1) ^ e : c >> 1;
- slic_crc_table[i] = c;
- }
-}
-
-/*
- * Return the MAC hast as described above.
- */
-static unsigned char slic_mcast_get_mac_hash(char *macaddr)
-{
- u32 crc;
- char *p;
- int i;
- unsigned char machash = 0;
-
- if (!slic_crc_init) {
- slic_mcast_init_crc32();
- slic_crc_init = 1;
- }
-
- crc = 0xFFFFFFFF; /* Preload shift register, per crc-32 spec */
- for (i = 0, p = macaddr; i < 6; ++p, ++i)
- crc = (crc >> 8) ^ slic_crc_table[(crc ^ *p) & 0xFF];
-
- /* Return bits 1-8, transposed */
- for (i = 1; i < 9; i++)
- machash |= (((crc >> i) & 1) << (8 - i));
-
- return machash;
-}
-
static void slic_mcast_set_bit(struct adapter *adapter, char *address)
{
unsigned char crcpoly;
/* Get the CRC polynomial for the mac address */
- crcpoly = slic_mcast_get_mac_hash(address);
+ /* we use bits 1-8 (lsb), bitwise reversed,
+ * msb (= lsb bit 0 before bitrev) is automatically discarded */
+ crcpoly = (ether_crc(ETH_ALEN, address)>>23);
/* We only have space on the SLIC for 64 entries. Lop
* off the top two bits. (2^6 = 64)
--
1.7.8.6
^ permalink raw reply [flat|nested] 5+ messages in thread