linux-kernel - [PATCH v3] firewire: cdev: reduce stack usage by ioctl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <tkrat.01048c8f2bd44ae6@s5r6.in-berlin.de>
Date:	Thu, 15 Oct 2009 21:16:53 +0200 (CEST)
From:	Stefan Richter <stefanr@...6.in-berlin.de>
To:	linux1394-devel@...ts.sourceforge.net
cc:	Jonathan Cameron <jic23@....ac.uk>, linux-kernel@...r.kernel.org
Subject: [PATCH v3] firewire: cdev: reduce stack usage by ioctl_dispatch

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@...6.in-berlin.de>
---

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/