[PATCH 1/4] s2io: rx_ring_sz bounds checking

[PATCH 1/4] s2io: rx_ring_sz bounds checking

[Jon Mason]

modparm rx_ring_sz can be set to be greater than the maximum allowable
number of blocks.  This results in an array overrun when probing the
driver, and causes memory corruption.

Also, the MAX_RX_DESC_1 multiply the max number of rings by max number
of blocker per ring by 127, but the driver does the same calculation
with 127+1.  This results in the possibility of the value being set
being larger than the maximum allowable value.

Finally, clean-up the s2io_ethtool_gringparam code to be more
intuitive.

Signed-off-by: Jon Mason <jon.mason@exar.com>
---
 drivers/net/s2io.c |   40 ++++++++++++++++++++++++----------------
 drivers/net/s2io.h |    5 ++---
 2 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index 0f4219c..a6d3eaf 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -5568,30 +5568,27 @@ static void s2io_ethtool_gringparam(struct net_device *dev,
 	struct s2io_nic *sp = netdev_priv(dev);
 	int i, tx_desc_count = 0, rx_desc_count = 0;
 
-	if (sp->rxd_mode == RXD_MODE_1)
+	if (sp->rxd_mode == RXD_MODE_1) {
 		ering->rx_max_pending = MAX_RX_DESC_1;
-	else if (sp->rxd_mode == RXD_MODE_3B)
+		ering->rx_jumbo_max_pending = MAX_RX_DESC_1;
+	} else {
 		ering->rx_max_pending = MAX_RX_DESC_2;
+		ering->rx_jumbo_max_pending = MAX_RX_DESC_2;
+	}
 
+	ering->rx_mini_max_pending = 0;
 	ering->tx_max_pending = MAX_TX_DESC;
-	for (i = 0 ; i < sp->config.tx_fifo_num ; i++)
-		tx_desc_count += sp->config.tx_cfg[i].fifo_len;
 
-	DBG_PRINT(INFO_DBG, "max txds: %d\n", sp->config.max_txds);
-	ering->tx_pending = tx_desc_count;
-	rx_desc_count = 0;
-	for (i = 0 ; i < sp->config.rx_ring_num ; i++)
+	for (i = 0; i < sp->config.rx_ring_num; i++)
 		rx_desc_count += sp->config.rx_cfg[i].num_rxd;
-
 	ering->rx_pending = rx_desc_count;
-
-	ering->rx_mini_max_pending = 0;
-	ering->rx_mini_pending = 0;
-	if (sp->rxd_mode == RXD_MODE_1)
-		ering->rx_jumbo_max_pending = MAX_RX_DESC_1;
-	else if (sp->rxd_mode == RXD_MODE_3B)
-		ering->rx_jumbo_max_pending = MAX_RX_DESC_2;
 	ering->rx_jumbo_pending = rx_desc_count;
+	ering->rx_mini_pending = 0;
+
+	for (i = 0; i < sp->config.tx_fifo_num; i++)
+		tx_desc_count += sp->config.tx_cfg[i].fifo_len;
+	ering->tx_pending = tx_desc_count;
+	DBG_PRINT(INFO_DBG, "max txds: %d\n", sp->config.max_txds);
 }
 
 /**
@@ -7692,6 +7689,8 @@ static void s2io_init_pci(struct s2io_nic *sp)
 static int s2io_verify_parm(struct pci_dev *pdev, u8 *dev_intr_type,
 			    u8 *dev_multiq)
 {
+	int i;
+
 	if ((tx_fifo_num > MAX_TX_FIFOS) || (tx_fifo_num < 1)) {
 		DBG_PRINT(ERR_DBG, "Requested number of tx fifos "
 			  "(%d) not supported\n", tx_fifo_num);
@@ -7750,6 +7749,15 @@ static int s2io_verify_parm(struct pci_dev *pdev, u8 *dev_intr_type,
 		DBG_PRINT(ERR_DBG, "Defaulting to 1-buffer mode\n");
 		rx_ring_mode = 1;
 	}
+
+	for (i = 0; i < MAX_RX_RINGS; i++)
+		if (rx_ring_sz[i] > MAX_RX_BLOCKS_PER_RING) {
+			DBG_PRINT(ERR_DBG, "Requested rx ring size not "
+				  "supported\nDefaulting to %d\n",
+				  MAX_RX_BLOCKS_PER_RING);
+			rx_ring_sz[i] = MAX_RX_BLOCKS_PER_RING;
+		}
+
 	return SUCCESS;
 }
 
diff --git a/drivers/net/s2io.h b/drivers/net/s2io.h
index 00b8614..1671443 100644
--- a/drivers/net/s2io.h
+++ b/drivers/net/s2io.h
@@ -355,9 +355,8 @@ struct stat_block {
 #define FIFO_OTHER_MAX_NUM			1
 
 
-#define MAX_RX_DESC_1  (MAX_RX_RINGS * MAX_RX_BLOCKS_PER_RING * 127 )
-#define MAX_RX_DESC_2  (MAX_RX_RINGS * MAX_RX_BLOCKS_PER_RING * 85 )
-#define MAX_RX_DESC_3  (MAX_RX_RINGS * MAX_RX_BLOCKS_PER_RING * 85 )
+#define MAX_RX_DESC_1  (MAX_RX_RINGS * MAX_RX_BLOCKS_PER_RING * 128)
+#define MAX_RX_DESC_2  (MAX_RX_RINGS * MAX_RX_BLOCKS_PER_RING * 86)
 #define MAX_TX_DESC    (MAX_AVAILABLE_TXDS)
 
 /* FIFO mappings for all possible number of fifos configured */
