Skip to contentSkip to conversation

libata: Read buffer overflow

PATCH In review Roel Kluin 2 participants 2 messages Aug 3, 2009 libata

lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 libata: Read buffer overflow

Check whether index is within bounds before grabbing the element.

Signed-off-by Roel Kluin <[email protected]>

---

drivers/ata/pata_mpc52xx.c +4 -2
index 2bc2dbe..f88c2ff 100644
@@ -294,10 +294,11 @@ mpc52xx_ata_compute_mdma_timings(struct mpc52xx_ata_priv *priv, int dev,
 				 int speed)
 {
 	struct mpc52xx_ata_timings *t = &priv->timings[dev];
-	const struct mdmaspec *s = &priv->mdmaspec[speed];
+	const struct mdmaspec *s;
 
 	if (speed < 0 || speed > 2)
 		return -EINVAL;
+	s = &priv->mdmaspec[speed];
 
 	t->mdma1 = (s->t0M << 24) | (s->td << 16) | (s->tkw << 8) | (s->tm);
 	t->mdma2 = (s->th << 24) | (s->tj << 16) | (s->tn << 8);

@@ -311,10 +312,11 @@ mpc52xx_ata_compute_udma_timings(struct mpc52xx_ata_priv *priv, int dev,
 				 int speed)
 {
 	struct mpc52xx_ata_timings *t = &priv->timings[dev];
-	const struct udmaspec *s = &priv->udmaspec[speed];
+	const struct udmaspec *s;
 
 	if (speed < 0 || speed > 2)
 		return -EINVAL;
+	s = &priv->udmaspec[speed];
 
 	t->udma1 = (s->t2cyc << 24) | (s->tcyc << 16) | (s->tds << 8) | s->tdh;
 	t->udma2 = (s->tdvs << 24) | (s->tdvh << 16) | (s->tfs << 8) | s->tli;
#1 lore raw
Quoted 1 line · Roel Kluin

Check whether index is within bounds before grabbing the element.

This isn't a correct description or analysis.

drivers/ata/pata_mpc52xx.c mpc52xx_ata_compute_mdma_timings(struct mpc52xx_ata_priv *priv, int dev, · line 297

Quoted 11 lines
Signed-off-by Roel Kluin <[email protected]>

---

drivers/ata/pata_mpc52xx.c +0 -1
index 2bc2dbe..f88c2ff 100644
@@ -294,10 +294,11 @@ mpc52xx_ata_compute_mdma_timings(struct mpc52xx_ata_priv *priv, int dev,
 				 int speed)
 {
 	struct mpc52xx_ata_timings *t = &priv->timings[dev];
-	const struct mdmaspec *s = &priv->mdmaspec[speed];

This is a *pointer* to the element, *not* a dereference. It's irrelevant whether the pointer is off the end of the array or not

drivers/ata/pata_mpc52xx.c mpc52xx_ata_compute_mdma_timings(struct mpc52xx_ata_priv *priv, int dev, · line 300

Quoted 4 lines
drivers/ata/pata_mpc52xx.c +1 -0
+	const struct mdmaspec *s;
 
 	if (speed < 0 || speed > 2)
 		return -EINVAL;

And since the check is immediately after, the pointer is never dereferenced.

drivers/ata/pata_mpc52xx.c mpc52xx_ata_compute_mdma_timings(struct mpc52xx_ata_priv *priv, int dev, · line 301

Quoted 1 line
drivers/ata/pata_mpc52xx.c +1 -0
+	s = &priv->mdmaspec[speed];

James