Skip to contentSkip to conversation

ds: reduce stack usage in ds_ioctl

PATCH In review Randy.Dunlap 1 participant 2 messages Oct 13, 2004 Pcmcia

lore
Share & tools
lore

Randy.Dunlap

author
#0 lore raw

PATCH 1/1 ds: reduce stack usage in ds_ioctl

Hi Russell,
Please consider for after 2.6.9.

Signature · 1 line
~Randy

Reduce stack usage from 696 (0x2b8) to 24 (0x18) (on x86-32).

Signed-off-by Randy Dunlap <[email protected]>

diffstat:=

 drivers/pcmcia/ds.c |   79 ++++++++++++++++++++++++++++++----------------------
 1 files changed, 46 insertions(+), 33 deletions(-)
drivers/pcmcia/ds.c renamed from drivers/pcmcia/ds.c~ds_ioctl +46 -33
diff -Naurp ./drivers/pcmcia/ds.c~ds_ioctl ./drivers/pcmcia/ds.c
@@ -892,7 +892,7 @@ static int ds_ioctl(struct inode * inode
     void __user *uarg = (char __user *)arg;
     u_int size;
     int ret, err;
-    ds_ioctl_arg_t buf;
+    ds_ioctl_arg_t *buf;
     user_info_t *user;
 
     ds_dbg(2, "ds_ioctl(socket %d, %#x, %#lx)\n", iminor(inode), cmd, arg);

@@ -926,46 +926,49 @@ static int ds_ioctl(struct inode * inode
 	    return err;
 	}
     }
+    buf = kmalloc(sizeof(ds_ioctl_arg_t), GFP_KERNEL);
+    if (!buf)
+	return -ENOMEM;
     
     err = ret = 0;
     
