diff options
author | Stefan Richter <stefanr@s5r6.in-berlin.de> | 2008-02-24 12:57:23 -0500 |
---|---|---|
committer | Stefan Richter <stefanr@s5r6.in-berlin.de> | 2008-03-02 06:35:46 -0500 |
commit | 15803478fdea964e5f76079851fcd13068208d5d (patch) | |
tree | 153bc372845c2223ec798ff4c0f3896221fe4e97 | |
parent | f8436158b1d76e6842856048f287799468b56eb2 (diff) |
firewire: potentially invalid pointers used in fw_card_bm_work
The bus management workqueue job was in danger to dereference NULL
pointers. Also, after having temporarily lifted card->lock, a few node
pointers and a device pointer may have become invalid.
Add NULL pointer checks and get the necessary references. Also, move
card->local_node out of fw_card_bm_work's sight during shutdown of the
card.
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Signed-off-by: Jarod Wilson <jwilson@redhat.com>
-rw-r--r-- | drivers/firewire/fw-card.c | 51 | ||||
-rw-r--r-- | drivers/firewire/fw-topology.c | 1 |
2 files changed, 35 insertions, 17 deletions
diff --git a/drivers/firewire/fw-card.c b/drivers/firewire/fw-card.c index 3e9719948a8e..e6395b298508 100644 --- a/drivers/firewire/fw-card.c +++ b/drivers/firewire/fw-card.c | |||
@@ -214,17 +214,29 @@ static void | |||
214 | fw_card_bm_work(struct work_struct *work) | 214 | fw_card_bm_work(struct work_struct *work) |
215 | { | 215 | { |
216 | struct fw_card *card = container_of(work, struct fw_card, work.work); | 216 | struct fw_card *card = container_of(work, struct fw_card, work.work); |
217 | struct fw_device *root; | 217 | struct fw_device *root_device; |
218 | struct fw_node *root_node, *local_node; | ||
218 | struct bm_data bmd; | 219 | struct bm_data bmd; |
219 | unsigned long flags; | 220 | unsigned long flags; |
220 | int root_id, new_root_id, irm_id, gap_count, generation, grace; | 221 | int root_id, new_root_id, irm_id, gap_count, generation, grace; |
221 | int do_reset = 0; | 222 | int do_reset = 0; |
222 | 223 | ||
223 | spin_lock_irqsave(&card->lock, flags); | 224 | spin_lock_irqsave(&card->lock, flags); |
225 | local_node = card->local_node; | ||
226 | root_node = card->root_node; | ||
227 | |||
228 | if (local_node == NULL) { | ||
229 | spin_unlock_irqrestore(&card->lock, flags); | ||
230 | return; | ||
231 | } | ||
232 | fw_node_get(local_node); | ||
233 | fw_node_get(root_node); | ||
224 | 234 | ||
225 | generation = card->generation; | 235 | generation = card->generation; |
226 | root = card->root_node->data; | 236 | root_device = root_node->data; |
227 | root_id = card->root_node->node_id; | 237 | if (root_device) |
238 | fw_device_get(root_device); | ||
239 | root_id = root_node->node_id; | ||
228 | grace = time_after(jiffies, card->reset_jiffies + DIV_ROUND_UP(HZ, 10)); | 240 | grace = time_after(jiffies, card->reset_jiffies + DIV_ROUND_UP(HZ, 10)); |
229 | 241 | ||
230 | if (card->bm_generation + 1 == generation || | 242 | if (card->bm_generation + 1 == generation || |
@@ -243,14 +255,14 @@ fw_card_bm_work(struct work_struct *work) | |||
243 | 255 | ||
244 | irm_id = card->irm_node->node_id; | 256 | irm_id = card->irm_node->node_id; |
245 | if (!card->irm_node->link_on) { | 257 | if (!card->irm_node->link_on) { |
246 | new_root_id = card->local_node->node_id; | 258 | new_root_id = local_node->node_id; |
247 | fw_notify("IRM has link off, making local node (%02x) root.\n", | 259 | fw_notify("IRM has link off, making local node (%02x) root.\n", |
248 | new_root_id); | 260 | new_root_id); |
249 | goto pick_me; | 261 | goto pick_me; |
250 | } | 262 | } |
251 | 263 | ||
252 | bmd.lock.arg = cpu_to_be32(0x3f); | 264 | bmd.lock.arg = cpu_to_be32(0x3f); |
253 | bmd.lock.data = cpu_to_be32(card->local_node->node_id); | 265 | bmd.lock.data = cpu_to_be32(local_node->node_id); |
254 | 266 | ||
255 | spin_unlock_irqrestore(&card->lock, flags); | 267 | spin_unlock_irqrestore(&card->lock, flags); |
256 | 268 | ||
@@ -267,12 +279,12 @@ fw_card_bm_work(struct work_struct *work) | |||
267 | * Another bus reset happened. Just return, | 279 | * Another bus reset happened. Just return, |
268 | * the BM work has been rescheduled. | 280 | * the BM work has been rescheduled. |
269 | */ | 281 | */ |
270 | return; | 282 | goto out; |
271 | } | 283 | } |
272 | 284 | ||
273 | if (bmd.rcode == RCODE_COMPLETE && bmd.old != 0x3f) | 285 | if (bmd.rcode == RCODE_COMPLETE && bmd.old != 0x3f) |
274 | /* Somebody else is BM, let them do the work. */ | 286 | /* Somebody else is BM, let them do the work. */ |
275 | return; | 287 | goto out; |
276 | 288 | ||
277 | spin_lock_irqsave(&card->lock, flags); | 289 | spin_lock_irqsave(&card->lock, flags); |
278 | if (bmd.rcode != RCODE_COMPLETE) { | 290 | if (bmd.rcode != RCODE_COMPLETE) { |
@@ -282,7 +294,7 @@ fw_card_bm_work(struct work_struct *work) | |||
282 | * do a bus reset and pick the local node as | 294 | * do a bus reset and pick the local node as |
283 | * root, and thus, IRM. | 295 | * root, and thus, IRM. |
284 | */ | 296 | */ |
285 | new_root_id = card->local_node->node_id; | 297 | new_root_id = local_node->node_id; |
286 | fw_notify("BM lock failed, making local node (%02x) root.\n", | 298 | fw_notify("BM lock failed, making local node (%02x) root.\n", |
287 | new_root_id); | 299 | new_root_id); |
288 | goto pick_me; | 300 | goto pick_me; |
@@ -295,7 +307,7 @@ fw_card_bm_work(struct work_struct *work) | |||
295 | */ | 307 | */ |
296 | spin_unlock_irqrestore(&card->lock, flags); | 308 | spin_unlock_irqrestore(&card->lock, flags); |
297 | schedule_delayed_work(&card->work, DIV_ROUND_UP(HZ, 10)); | 309 | schedule_delayed_work(&card->work, DIV_ROUND_UP(HZ, 10)); |
298 | return; | 310 | goto out; |
299 | } | 311 | } |
300 | 312 | ||
301 | /* | 313 | /* |
@@ -305,20 +317,20 @@ fw_card_bm_work(struct work_struct *work) | |||
305 | */ | 317 | */ |
306 | card->bm_generation = generation; | 318 | card->bm_generation = generation; |
307 | 319 | ||
308 | if (root == NULL) { | 320 | if (root_device == NULL) { |
309 | /* | 321 | /* |
310 | * Either link_on is false, or we failed to read the | 322 | * Either link_on is false, or we failed to read the |
311 | * config rom. In either case, pick another root. | 323 | * config rom. In either case, pick another root. |
312 | */ | 324 | */ |
313 | new_root_id = card->local_node->node_id; | 325 | new_root_id = local_node->node_id; |
314 | } else if (atomic_read(&root->state) != FW_DEVICE_RUNNING) { | 326 | } else if (atomic_read(&root_device->state) != FW_DEVICE_RUNNING) { |
315 | /* | 327 | /* |
316 | * If we haven't probed this device yet, bail out now | 328 | * If we haven't probed this device yet, bail out now |
317 | * and let's try again once that's done. | 329 | * and let's try again once that's done. |
318 | */ | 330 | */ |
319 | spin_unlock_irqrestore(&card->lock, flags); | 331 | spin_unlock_irqrestore(&card->lock, flags); |
320 | return; | 332 | goto out; |
321 | } else if (root->config_rom[2] & BIB_CMC) { | 333 | } else if (root_device->config_rom[2] & BIB_CMC) { |
322 | /* | 334 | /* |
323 | * FIXME: I suppose we should set the cmstr bit in the | 335 | * FIXME: I suppose we should set the cmstr bit in the |
324 | * STATE_CLEAR register of this node, as described in | 336 | * STATE_CLEAR register of this node, as described in |
@@ -332,7 +344,7 @@ fw_card_bm_work(struct work_struct *work) | |||
332 | * successfully read the config rom, but it's not | 344 | * successfully read the config rom, but it's not |
333 | * cycle master capable. | 345 | * cycle master capable. |
334 | */ | 346 | */ |
335 | new_root_id = card->local_node->node_id; | 347 | new_root_id = local_node->node_id; |
336 | } | 348 | } |
337 | 349 | ||
338 | pick_me: | 350 | pick_me: |
@@ -341,8 +353,8 @@ fw_card_bm_work(struct work_struct *work) | |||
341 | * the typically much larger 1394b beta repeater delays though. | 353 | * the typically much larger 1394b beta repeater delays though. |
342 | */ | 354 | */ |
343 | if (!card->beta_repeaters_present && | 355 | if (!card->beta_repeaters_present && |
344 | card->root_node->max_hops < ARRAY_SIZE(gap_count_table)) | 356 | root_node->max_hops < ARRAY_SIZE(gap_count_table)) |
345 | gap_count = gap_count_table[card->root_node->max_hops]; | 357 | gap_count = gap_count_table[root_node->max_hops]; |
346 | else | 358 | else |
347 | gap_count = 63; | 359 | gap_count = 63; |
348 | 360 | ||
@@ -364,6 +376,11 @@ fw_card_bm_work(struct work_struct *work) | |||
364 | fw_send_phy_config(card, new_root_id, generation, gap_count); | 376 | fw_send_phy_config(card, new_root_id, generation, gap_count); |
365 | fw_core_initiate_bus_reset(card, 1); | 377 | fw_core_initiate_bus_reset(card, 1); |
366 | } | 378 | } |
379 | out: | ||
380 | if (root_device) | ||
381 | fw_device_put(root_device); | ||
382 | fw_node_put(root_node); | ||
383 | fw_node_put(local_node); | ||
367 | } | 384 | } |
368 | 385 | ||
369 | static void | 386 | static void |
diff --git a/drivers/firewire/fw-topology.c b/drivers/firewire/fw-topology.c index 172c1867e9aa..e47bb040197a 100644 --- a/drivers/firewire/fw-topology.c +++ b/drivers/firewire/fw-topology.c | |||
@@ -383,6 +383,7 @@ void fw_destroy_nodes(struct fw_card *card) | |||
383 | card->color++; | 383 | card->color++; |
384 | if (card->local_node != NULL) | 384 | if (card->local_node != NULL) |
385 | for_each_fw_node(card, card->local_node, report_lost_node); | 385 | for_each_fw_node(card, card->local_node, report_lost_node); |
386 | card->local_node = NULL; | ||
386 | spin_unlock_irqrestore(&card->lock, flags); | 387 | spin_unlock_irqrestore(&card->lock, flags); |
387 | } | 388 | } |
388 | 389 | ||