Skip to contentSkip to conversation

pch_can: off by one bugs

PATCH In review Dan Carpenter 3 participants 3 messages Dec 20, 2010 – Dec 21, 2010

Subsystems (4)
Networking Network drivers Kernel janitors CAN network drivers
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 pch_can: off by one bugs

priv->tx_enable[] has PCH_TX_OBJ_END elements so this code is reading and writing one past the end of the array.

Signed-off-by Dan Carpenter <[email protected]>
drivers/net/can/pch_can.c +2 -2
index 8d45fdd..b2c1292 100644
@@ -1077,7 +1077,7 @@ static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
 	pch_can_set_int_enables(priv, PCH_CAN_DISABLE);
 
 	/* Save Tx buffer enable state */
-	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++)
+	for (i = PCH_TX_OBJ_START; i < PCH_TX_OBJ_END; i++)
 		priv->tx_enable[i] = pch_can_get_rxtx_ir(priv, i, PCH_TX_IFREG);
 
 	/* Disable all Transmit buffers */

@@ -1138,7 +1138,7 @@ static int pch_can_resume(struct pci_dev *pdev)
 	pch_can_set_optmode(priv);
 
 	/* Enabling the transmit buffer. */
-	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++)
+	for (i = PCH_TX_OBJ_START; i < PCH_TX_OBJ_END; i++)
 		pch_can_set_rxtx(priv, i, priv->tx_enable[i], PCH_TX_IFREG);
 
 	/* Configuring the receive buffer and enabling them. */
#1 lore raw

Hello,

Quoted 20 lines · Dan Carpenter

priv->tx_enable[] has PCH_TX_OBJ_END elements so this code is reading and writing one past the end of the array.

Signed-off-by Dan Carpenter <[email protected]>
drivers/net/can/pch_can.c +1 -1
index 8d45fdd..b2c1292 100644
@@ -1077,7 +1077,7 @@ static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
 	pch_can_set_int_enables(priv, PCH_CAN_DISABLE);
 
 	/* Save Tx buffer enable state */
-	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++)
+	for (i = PCH_TX_OBJ_START; i < PCH_TX_OBJ_END; i++)
 		priv->tx_enable[i] = pch_can_get_rxtx_ir(priv, i, PCH_TX_IFREG);
 
 	/* Disable all Transmit buffers */

drivers/net/can/pch_can.c static int pch_can_resume(struct pci_dev *pdev) · line 1144

Quoted 8 lines
drivers/net/can/pch_can.c +1 -1
@@ -1138,7 +1138,7 @@ static int pch_can_resume(struct pci_dev *pdev)
 	pch_can_set_optmode(priv);
 
 	/* Enabling the transmit buffer. */
-	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++)
+	for (i = PCH_TX_OBJ_START; i < PCH_TX_OBJ_END; i++)
 		pch_can_set_rxtx(priv, i, priv->tx_enable[i], PCH_TX_IFREG);
 
 	/* Configuring the receive buffer and enabling them. */

This fix does not look correct too me. There are much more loop using "i <= PCH_TX_OBJ_END" and the message numbering is from 1..32. Therefore using "priv->tx_enable[i - 1]" seems more appropriate to me. Tomaya, could you please check.

Thanks,

Wolfgang.

#2 lore raw
Quoted 1 line · Wolfgang Grandegger

This fix does not look correct too me.

I agree with Wolfgang's comments.

Quoted 1 line

Dan

Thank you for your report.
I will post the patch by this week.

Thanks,
---
Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

----- Original Message -----
From: "Wolfgang Grandegger" <[email protected]>
To: "Dan Carpenter" <[email protected]>
Cc: <[email protected]>; <[email protected]>; <[email protected]>; "Tomoya MORINAGA" <[email protected]>
Sent: Wednesday, December 22, 2010 5:41 AM
Subject: Re: [patch -next] pch_can: off by one bugs

Quoted 47 lines

Hello,

Quoted 21 lines · Dan Carpenter

priv->tx_enable[] has PCH_TX_OBJ_END elements so this code is reading and writing one past the end of the array.

Signed-off-by Dan Carpenter <[email protected]>
drivers/net/can/pch_can.c +1 -1
index 8d45fdd..b2c1292 100644
@@ -1077,7 +1077,7 @@ static int pch_can_suspend(struct pci_dev *pdev, 
 pm_message_t state)
 pch_can_set_int_enables(priv, PCH_CAN_DISABLE);
 
 /* Save Tx buffer enable state */
- for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++)
+ for (i = PCH_TX_OBJ_START; i < PCH_TX_OBJ_END; i++)
 priv->tx_enable[i] = pch_can_get_rxtx_ir(priv, i, PCH_TX_IFREG);

/* Disable all Transmit buffers */

drivers/net/can/pch_can.c static int pch_can_resume(struct pci_dev *pdev) · line 1144

Quoted 8 lines
drivers/net/can/pch_can.c +1 -1
@@ -1138,7 +1138,7 @@ static int pch_can_resume(struct pci_dev *pdev)
 pch_can_set_optmode(priv);
 
 /* Enabling the transmit buffer. */
- for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++)
+ for (i = PCH_TX_OBJ_START; i < PCH_TX_OBJ_END; i++)
 pch_can_set_rxtx(priv, i, priv->tx_enable[i], PCH_TX_IFREG);
 
 /* Configuring the receive buffer and enabling them. */

This fix does not look correct too me. There are much more loop using "i <= PCH_TX_OBJ_END" and the message numbering is from 1..32. Therefore using "priv->tx_enable[i - 1]" seems more appropriate to me. Tomaya, could you please check.

Thanks,

Wolfgang.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html