-    if (cmd & IOC_IN) __copy_from_user((char *)&buf, uarg, size);
+    if (cmd & IOC_IN) __copy_from_user((char *)buf, uarg, size);
     
     switch (cmd) {
     case DS_ADJUST_RESOURCE_INFO:
-	ret = pcmcia_adjust_resource_info(s->handle, &buf.adjust);
+	ret = pcmcia_adjust_resource_info(s->handle, &buf->adjust);
 	break;
     case DS_GET_CARD_SERVICES_INFO:
-	ret = pcmcia_get_card_services_info(&buf.servinfo);
+	ret = pcmcia_get_card_services_info(&buf->servinfo);
 	break;
     case DS_GET_CONFIGURATION_INFO:
-	ret = pcmcia_get_configuration_info(s->handle, &buf.config);
+	ret = pcmcia_get_configuration_info(s->handle, &buf->config);
 	break;
     case DS_GET_FIRST_TUPLE:
 	pcmcia_validate_mem(s->parent);
-	ret = pcmcia_get_first_tuple(s->handle, &buf.tuple);
+	ret = pcmcia_get_first_tuple(s->handle, &buf->tuple);
 	break;
     case DS_GET_NEXT_TUPLE:
-	ret = pcmcia_get_next_tuple(s->handle, &buf.tuple);
+	ret = pcmcia_get_next_tuple(s->handle, &buf->tuple);
 	break;
     case DS_GET_TUPLE_DATA:
-	buf.tuple.TupleData = buf.tuple_parse.data;
-	buf.tuple.TupleDataMax = sizeof(buf.tuple_parse.data);
-	ret = pcmcia_get_tuple_data(s->handle, &buf.tuple);
+	buf->tuple.TupleData = buf->tuple_parse.data;
+	buf->tuple.TupleDataMax = sizeof(buf->tuple_parse.data);
+	ret = pcmcia_get_tuple_data(s->handle, &buf->tuple);
 	break;
     case DS_PARSE_TUPLE:
-	buf.tuple.TupleData = buf.tuple_parse.data;
-	ret = pcmcia_parse_tuple(s->handle, &buf.tuple, &buf.tuple_parse.parse);
+	buf->tuple.TupleData = buf->tuple_parse.data;
+	ret = pcmcia_parse_tuple(s->handle, &buf->tuple, &buf->tuple_parse.parse);
 	break;
     case DS_RESET_CARD:
 	ret = pcmcia_reset_card(s->handle, NULL);
 	break;
     case DS_GET_STATUS:
-	ret = pcmcia_get_status(s->handle, &buf.status);
+	ret = pcmcia_get_status(s->handle, &buf->status);
 	break;
     case DS_VALIDATE_CIS:
 	pcmcia_validate_mem(s->parent);
-	ret = pcmcia_validate_cis(s->handle, &buf.cisinfo);
+	ret = pcmcia_validate_cis(s->handle, &buf->cisinfo);
 	break;
     case DS_SUSPEND_CARD:
 	ret = pcmcia_suspend_card(s->parent);

@@ -980,46 +983,54 @@ static int ds_ioctl(struct inode * inode
 	err = pcmcia_insert_card(s->parent);
 	break;
     case DS_ACCESS_CONFIGURATION_REGISTER:
-	if ((buf.conf_reg.Action == CS_WRITE) && !capable(CAP_SYS_ADMIN))
-	    return -EPERM;
-	ret = pcmcia_access_configuration_register(s->handle, &buf.conf_reg);
+	if ((buf->conf_reg.Action == CS_WRITE) && !capable(CAP_SYS_ADMIN)) {
+	    err = -EPERM;
+	    goto free_out;
+	}
+	ret = pcmcia_access_configuration_register(s->handle, &buf->conf_reg);
 	break;
     case DS_GET_FIRST_REGION:
-        ret = pcmcia_get_first_region(s->handle, &buf.region);
+        ret = pcmcia_get_first_region(s->handle, &buf->region);
 	break;
     case DS_GET_NEXT_REGION:
-	ret = pcmcia_get_next_region(s->handle, &buf.region);
+	ret = pcmcia_get_next_region(s->handle, &buf->region);
 	break;
     case DS_GET_FIRST_WINDOW:
-	buf.win_info.handle = (window_handle_t)s->handle;
-	ret = pcmcia_get_first_window(&buf.win_info.handle, &buf.win_info.window);
+	buf->win_info.handle = (window_handle_t)s->handle;
+	ret = pcmcia_get_first_window(&buf->win_info.handle, &buf->win_info.window);
 	break;
     case DS_GET_NEXT_WINDOW:
-	ret = pcmcia_get_next_window(&buf.win_info.handle, &buf.win_info.window);
+	ret = pcmcia_get_next_window(&buf->win_info.handle, &buf->win_info.window);
 	break;
     case DS_GET_MEM_PAGE:
-	ret = pcmcia_get_mem_page(buf.win_info.handle,
-			   &buf.win_info.map);
+	ret = pcmcia_get_mem_page(buf->win_info.handle,
+			   &buf->win_info.map);
 	break;
     case DS_REPLACE_CIS:
-	ret = pcmcia_replace_cis(s->handle, &buf.cisdump);
+	ret = pcmcia_replace_cis(s->handle, &buf->cisdump);
 	break;
     case DS_BIND_REQUEST:
-	if (!capable(CAP_SYS_ADMIN)) return -EPERM;
-	err = bind_request(s, &buf.bind_info);
+	if (!capable(CAP_SYS_ADMIN)) {
+	    err = -EPERM;
+	    goto free_out;
+	}
+	err = bind_request(s, &buf->bind_info);
 	break;
     case DS_GET_DEVICE_INFO:
-	err = get_device_info(s, &buf.bind_info, 1);
+	err = get_device_info(s, &buf->bind_info, 1);
 	break;
     case DS_GET_NEXT_DEVICE:
-	err = get_device_info(s, &buf.bind_info, 0);
+	err = get_device_info(s, &buf->bind_info, 0);
 	break;
     case DS_UNBIND_REQUEST:
-	err = unbind_request(s, &buf.bind_info);
+	err = unbind_request(s, &buf->bind_info);
 	break;
     case DS_BIND_MTD:
-	if (!capable(CAP_SYS_ADMIN)) return -EPERM;
-	err = bind_mtd(s, &buf.mtd_info);
+	if (!capable(CAP_SYS_ADMIN)) {
+	    err = -EPERM;
+	    goto free_out;
+	}
+	err = bind_mtd(s, &buf->mtd_info);
 	break;
     default:
 	err = -EINVAL;

@@ -1046,8 +1057,10 @@ static int ds_ioctl(struct inode * inode
 	}
     }
 
-    if (cmd & IOC_OUT) __copy_to_user(uarg, (char *)&buf, size);
+    if (cmd & IOC_OUT) __copy_to_user(uarg, (char *)buf, size);
 
+free_out:
+    kfree(buf);
     return err;
 } /* ds_ioctl */
author
#1 lore raw

Hi,

I've updated this patch to apply after all other changes to drivers/pcmcia/ds.c that are in current PCMCIA patchsets at
http://pcmcia.arm.linux.org.uk/

It can be applied after patchset 16, as part of patchset 17 or 18.

Signature · 1 line
~Randy

Reduce stack usage from 696 (0x2b8) to 24 (0x18) (on x86-32).

