[PATCH] orinoco: Fix walking past the end of the buffer

[PATCH] orinoco: Fix walking past the end of the buffer

[Denis Kirjanov]

Fix walking past the end of the bitrate_table array
in the case when the loop counter == BITRATE_TABLE_SIZE.
Mark bitrate as invalid in this case for the orinoco_ioctl_setrate()

Signed-off-by: Denis Kirjanov <dkirjanov@kernel.org>
---

diff --git a/drivers/net/wireless/orinoco/hw.c b/drivers/net/wireless/orinoco/hw.c
index 077baa8..191bc03 100644
--- a/drivers/net/wireless/orinoco/hw.c
+++ b/drivers/net/wireless/orinoco/hw.c
@@ -765,9 +765,12 @@ int orinoco_hw_get_act_bitrate(struct orinoco_private *priv, int *bitrate)
 			if (bitrate_table[i].intersil_txratectrl == val)
 				break;
 
-		if (i >= BITRATE_TABLE_SIZE)
+		if (i >= BITRATE_TABLE_SIZE) {
 			printk(KERN_INFO "%s: Unable to determine current bitrate (0x%04hx)\n",
 			       priv->ndev->name, val);
+			*bitrate = 100001; /* Mark as invalid */
+			break;
+		}
 
 		*bitrate = bitrate_table[i].bitrate * 100000;
 		break;

Re: [PATCH] orinoco: Fix walking past the end of the buffer

[Dave Kilroy]

On Wed, Aug 11, 2010 at 9:32 PM, Denis Kirjanov <dkirjanov@kernel.org> wrote:
> diff --git a/drivers/net/wireless/orinoco/hw.c b/drivers/net/wireless/orinoco/hw.c
> index 077baa8..191bc03 100644
> --- a/drivers/net/wireless/orinoco/hw.c
> +++ b/drivers/net/wireless/orinoco/hw.c
> @@ -765,9 +765,12 @@ int orinoco_hw_get_act_bitrate(struct orinoco_private *priv, int *bitrate)
>                        if (bitrate_table[i].intersil_txratectrl == val)
>                                break;
>
> -               if (i >= BITRATE_TABLE_SIZE)
> +               if (i >= BITRATE_TABLE_SIZE) {
>                        printk(KERN_INFO "%s: Unable to determine current bitrate (0x%04hx)\n",
>                               priv->ndev->name, val);
> +                       *bitrate = 100001; /* Mark as invalid */

We should propogate the failure by returning an error in the return
code rather than a cryptic bitrate value. The calling function(s)
should then propogate the error through wext/cfg80211 as appropriate.

> +                       break;
> +               }
>
>                *bitrate = bitrate_table[i].bitrate * 100000;
>                break;

We can also make the structure easier to understand by setting the
bitrate within the for loop. Something like the following (I only have
access to gmail ATM, so can't format a proper patch):

		for (i = 0; i < BITRATE_TABLE_SIZE; i++)
			if (bitrate_table[i].intersil_txratectrl == val) {
				*bitrate = bitrate_table[i].bitrate * 100000;
				break;
			}

		if (i >= BITRATE_TABLE_SIZE) {
			printk(KERN_INFO "%s: Unable to determine current bitrate (0x%04hx)\n",
			       priv->ndev->name, val);
			err = -EIO; /* maybe chose a better value... */
		}

		break;

Could you update the patch along those lines please?

Thanks,

Dave.

[PATCH] orinoco: Fix walking past the end of the buffer

[David Kilroy]

Fix walking past the end of the bitrate_table array
in the case when the loop counter == BITRATE_TABLE_SIZE.

Reported by: Denis Kirjanov <dkirjanov@kernel.org>
Signed-off-by: David Kilroy <kilroyd@googlemail.com>
---

This patch improves upon Denis' original by reporting the error to
the caller and handling it there instead of passing an invalid
bitrate to userspace. In this case we can ignore the error and send
userspace the known configuration.

---
 drivers/net/wireless/orinoco/hw.c   |    9 ++++++---
 drivers/net/wireless/orinoco/wext.c |   11 +++++++++--
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/orinoco/hw.c b/drivers/net/wireless/orinoco/hw.c
index 077baa8..b4772c1 100644
--- a/drivers/net/wireless/orinoco/hw.c
+++ b/drivers/net/wireless/orinoco/hw.c
@@ -762,14 +762,17 @@ int orinoco_hw_get_act_bitrate(struct orinoco_private *priv, int *bitrate)
 	case FIRMWARE_TYPE_INTERSIL: /* Intersil style rate */
 	case FIRMWARE_TYPE_SYMBOL: /* Symbol style rate */
 		for (i = 0; i < BITRATE_TABLE_SIZE; i++)
-			if (bitrate_table[i].intersil_txratectrl == val)
+			if (bitrate_table[i].intersil_txratectrl == val) {
+				*bitrate = bitrate_table[i].bitrate * 100000;
 				break;
+			}
 
-		if (i >= BITRATE_TABLE_SIZE)
+		if (i >= BITRATE_TABLE_SIZE) {
 			printk(KERN_INFO "%s: Unable to determine current bitrate (0x%04hx)\n",
 			       priv->ndev->name, val);
+			err = -EIO;
+		}
 
-		*bitrate = bitrate_table[i].bitrate * 100000;
 		break;
 	default:
 		BUG();
diff --git a/drivers/net/wireless/orinoco/wext.c b/drivers/net/wireless/orinoco/wext.c
index cf7be1e..93505f9 100644
--- a/drivers/net/wireless/orinoco/wext.c
+++ b/drivers/net/wireless/orinoco/wext.c
@@ -589,8 +589,15 @@ static int orinoco_ioctl_getrate(struct net_device *dev,
 
 	/* If the interface is running we try to find more about the
 	   current mode */
-	if (netif_running(dev))
-		err = orinoco_hw_get_act_bitrate(priv, &bitrate);
+	if (netif_running(dev)) {
+		int act_bitrate;
+		int lerr;
+
+		/* Ignore errors if we can't get the actual bitrate */
+		lerr = orinoco_hw_get_act_bitrate(priv, &act_bitrate);
+		if (!lerr)
+			bitrate = act_bitrate;
+	}
 
 	orinoco_unlock(priv, &flags);
 
-- 
1.7.1