2003-03-22 06:43:15

by Randy.Dunlap

[permalink] [raw]
Subject: [PATCH] reduce stack in wireless/airo.c

patch_name: airo-stack.patch
patch_version: 2003-03-21.22:14:20
author: Randy.Dunlap <[email protected]>
description: reduce stack usage in drivers/net/wireless/airo::
readrids() and writerids();
product: Linux
product_versions: 2.5.65
changelog: kmalloc() 2 KB buffers instead of declaring them on stack;
maintainer: Jean Tourrilhes <[email protected]>
diffstat: =
drivers/net/wireless/airo.c | 80 ++++++++++++++++++++++++++++----------------
1 files changed, 52 insertions(+), 28 deletions(-)


diff -Naur ./drivers/net/wireless/airo.c%RIDS ./drivers/net/wireless/airo.c
--- ./drivers/net/wireless/airo.c%RIDS Mon Mar 17 13:43:40 2003
+++ ./drivers/net/wireless/airo.c Fri Mar 21 22:14:01 2003
@@ -5894,6 +5894,7 @@
#endif /* WIRELESS_EXT */

#ifdef CISCO_EXT
+#define RIDS_SIZE 2048
/*
* This just translates from driver IOCTL codes to the command codes to
* feed to the radio's host interface. Things can be added/deleted
@@ -5902,11 +5903,15 @@
*/
static int readrids(struct net_device *dev, aironet_ioctl *comp) {
unsigned short ridcode;
- unsigned char iobuf[2048];
+ unsigned char *iobuf;
struct airo_info *ai = dev->priv;
+ int ret = 0;

if (ai->flags & FLAG_FLASHING)
return -EIO;
+ iobuf = kmalloc(RIDS_SIZE, GFP_KERNEL);
+ if (!iobuf)
+ return -ENOMEM;

switch(comp->command)
{
@@ -5919,13 +5924,17 @@
case AIROGEHTENC: ridcode = RID_ETHERENCAP; break;
case AIROGWEPKTMP: ridcode = RID_WEP_TEMP;
/* Only super-user can read WEP keys */
- if (!capable(CAP_NET_ADMIN))
- return -EPERM;
+ if (!capable(CAP_NET_ADMIN)) {
+ ret = -EPERM;
+ goto rr_free;
+ }
break;
case AIROGWEPKNV: ridcode = RID_WEP_PERM;
/* Only super-user can read WEP keys */
- if (!capable(CAP_NET_ADMIN))
- return -EPERM;
+ if (!capable(CAP_NET_ADMIN)) {
+ ret = -EPERM;
+ goto rr_free;
+ }
break;
case AIROGSTAT: ridcode = RID_STATUS; break;
case AIROGSTATSD32: ridcode = RID_STATSDELTA; break;
@@ -5933,23 +5942,25 @@
case AIROGMICSTATS:
if (copy_to_user(comp->data, &ai->micstats,
min((int)comp->len,(int)sizeof(ai->micstats))))
- return -EFAULT;
- return 0;
+ ret = -EFAULT;
+ goto rr_free;
default:
- return -EINVAL;
- break;
+ ret = -EINVAL;
+ goto rr_free;
}

- PC4500_readrid(ai,ridcode,iobuf,sizeof(iobuf));
+ PC4500_readrid(ai,ridcode,iobuf,RIDS_SIZE);
/* get the count of bytes in the rid docs say 1st 2 bytes is it.
* then return it to the user
* 9/22/2000 Honor user given length
*/

if (copy_to_user(comp->data, iobuf,
- min((int)comp->len, (int)sizeof(iobuf))))
- return -EFAULT;
- return 0;
+ min((int)comp->len, (int)RIDS_SIZE)))
+ ret = -EFAULT;
+rr_free:
+ kfree(iobuf);
+ return ret;
}

/*
@@ -5961,7 +5972,8 @@
int ridcode, enabled;
Resp rsp;
static int (* writer)(struct airo_info *, u16 rid, const void *, int);
- unsigned char iobuf[2048];
+ unsigned char *iobuf;
+ int ret = 0;

/* Only super-user can write RIDs */
if (!capable(CAP_NET_ADMIN))
@@ -5970,6 +5982,10 @@
if (ai->flags & FLAG_FLASHING)
return -EIO;

+ iobuf = kmalloc(RIDS_SIZE, GFP_KERNEL);
+ if (!iobuf)
+ return -ENOMEM;
+
ridcode = 0;
writer = do_writerid;

@@ -5991,8 +6007,8 @@
*/
case AIROPMACON:
if (enable_MAC(ai, &rsp) != 0)
- return -EIO;
- return 0;
+ ret = -EIO;
+ goto wr_free;

