Commit f71f94845e0126884eca8ce57a92e30b189c8e71
Committed by
Linus Torvalds
Linus Torvalds
1 parent
fbfe9c847e
Exists in
master
and in
38 other branches
um: fix oopsable race in line_close()
tty->count is decremented only after ->close() had been called and several tasks can hit it in parallel. As the result, using tty->count to check if you are the last one is broken. We end up leaving line->tty not reset to NULL and the next IRQ on that sucker will blow up trying to dereference pointers from kfree'd struct tty. Fix is obvious: we need to use a counter of our own. Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Richard Weinberger <richard@nod.at> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Showing 2 changed files with 13 additions and 13 deletions Side-by-side Diff
arch/um/drivers/line.c
| ... | ... | @@ -399,8 +399,8 @@ |
| 399 | 399 | * is done under a spinlock. Checking whether the device is in use is |
| 400 | 400 | * line->tty->count > 1, also under the spinlock. |
| 401 | 401 | * |
| 402 | - * tty->count serves to decide whether the device should be enabled or | |
| 403 | - * disabled on the host. If it's equal to 1, then we are doing the | |
| 402 | + * line->count serves to decide whether the device should be enabled or | |
| 403 | + * disabled on the host. If it's equal to 0, then we are doing the | |
| 404 | 404 | * first open or last close. Otherwise, open and close just return. |
| 405 | 405 | */ |
| 406 | 406 | |
| 407 | 407 | |
| 408 | 408 | |
| 409 | 409 | |
| ... | ... | @@ -414,16 +414,16 @@ |
| 414 | 414 | goto out_unlock; |
| 415 | 415 | |
| 416 | 416 | err = 0; |
| 417 | - if (tty->count > 1) | |
| 417 | + if (line->count++) | |
| 418 | 418 | goto out_unlock; |
| 419 | 419 | |
| 420 | - spin_unlock(&line->count_lock); | |
| 421 | - | |
| 420 | + BUG_ON(tty->driver_data); | |
| 422 | 421 | tty->driver_data = line; |
| 423 | 422 | line->tty = tty; |
| 424 | 423 | |
| 424 | + spin_unlock(&line->count_lock); | |
| 425 | 425 | err = enable_chan(line); |
| 426 | - if (err) | |
| 426 | + if (err) /* line_close() will be called by our caller */ | |
| 427 | 427 | return err; |
| 428 | 428 | |
| 429 | 429 | INIT_DELAYED_WORK(&line->task, line_timer_cb); |
| ... | ... | @@ -436,7 +436,7 @@ |
| 436 | 436 | chan_window_size(&line->chan_list, &tty->winsize.ws_row, |
| 437 | 437 | &tty->winsize.ws_col); |
| 438 | 438 | |
| 439 | - return err; | |
| 439 | + return 0; | |
| 440 | 440 | |
| 441 | 441 | out_unlock: |
| 442 | 442 | spin_unlock(&line->count_lock); |
| 443 | 443 | |
| 444 | 444 | |
| 445 | 445 | |
| ... | ... | @@ -460,17 +460,16 @@ |
| 460 | 460 | flush_buffer(line); |
| 461 | 461 | |
| 462 | 462 | spin_lock(&line->count_lock); |
| 463 | - if (!line->valid) | |
| 464 | - goto out_unlock; | |
| 463 | + BUG_ON(!line->valid); | |
| 465 | 464 | |
| 466 | - if (tty->count > 1) | |
| 465 | + if (--line->count) | |
| 467 | 466 | goto out_unlock; |
| 468 | 467 | |
| 469 | - spin_unlock(&line->count_lock); | |
| 470 | - | |
| 471 | 468 | line->tty = NULL; |
| 472 | 469 | tty->driver_data = NULL; |
| 473 | 470 | |
| 471 | + spin_unlock(&line->count_lock); | |
| 472 | + | |
| 474 | 473 | if (line->sigio) { |
| 475 | 474 | unregister_winch(tty); |
| 476 | 475 | line->sigio = 0; |
| ... | ... | @@ -498,7 +497,7 @@ |
| 498 | 497 | |
| 499 | 498 | spin_lock(&line->count_lock); |
| 500 | 499 | |
| 501 | - if (line->tty != NULL) { | |
| 500 | + if (line->count) { | |
| 502 | 501 | *error_out = "Device is already open"; |
| 503 | 502 | goto out; |
| 504 | 503 | } |