firewire: core: fix race at address_handler unregistration

Fix the following unlikely but possible race:

CPU 1                             CPU 2
------------------------------------------------------------------------
AR-request tasklet
    lookup handler
                                  unregister handler
				  free handler->callback_data or handler
    call handler->callback

The application which registered the handler has no way to stop nodes
sending new requests to their address range, hence cannot prevent this
race.

Fix it simply by extending the address_handler_lock-protected region
from only around the lookup to around both lookup and call.  We only
need to do so in the exclusive region handler; the FCP region handler
already holds the lock around the handler->callback call.

Alas this removes the current ability to execute the callback in
parallel on different CPUs if it was called for different FireWire cards
at the same time.  (For a single card, the handler is already
serialized.)  If this loss of a rather obscure feature is not tolerable,
a more complex fix would be required:  Add a handler reference counter;
wait in fw_core_remove_address_handler() for this conter to become zero.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 89e310ba03ba0f0eba8c116c31f3e84b5c48f2e1..190bf61533a27a21187d70b2bd84c0d7c4ff11e8 100644 (file)
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -602,6 +602,9 @@ EXPORT_SYMBOL(fw_core_add_address_handler);
 
 /**
  * fw_core_remove_address_handler() - unregister an address handler
+ *
+ * When fw_core_remove_address_handler() returns, @handler->callback() is
+ * guaranteed to not run on any CPU anymore.
  */
 void fw_core_remove_address_handler(struct fw_address_handler *handler)
 {
@@ -838,16 +841,16 @@ static void handle_exclusive_region_request(struct fw_card *card,
        spin_lock_irqsave(&address_handler_lock, flags);
        handler = lookup_enclosing_address_handler(&address_handler_list,
                                                   offset, request->length);
-       spin_unlock_irqrestore(&address_handler_lock, flags);
-
-       if (handler == NULL)
-               fw_send_response(card, request, RCODE_ADDRESS_ERROR);
-       else
+       if (handler)
                handler->address_callback(card, request,
                                          tcode, destination, source,
                                          p->generation, offset,
                                          request->data, request->length,
                                          handler->callback_data);
+       spin_unlock_irqrestore(&address_handler_lock, flags);
+
+       if (!handler)
+               fw_send_response(card, request, RCODE_ADDRESS_ERROR);
 }
 
 static void handle_fcp_region_request(struct fw_card *card,