/*
* Evidently this code in the airo driver does not get a symbol
@@ -6000,32 +6016,38 @@
*/
case AIROPMACOFF:
disable_MAC(ai);
- return 0;
+ goto wr_free;

/* This command merely clears the counts does not actually store any data
* only reads rid. But as it changes the cards state, I put it in the
* writerid routines.
*/
case AIROPSTCLR:
- PC4500_readrid(ai,RID_STATSDELTACLEAR,iobuf,sizeof(iobuf));
+ PC4500_readrid(ai,RID_STATSDELTACLEAR,iobuf,RIDS_SIZE);

enabled = ai->micstats.enabled;
memset(&ai->micstats,0,sizeof(ai->micstats));
ai->micstats.enabled = enabled;

if (copy_to_user(comp->data, iobuf,
- min((int)comp->len, (int)sizeof(iobuf))))
- return -EFAULT;
- return 0;
+ min((int)comp->len, (int)RIDS_SIZE)))
+ ret = -EFAULT;
+ goto wr_free;

default:
- return -EOPNOTSUPP; /* Blarg! */
+ ret = -EOPNOTSUPP; /* Blarg! */
+ goto wr_free;
+ }
+
+ if (comp->len > RIDS_SIZE) {
+ ret = -EINVAL;
+ goto wr_free;
}
- if(comp->len > sizeof(iobuf))
- return -EINVAL;

- if (copy_from_user(iobuf,comp->data,comp->len))
- return -EFAULT;
+ if (copy_from_user(iobuf,comp->data,comp->len)) {
+ ret = -EFAULT;
+ goto wr_free;
+ }

if (comp->command == AIROPCFG) {
ConfigRid *cfg = (ConfigRid *)iobuf;
@@ -6040,8 +6062,10 @@
}

if((*writer)(ai, ridcode, iobuf,comp->len))
- return -EIO;
- return 0;
+ ret = -EIO;
+wr_free:
+ kfree(iobuf);
+ return ret;
}