Signed-off-by Randy Dunlap <[email protected]>

diffstat:=

 drivers/pcmcia/ds.c |   93 ++++++++++++++++++++++++++++++----------------------
 1 files changed, 55 insertions(+), 38 deletions(-)
drivers/pcmcia/ds.c renamed from drivers/pcmcia/ds.c~ds_ioctl +55 -38
diff -Naurp ./drivers/pcmcia/ds.c~ds_ioctl ./drivers/pcmcia/ds.c
@@ -895,7 +895,7 @@ static int ds_ioctl(struct inode * inode
     void __user *uarg = (char __user *)arg;
     u_int size;
     int ret, err;
-    ds_ioctl_arg_t buf;
+    ds_ioctl_arg_t *buf;
     user_info_t *user;
 
     ds_dbg(2, "ds_ioctl(socket %d, %#x, %#lx)\n", iminor(inode), cmd, arg);

@@ -929,54 +929,58 @@ static int ds_ioctl(struct inode * inode
 	    return err;
 	}
     }
+    buf = kmalloc(sizeof(ds_ioctl_arg_t), GFP_KERNEL);
+    if (!buf)
+	return -ENOMEM;
     
     err = ret = 0;
     
-    if (cmd & IOC_IN) __copy_from_user((char *)&buf, uarg, size);
+    if (cmd & IOC_IN) __copy_from_user((char *)buf, uarg, size);
     
     switch (cmd) {
     case DS_ADJUST_RESOURCE_INFO:
-	ret = pcmcia_adjust_resource_info(&buf.adjust);
+	ret = pcmcia_adjust_resource_info(&buf->adjust);
 	break;
     case DS_GET_CARD_SERVICES_INFO:
-	ret = pcmcia_get_card_services_info(&buf.servinfo);
+	ret = pcmcia_get_card_services_info(&buf->servinfo);
 	break;
     case DS_GET_CONFIGURATION_INFO:
-	if (buf.config.Function && 
-	   (buf.config.Function >= s->parent->functions))
+	if (buf->config.Function && 
+	   (buf->config.Function >= s->parent->functions))
 	    ret = CS_BAD_ARGS;
 	else
-	    ret = pccard_get_configuration_info(s->parent, buf.config.Function, &buf.config);
+	    ret = pccard_get_configuration_info(s->parent,
+			buf->config.Function, &buf->config);
 	break;
     case DS_GET_FIRST_TUPLE:
 	pcmcia_validate_mem(s->parent);
-	ret = pccard_get_first_tuple(s->parent, BIND_FN_ALL, &buf.tuple);
+	ret = pccard_get_first_tuple(s->parent, BIND_FN_ALL, &buf->tuple);
 	break;
     case DS_GET_NEXT_TUPLE:
-	ret = pccard_get_next_tuple(s->parent, BIND_FN_ALL, &buf.tuple);
+	ret = pccard_get_next_tuple(s->parent, BIND_FN_ALL, &buf->tuple);
 	break;
     case DS_GET_TUPLE_DATA:
-	buf.tuple.TupleData = buf.tuple_parse.data;
-	buf.tuple.TupleDataMax = sizeof(buf.tuple_parse.data);
-	ret = pccard_get_tuple_data(s->parent, &buf.tuple);
+	buf->tuple.TupleData = buf->tuple_parse.data;
+	buf->tuple.TupleDataMax = sizeof(buf->tuple_parse.data);
+	ret = pccard_get_tuple_data(s->parent, &buf->tuple);
 	break;
     case DS_PARSE_TUPLE:
-	buf.tuple.TupleData = buf.tuple_parse.data;
-	ret = pccard_parse_tuple(&buf.tuple, &buf.tuple_parse.parse);
+	buf->tuple.TupleData = buf->tuple_parse.data;
+	ret = pccard_parse_tuple(&buf->tuple, &buf->tuple_parse.parse);
 	break;
     case DS_RESET_CARD:
 	ret = pccard_reset_card(s->parent);
 	break;
     case DS_GET_STATUS:
-	if (buf.status.Function && 
-	   (buf.status.Function >= s->parent->functions))
+	if (buf->status.Function && 
+	   (buf->status.Function >= s->parent->functions))
 	    ret = CS_BAD_ARGS;
 	else
-	ret = pccard_get_status(s->parent, buf.status.Function, &buf.status);
+	ret = pccard_get_status(s->parent, buf->status.Function, &buf->status);
 	break;
     case DS_VALIDATE_CIS:
 	pcmcia_validate_mem(s->parent);
-	ret = pccard_validate_cis(s->parent, BIND_FN_ALL, &buf.cisinfo);
+	ret = pccard_validate_cis(s->parent, BIND_FN_ALL, &buf->cisinfo);
 	break;
     case DS_SUSPEND_CARD:
 	ret = pcmcia_suspend_card(s->parent);

@@ -991,49 +995,60 @@ static int ds_ioctl(struct inode * inode
 	err = pcmcia_insert_card(s->parent);
 	break;
     case DS_ACCESS_CONFIGURATION_REGISTER:
-	if ((buf.conf_reg.Action == CS_WRITE) && !capable(CAP_SYS_ADMIN))
-	    return -EPERM;
-	if (buf.conf_reg.Function && 
-	   (buf.conf_reg.Function >= s->parent->functions))
+	if ((buf->conf_reg.Action == CS_WRITE) && !capable(CAP_SYS_ADMIN)) {
+	    err = -EPERM;
+	    goto free_out;
+	}
+	if (buf->conf_reg.Function && 
+	   (buf->conf_reg.Function >= s->parent->functions))
 	    ret = CS_BAD_ARGS;
 	else
-	    ret = pccard_access_configuration_register(s->parent, buf.conf_reg.Function, &buf.conf_reg);
+	    ret = pccard_access_configuration_register(s->parent,
+			buf->conf_reg.Function, &buf->conf_reg);
 	break;
     case DS_GET_FIRST_REGION:
-        ret = pccard_get_first_region(s->parent, &buf.region);
+	ret = pccard_get_first_region(s->parent, &buf->region);
 	break;
     case DS_GET_NEXT_REGION:
-	ret = pccard_get_next_region(s->parent, &buf.region);
+	ret = pccard_get_next_region(s->parent, &buf->region);
 	break;
     case DS_GET_FIRST_WINDOW:
-	ret = pcmcia_get_window(s->parent, &buf.win_info.handle, 0, &buf.win_info.window);
+	ret = pcmcia_get_window(s->parent, &buf->win_info.handle, 0,
+			&buf->win_info.window);
 	break;
     case DS_GET_NEXT_WINDOW:
-	ret = pcmcia_get_window(s->parent, &buf.win_info.handle, buf.win_info.handle->index + 1, &buf.win_info.window);
+	ret = pcmcia_get_window(s->parent, &buf->win_info.handle,
+		buf->win_info.handle->index + 1, &buf->win_info.window);
 	break;
     case DS_GET_MEM_PAGE:
-	ret = pcmcia_get_mem_page(buf.win_info.handle,
-			   &buf.win_info.map);
+	ret = pcmcia_get_mem_page(buf->win_info.handle,
+			   &buf->win_info.map);
 	break;
     case DS_REPLACE_CIS:
-	ret = pcmcia_replace_cis(s->parent, &buf.cisdump);
+	ret = pcmcia_replace_cis(s->parent, &buf->cisdump);
 	break;
     case DS_BIND_REQUEST:
-	if (!capable(CAP_SYS_ADMIN)) return -EPERM;
-	err = bind_request(s, &buf.bind_info);
+	if (!capable(CAP_SYS_ADMIN)) {
+	    err = -EPERM;
+	    goto free_out;
+	}
+	err = bind_request(s, &buf->bind_info);
 	break;
     case DS_GET_DEVICE_INFO:
-	err = get_device_info(s, &buf.bind_info, 1);
+	err = get_device_info(s, &buf->bind_info, 1);
 	break;
     case DS_GET_NEXT_DEVICE:
-	err = get_device_info(s, &buf.bind_info, 0);
+	err = get_device_info(s, &buf->bind_info, 0);
 	break;
     case DS_UNBIND_REQUEST:
-	err = unbind_request(s, &buf.bind_info);
+	err = unbind_request(s, &buf->bind_info);
 	break;
     case DS_BIND_MTD:
-	if (!capable(CAP_SYS_ADMIN)) return -EPERM;
-	err = bind_mtd(s, &buf.mtd_info);
+	if (!capable(CAP_SYS_ADMIN)) {
+	    err = -EPERM;
+	    goto free_out;
+	}
+	err = bind_mtd(s, &buf->mtd_info);
 	break;
     default:
 	err = -EINVAL;

@@ -1060,8 +1075,10 @@ static int ds_ioctl(struct inode * inode
 	}
     }
 
-    if (cmd & IOC_OUT) __copy_to_user(uarg, (char *)&buf, size);
+    if (cmd & IOC_OUT) __copy_to_user(uarg, (char *)buf, size);
 
+free_out:
+    kfree(buf);
     return err;
 } /* ds_ioctl */