summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorStefan Richter <stefanr@s5r6.in-berlin.de>2010-01-24 16:45:03 +0100
committerStefan Richter <stefanr@s5r6.in-berlin.de>2010-01-26 20:54:50 +0100
commit281e20323ab72180137824a298ee9e21e6f9acf6 (patch)
treeb807edaafec3c00e442d41d9091b9783a53820f9
parent6d3faf6f431bafb25f4b9926c50a7e5c267738c6 (diff)
downloadop-kernel-dev-281e20323ab72180137824a298ee9e21e6f9acf6.zip
op-kernel-dev-281e20323ab72180137824a298ee9e21e6f9acf6.tar.gz
firewire: core: fix use-after-free regression in FCP handler
Commit db5d247a "firewire: fix use of multiple AV/C devices, allow multiple FCP listeners" introduced a regression into 2.6.33-rc3: The core freed payloads of incoming requests to FCP_Request or FCP_Response before a userspace driver accessed them. We need to copy such payloads for each registered userspace client and free the copies according to the lifetime rules of non-FCP client request resources. (This could possibly be optimized by reference counts instead of copies.) The presently only kernelspace driver which listens for FCP requests, firedtv, was not affected because it already copies FCP frames into an own buffer before returning to firewire-core's FCP handler dispatcher. Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
-rw-r--r--drivers/firewire/core-cdev.c50
1 files changed, 36 insertions, 14 deletions
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index e6d63849..4eeaed5 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -35,6 +35,7 @@
#include <linux/preempt.h>
#include <linux/sched.h>
#include <linux/spinlock.h>
+#include <linux/string.h>
#include <linux/time.h>
#include <linux/uaccess.h>
#include <linux/vmalloc.h>
@@ -595,13 +596,20 @@ static int ioctl_send_request(struct client *client, void *buffer)
client->device->max_speed);
}
+static inline bool is_fcp_request(struct fw_request *request)
+{
+ return request == NULL;
+}
+
static void release_request(struct client *client,
struct client_resource *resource)
{
struct inbound_transaction_resource *r = container_of(resource,
struct inbound_transaction_resource, resource);
- if (r->request)
+ if (is_fcp_request(r->request))
+ kfree(r->data);
+ else
fw_send_response(client->device->card, r->request,
RCODE_CONFLICT_ERROR);
kfree(r);
@@ -616,6 +624,7 @@ static void handle_request(struct fw_card *card, struct fw_request *request,
struct address_handler_resource *handler = callback_data;
struct inbound_transaction_resource *r;
struct inbound_transaction_event *e;
+ void *fcp_frame = NULL;
int ret;
r = kmalloc(sizeof(*r), GFP_ATOMIC);
@@ -627,6 +636,18 @@ static void handle_request(struct fw_card *card, struct fw_request *request,
r->data = payload;
r->length = length;
+ if (is_fcp_request(request)) {
+ /*
+ * FIXME: Let core-transaction.c manage a
+ * single reference-counted copy?
+ */
+ fcp_frame = kmemdup(payload, length, GFP_ATOMIC);
+ if (fcp_frame == NULL)
+ goto failed;
+
+ r->data = fcp_frame;
+ }
+
r->resource.release = release_request;
ret = add_client_resource(handler->client, &r->resource, GFP_ATOMIC);
if (ret < 0)
@@ -640,13 +661,15 @@ static void handle_request(struct fw_card *card, struct fw_request *request,
e->request.closure = handler->closure;
queue_event(handler->client, &e->event,
- &e->request, sizeof(e->request), payload, length);
+ &e->request, sizeof(e->request), r->data, length);
return;
failed:
kfree(r);
kfree(e);
- if (request)
+ kfree(fcp_frame);
+
+ if (!is_fcp_request(request))
fw_send_response(card, request, RCODE_CONFLICT_ERROR);
}
@@ -717,18 +740,17 @@ static int ioctl_send_response(struct client *client, void *buffer)
r = container_of(resource, struct inbound_transaction_resource,
resource);
- if (r->request) {
- if (request->length < r->length)
- r->length = request->length;
- if (copy_from_user(r->data, u64_to_uptr(request->data),
- r->length)) {
- ret = -EFAULT;
- kfree(r->request);
- goto out;
- }
- fw_send_response(client->device->card, r->request,
- request->rcode);
+ if (is_fcp_request(r->request))
+ goto out;
+
+ if (request->length < r->length)
+ r->length = request->length;
+ if (copy_from_user(r->data, u64_to_uptr(request->data), r->length)) {
+ ret = -EFAULT;
+ kfree(r->request);
+ goto out;
}
+ fw_send_response(client->device->card, r->request, request->rcode);
out:
kfree(r);
OpenPOWER on IntegriCloud