-- 
1.7.0.4

[PATCH 2/4] s2io: make strings at tables const

[Jon Mason]

Put immutable data in read/only section.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Jon Mason <jon.mason@exar.com>
---
 drivers/net/s2io.c |    8 ++++----
 drivers/net/s2io.h |    4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index a6d3eaf..6a87b8c 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -91,11 +91,11 @@
 #define DRV_VERSION "2.0.26.27"
 
 /* S2io Driver name & version. */
-static char s2io_driver_name[] = "Neterion";
-static char s2io_driver_version[] = DRV_VERSION;
+static const char s2io_driver_name[] = "Neterion";
+static const char s2io_driver_version[] = DRV_VERSION;
 
-static int rxd_size[2] = {32, 48};
-static int rxd_count[2] = {127, 85};
+static const int rxd_size[2] = {32, 48};
+static const int rxd_count[2] = {127, 85};
 
 static inline int RXD_IS_UP2DT(struct RxD_t *rxdp)
 {
diff --git a/drivers/net/s2io.h b/drivers/net/s2io.h
index 1671443..7d16030 100644
--- a/drivers/net/s2io.h
+++ b/drivers/net/s2io.h
@@ -360,7 +360,7 @@ struct stat_block {
 #define MAX_TX_DESC    (MAX_AVAILABLE_TXDS)
 
 /* FIFO mappings for all possible number of fifos configured */
-static int fifo_map[][MAX_TX_FIFOS] = {
+static const int fifo_map[][MAX_TX_FIFOS] = {
 	{0, 0, 0, 0, 0, 0, 0, 0},
 	{0, 0, 0, 0, 1, 1, 1, 1},
 	{0, 0, 0, 1, 1, 1, 2, 2},
@@ -371,7 +371,7 @@ static int fifo_map[][MAX_TX_FIFOS] = {
 	{0, 1, 2, 3, 4, 5, 6, 7},
 };
 
-static u16 fifo_selector[MAX_TX_FIFOS] = {0, 1, 3, 3, 7, 7, 7, 7};
+static const u16 fifo_selector[MAX_TX_FIFOS] = {0, 1, 3, 3, 7, 7, 7, 7};
 
 /* Maintains Per FIFO related information. */
 struct tx_fifo_config {
-- 
1.7.0.4

[PATCH 3/4] s2io: Update Driver Version

[Jon Mason]

Update Driver Version

Signed-off-by: Jon Mason <jon.mason@exar.com>
---
 drivers/net/s2io.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index 6a87b8c..80efc05 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -88,7 +88,7 @@
 #include "s2io.h"
 #include "s2io-regs.h"
 
-#define DRV_VERSION "2.0.26.27"
+#define DRV_VERSION "2.0.26.28"
 
 /* S2io Driver name & version. */
 static const char s2io_driver_name[] = "Neterion";
-- 
1.7.0.4

[PATCH 4/4] Using static const generally increases object text and decreases data size. It also generally decreases overall object size.

[Jon Mason]

   text	   data	    bss	    dec	    hex	filename
 109387	    389	  24432	 134208	  20c40	drivers/net/s2io.o.old
 109358	    389	  24432	 134179	  20c23	drivers/net/s2io.o.new

Signed-off-by: Joe Perches <joe@perches.com>
Acked-by: Jon Mason <jon.mason@exar.com>
---
 drivers/net/s2io.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index 80efc05..9a1e32f 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -3598,10 +3598,12 @@ static int s2io_set_swapper(struct s2io_nic *sp)
 	val64 = readq(&bar0->pif_rd_swapper_fb);
 	if (val64 != 0x0123456789ABCDEFULL) {
 		int i = 0;
-		u64 value[] = { 0xC30000C3C30000C3ULL,   /* FE=1, SE=1 */
-				0x8100008181000081ULL,  /* FE=1, SE=0 */
-				0x4200004242000042ULL,  /* FE=0, SE=1 */
-				0};                     /* FE=0, SE=0 */
+		static const u64 value[] = {
+			0xC30000C3C30000C3ULL,	/* FE=1, SE=1 */
+			0x8100008181000081ULL,	/* FE=1, SE=0 */
+			0x4200004242000042ULL,	/* FE=0, SE=1 */
+			0			/* FE=0, SE=0 */
+		};
 
 		while (i < 4) {
 			writeq(value[i], &bar0->swapper_ctrl);
@@ -3627,10 +3629,12 @@ static int s2io_set_swapper(struct s2io_nic *sp)
 
 	if (val64 != valt) {
 		int i = 0;
-		u64 value[] = { 0x00C3C30000C3C300ULL,  /* FE=1, SE=1 */
-				0x0081810000818100ULL,  /* FE=1, SE=0 */
-				0x0042420000424200ULL,  /* FE=0, SE=1 */
-				0};                     /* FE=0, SE=0 */
+		static const u64 value[] = {
+			0x00C3C30000C3C300ULL,	/* FE=1, SE=1 */
+			0x0081810000818100ULL,	/* FE=1, SE=0 */
+			0x0042420000424200ULL,	/* FE=0, SE=1 */
+			0			/* FE=0, SE=0 */
+		};
 
 		while (i < 4) {
 			writeq((value[i] | valr), &bar0->swapper_ctrl);
-- 
1.7.0.4

Re: [PATCH 1/4] s2io: rx_ring_sz bounds checking

[David Miller]

From: Jon Mason <jon.mason@exar.com>
Date: Fri, 10 Dec 2010 19:40:01 -0600

> modparm rx_ring_sz can be set to be greater than the maximum allowable
> number of blocks.  This results in an array overrun when probing the
> driver, and causes memory corruption.
> 
> Also, the MAX_RX_DESC_1 multiply the max number of rings by max number
> of blocker per ring by 127, but the driver does the same calculation
> with 127+1.  This results in the possibility of the value being set
> being larger than the maximum allowable value.
> 
> Finally, clean-up the s2io_ethtool_gringparam code to be more
> intuitive.
> 
> Signed-off-by: Jon Mason <jon.mason@exar.com>

Applied.

Re: [PATCH 2/4] s2io: make strings at tables const

[David Miller]

From: Jon Mason <jon.mason@exar.com>
Date: Fri, 10 Dec 2010 19:40:02 -0600

> Put immutable data in read/only section.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> Signed-off-by: Jon Mason <jon.mason@exar.com>

Applied.

Re: [PATCH 3/4] s2io: Update Driver Version

[David Miller]

From: Jon Mason <jon.mason@exar.com>
Date: Fri, 10 Dec 2010 19:40:03 -0600

> Update Driver Version
> 
> Signed-off-by: Jon Mason <jon.mason@exar.com>

Applied.

Re: [PATCH 4/4] Using static const generally increases object text and decreases data size. It also generally decreases overall object size.

[David Miller]

From: Jon Mason <jon.mason@exar.com>
Date: Fri, 10 Dec 2010 19:40:04 -0600

>    text	   data	    bss	    dec	    hex	filename
>  109387	    389	  24432	 134208	  20c40	drivers/net/s2io.o.old
>  109358	    389	  24432	 134179	  20c23	drivers/net/s2io.o.new
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> Acked-by: Jon Mason <jon.mason@exar.com>

Applied, but I had to add an "s2io: " prefix to the subject line.

Thanks.