/*****************************************************************************


Attachments:
airo-stack.patch (4.76 kB)

2003-03-24 19:00:55

by Jean Tourrilhes

[permalink] [raw]
Subject: Re: [PATCH] reduce stack in wireless/airo.c

On Fri, Mar 21, 2003 at 10:51:37PM -0800, Randy.Dunlap wrote:
> Hi,
>
> This reduces stack usage in drivers/net/wireless/airo.c by
> dynamically allocating 2KB buffers in 2 places.
>
> Patch is to 2.5.65. Please apply.
>
> ~Randy

I think it would be polite and fair for you to also contact
the driver maintainers (Ben and Javier).
Thanks...

Jean


> patch_name: airo-stack.patch
> patch_version: 2003-03-21.22:14:20
> author: Randy.Dunlap <[email protected]>
> description: reduce stack usage in drivers/net/wireless/airo::
> readrids() and writerids();
> product: Linux
> product_versions: 2.5.65
> changelog: kmalloc() 2 KB buffers instead of declaring them on stack;
> maintainer: Jean Tourrilhes <[email protected]>
> diffstat: =
> drivers/net/wireless/airo.c | 80 ++++++++++++++++++++++++++++----------------
> 1 files changed, 52 insertions(+), 28 deletions(-)
>
>
> diff -Naur ./drivers/net/wireless/airo.c%RIDS ./drivers/net/wireless/airo.c
> --- ./drivers/net/wireless/airo.c%RIDS Mon Mar 17 13:43:40 2003
> +++ ./drivers/net/wireless/airo.c Fri Mar 21 22:14:01 2003
> @@ -5894,6 +5894,7 @@
> #endif /* WIRELESS_EXT */
>
> #ifdef CISCO_EXT
> +#define RIDS_SIZE 2048
> /*
> * This just translates from driver IOCTL codes to the command codes to
> * feed to the radio's host interface. Things can be added/deleted
> @@ -5902,11 +5903,15 @@
> */
> static int readrids(struct net_device *dev, aironet_ioctl *comp) {
> unsigned short ridcode;
> - unsigned char iobuf[2048];
> + unsigned char *iobuf;
> struct airo_info *ai = dev->priv;
> + int ret = 0;
>
> if (ai->flags & FLAG_FLASHING)
> return -EIO;
> + iobuf = kmalloc(RIDS_SIZE, GFP_KERNEL);
> + if (!iobuf)
> + return -ENOMEM;
>
> switch(comp->command)
> {
> @@ -5919,13 +5924,17 @@
> case AIROGEHTENC: ridcode = RID_ETHERENCAP; break;
> case AIROGWEPKTMP: ridcode = RID_WEP_TEMP;
> /* Only super-user can read WEP keys */
> - if (!capable(CAP_NET_ADMIN))
> - return -EPERM;
> + if (!capable(CAP_NET_ADMIN)) {
> + ret = -EPERM;
> + goto rr_free;
> + }
> break;
> case AIROGWEPKNV: ridcode = RID_WEP_PERM;
> /* Only super-user can read WEP keys */
> - if (!capable(CAP_NET_ADMIN))
> - return -EPERM;
> + if (!capable(CAP_NET_ADMIN)) {
> + ret = -EPERM;
> + goto rr_free;
> + }
> break;
> case AIROGSTAT: ridcode = RID_STATUS; break;
> case AIROGSTATSD32: ridcode = RID_STATSDELTA; break;
> @@ -5933,23 +5942,25 @@
> case AIROGMICSTATS:
> if (copy_to_user(comp->data, &ai->micstats,
> min((int)comp->len,(int)sizeof(ai->micstats))))
> - return -EFAULT;
> - return 0;
> + ret = -EFAULT;
> + goto rr_free;
> default:
> - return -EINVAL;
> - break;
> + ret = -EINVAL;
> + goto rr_free;
> }
>
> - PC4500_readrid(ai,ridcode,iobuf,sizeof(iobuf));
> + PC4500_readrid(ai,ridcode,iobuf,RIDS_SIZE);
> /* get the count of bytes in the rid docs say 1st 2 bytes is it.
> * then return it to the user
> * 9/22/2000 Honor user given length
> */
>
> if (copy_to_user(comp->data, iobuf,
> - min((int)comp->len, (int)sizeof(iobuf))))
> - return -EFAULT;
> - return 0;
> + min((int)comp->len, (int)RIDS_SIZE)))
> + ret = -EFAULT;
> +rr_free:
> + kfree(iobuf);
> + return ret;
> }
>
> /*
> @@ -5961,7 +5972,8 @@
> int ridcode, enabled;
> Resp rsp;
> static int (* writer)(struct airo_info *, u16 rid, const void *, int);
> - unsigned char iobuf[2048];
> + unsigned char *iobuf;
> + int ret = 0;
>
> /* Only super-user can write RIDs */
> if (!capable(CAP_NET_ADMIN))
> @@ -5970,6 +5982,10 @@
> if (ai->flags & FLAG_FLASHING)
> return -EIO;
>
> + iobuf = kmalloc(RIDS_SIZE, GFP_KERNEL);
> + if (!iobuf)
> + return -ENOMEM;
> +
> ridcode = 0;
> writer = do_writerid;
>
> @@ -5991,8 +6007,8 @@
> */
> case AIROPMACON:
> if (enable_MAC(ai, &rsp) != 0)
> - return -EIO;
> - return 0;
> + ret = -EIO;
> + goto wr_free;
>
> /*
> * Evidently this code in the airo driver does not get a symbol
> @@ -6000,32 +6016,38 @@
> */
> case AIROPMACOFF:
> disable_MAC(ai);
> - return 0;
> + goto wr_free;
>
> /* This command merely clears the counts does not actually store any data
> * only reads rid. But as it changes the cards state, I put it in the
> * writerid routines.
> */
> case AIROPSTCLR:
> - PC4500_readrid(ai,RID_STATSDELTACLEAR,iobuf,sizeof(iobuf));
> + PC4500_readrid(ai,RID_STATSDELTACLEAR,iobuf,RIDS_SIZE);
>
> enabled = ai->micstats.enabled;
> memset(&ai->micstats,0,sizeof(ai->micstats));
> ai->micstats.enabled = enabled;
>
> if (copy_to_user(comp->data, iobuf,
> - min((int)comp->len, (int)sizeof(iobuf))))
> - return -EFAULT;
> - return 0;
> + min((int)comp->len, (int)RIDS_SIZE)))
> + ret = -EFAULT;
> + goto wr_free;
>
> default:
> - return -EOPNOTSUPP; /* Blarg! */
> + ret = -EOPNOTSUPP; /* Blarg! */
> + goto wr_free;
> + }
> +
> + if (comp->len > RIDS_SIZE) {
> + ret = -EINVAL;
> + goto wr_free;
> }
> - if(comp->len > sizeof(iobuf))
> - return -EINVAL;
>
> - if (copy_from_user(iobuf,comp->data,comp->len))
> - return -EFAULT;
> + if (copy_from_user(iobuf,comp->data,comp->len)) {
> + ret = -EFAULT;
> + goto wr_free;
> + }
>
> if (comp->command == AIROPCFG) {
> ConfigRid *cfg = (ConfigRid *)iobuf;
> @@ -6040,8 +6062,10 @@
> }
>
> if((*writer)(ai, ridcode, iobuf,comp->len))
> - return -EIO;
> - return 0;
> + ret = -EIO;
> +wr_free:
> + kfree(iobuf);
> + return ret;
> }
>
> /*****************************************************************************