firewire: fill_bus_reset_event needs lock protection

Callers of fill_bus_reset_event() have to take card->lock. Otherwise
access to node data may oops if node removal is in progress.

A lockless alternative would be

- event->local_node_id = card->local_node->node_id;
+ tmp = fw_node_get(card->local_node);
+ event->local_node_id = tmp->node_id;
+ fw_node_put(tmp);

and ditto with the other node pointers which fill_bus_reset_event()
accesses. But I went the locked route because one of the two callers
already holds the lock. As a bonus, we don't need the memory barrier
anymore because device->generation and device->node_id are written in
a card->lock protected section.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Signed-off-by: Kristian Høgsberg <krh@redhat.com>

+7 -2
+7 -2
drivers/firewire/fw-cdev.c
··· 205 return dequeue_event(client, buffer, count); 206 } 207 208 static void 209 fill_bus_reset_event(struct fw_cdev_event_bus_reset *event, 210 struct client *client) ··· 215 event->closure = client->bus_reset_closure; 216 event->type = FW_CDEV_EVENT_BUS_RESET; 217 event->generation = client->device->generation; 218 - smp_rmb(); /* node_id must not be older than generation */ 219 event->node_id = client->device->node_id; 220 event->local_node_id = card->local_node->node_id; 221 event->bm_node_id = 0; /* FIXME: We don't track the BM. */ ··· 274 { 275 struct fw_cdev_get_info *get_info = buffer; 276 struct fw_cdev_event_bus_reset bus_reset; 277 unsigned long ret = 0; 278 279 client->version = get_info->version; ··· 300 client->bus_reset_closure = get_info->bus_reset_closure; 301 if (get_info->bus_reset != 0) { 302 void __user *uptr = u64_to_uptr(get_info->bus_reset); 303 304 fill_bus_reset_event(&bus_reset, client); 305 if (copy_to_user(uptr, &bus_reset, sizeof(bus_reset))) 306 return -EFAULT; 307 } 308 309 - get_info->card = client->device->card->index; 310 311 return 0; 312 }
··· 205 return dequeue_event(client, buffer, count); 206 } 207 208 + /* caller must hold card->lock so that node pointers can be dereferenced here */ 209 static void 210 fill_bus_reset_event(struct fw_cdev_event_bus_reset *event, 211 struct client *client) ··· 214 event->closure = client->bus_reset_closure; 215 event->type = FW_CDEV_EVENT_BUS_RESET; 216 event->generation = client->device->generation; 217 event->node_id = client->device->node_id; 218 event->local_node_id = card->local_node->node_id; 219 event->bm_node_id = 0; /* FIXME: We don't track the BM. */ ··· 274 { 275 struct fw_cdev_get_info *get_info = buffer; 276 struct fw_cdev_event_bus_reset bus_reset; 277 + struct fw_card *card = client->device->card; 278 unsigned long ret = 0; 279 280 client->version = get_info->version; ··· 299 client->bus_reset_closure = get_info->bus_reset_closure; 300 if (get_info->bus_reset != 0) { 301 void __user *uptr = u64_to_uptr(get_info->bus_reset); 302 + unsigned long flags; 303 304 + spin_lock_irqsave(&card->lock, flags); 305 fill_bus_reset_event(&bus_reset, client); 306 + spin_unlock_irqrestore(&card->lock, flags); 307 + 308 if (copy_to_user(uptr, &bus_reset, sizeof(bus_reset))) 309 return -EFAULT; 310 } 311 312 + get_info->card = card->index; 313 314 return 0; 315 }