[PATCH v3] firewire: cdev: reduce stack usage by ioctl_dispatch

From: Stefan Richter
Date: Thu Oct 15 2009 - 15:18:14 EST


Replace a hardcoded buffer size by a sizeof union {}. This shrinks the
stack-allocated ioctl argument buffer from 256 to 40 bytes. (This is
not much, but subsequent stack usage particularly by the queue_iso ioctl
handler adds up.)

The new form is also easier to keep up to date than a hardcoded size if
more ioctls are added.

Signed-off-by: Stefan Richter <stefanr@xxxxxxxxxxxxxxxxx>
---

On 15 Oct, Jonathan Cameron wrote:
[v2 with BUILD_BUG_ON() tests]
> Obviously this will work fine, but curious what advantage this have over
> the approach (used for example in some of the network drivers) of defining a
> union containing all the ioctl structs? Then you use the size of that
> to define your buffer. Thus any new larger ioctl will expand the buffer
> size as long as it's in the union.

Uh, right, that's better. Thanks for the pointer.

Plus, I realized just now that the runtime tests of _IOC_SIZE(cmd) need
to stay, regardless of how the buffer declaration is coded. So here is
yet another update.

drivers/firewire/core-cdev.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)

Index: linux-2.6.31/drivers/firewire/core-cdev.c
===================================================================
--- linux-2.6.31.orig/drivers/firewire/core-cdev.c
+++ linux-2.6.31/drivers/firewire/core-cdev.c
@@ -1299,7 +1299,23 @@ static int (* const ioctl_handlers[])(st
static int dispatch_ioctl(struct client *client,
unsigned int cmd, void __user *arg)
{
- char buffer[256];
+ char buffer[sizeof(union {
+ struct fw_cdev_get_info _00;
+ struct fw_cdev_send_request _01;
+ struct fw_cdev_allocate _02;
+ struct fw_cdev_deallocate _03;
+ struct fw_cdev_send_response _04;
+ struct fw_cdev_initiate_bus_reset _05;
+ struct fw_cdev_add_descriptor _06;
+ struct fw_cdev_remove_descriptor _07;
+ struct fw_cdev_create_iso_context _08;
+ struct fw_cdev_queue_iso _09;
+ struct fw_cdev_start_iso _0a;
+ struct fw_cdev_stop_iso _0b;
+ struct fw_cdev_get_cycle_timer _0c;
+ struct fw_cdev_allocate_iso_resource _0d;
+ struct fw_cdev_send_stream_packet _13;
+ })];
int ret;

if (_IOC_TYPE(cmd) != '#' ||


--
Stefan Richter
-=====-==--= =-=- -====
http://arcgraph.de/sr/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/