firewire: avoid memleak after phy config transmit failure
authorStefan Richter <stefanr@s5r6.in-berlin.de>
Tue, 22 Jul 2008 19:35:47 +0000 (21:35 +0200)
committerStefan Richter <stefanr@s5r6.in-berlin.de>
Fri, 25 Jul 2008 18:10:32 +0000 (20:10 +0200)
Use only statically allocated data for PHY config packet transmission.
With the previous incarnation, some data wouldn't be freed if the packet
transmit callback was never called.

A theoretical drawback now is that, in PCs with more than one card,
card A may complete() for a waiter on card B.  But this is highly
unlikely and its impact not serious.  Bus manager B may reset bus B
before the PHY config went out, but the next phy config on B should be
fine.  However, with a timeout of 100ms, this situation is close to
impossible.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
drivers/firewire/fw-transaction.c

index 861dd60de7d9e09b451c1861b426a1dd24d29c55..e5d1a0b64fcf1b714a186d31a8b7aabf90e7aa97 100644 (file)
@@ -22,6 +22,7 @@
 #include <linux/kernel.h>
 #include <linux/kref.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/pci.h>
@@ -295,58 +296,41 @@ fw_send_request(struct fw_card *card, struct fw_transaction *t,
 }
 EXPORT_SYMBOL(fw_send_request);
 
-struct fw_phy_packet {
-       struct fw_packet packet;
-       struct completion done;
-       struct kref kref;
-};
-
-static void phy_packet_release(struct kref *kref)
-{
-       struct fw_phy_packet *p =
-                       container_of(kref, struct fw_phy_packet, kref);
-       kfree(p);
-}
+static DEFINE_MUTEX(phy_config_mutex);
+static DECLARE_COMPLETION(phy_config_done);
 
 static void transmit_phy_packet_callback(struct fw_packet *packet,
                                         struct fw_card *card, int status)
 {
-       struct fw_phy_packet *p =
-                       container_of(packet, struct fw_phy_packet, packet);
-
-       complete(&p->done);
-       kref_put(&p->kref, phy_packet_release);
+       complete(&phy_config_done);
 }
 
+static struct fw_packet phy_config_packet = {
+       .header_length  = 8,
+       .payload_length = 0,
+       .speed          = SCODE_100,
+       .callback       = transmit_phy_packet_callback,
+};
+
 void fw_send_phy_config(struct fw_card *card,
                        int node_id, int generation, int gap_count)
 {
-       struct fw_phy_packet *p;
        long timeout = DIV_ROUND_UP(HZ, 10);
        u32 data = PHY_IDENTIFIER(PHY_PACKET_CONFIG) |
                   PHY_CONFIG_ROOT_ID(node_id) |
                   PHY_CONFIG_GAP_COUNT(gap_count);
 
-       p = kmalloc(sizeof(*p), GFP_KERNEL);
-       if (p == NULL)
-               return;
+       mutex_lock(&phy_config_mutex);
+
+       phy_config_packet.header[0] = data;
+       phy_config_packet.header[1] = ~data;
+       phy_config_packet.generation = generation;
+       INIT_COMPLETION(phy_config_done);
+
+       card->driver->send_request(card, &phy_config_packet);
+       wait_for_completion_timeout(&phy_config_done, timeout);
 
-       p->packet.header[0] = data;
-       p->packet.header[1] = ~data;
-       p->packet.header_length = 8;
-       p->packet.payload_length = 0;
-       p->packet.speed = SCODE_100;
-       p->packet.generation = generation;
-       p->packet.callback = transmit_phy_packet_callback;
-       init_completion(&p->done);
-       kref_set(&p->kref, 2);
-
-       card->driver->send_request(card, &p->packet);
-       timeout = wait_for_completion_timeout(&p->done, timeout);
-       kref_put(&p->kref, phy_packet_release);
-
-       /* will leak p if the callback is never executed */
-       WARN_ON(timeout == 0);
+       mutex_unlock(&phy_config_mutex);
 }
 
 void fw_flush_transactions(struct fw_card *card)