diff options
| author | Oleg Nesterov <oleg@redhat.com> | 2012-09-30 15:12:44 -0400 |
|---|---|---|
| committer | Oleg Nesterov <oleg@redhat.com> | 2012-10-07 15:19:43 -0400 |
| commit | 71434f2fcba5c22d6e0d51878ba8e241a5dea5fb (patch) | |
| tree | 0a61c5a75390a4eb39f018cc39cf8f40b0103058 /kernel | |
| parent | 4710f05fd146d4739e57a8832a3abc5bd3bf0997 (diff) | |
uprobes: Fix the racy uprobe->flags manipulation
Multiple threads can manipulate uprobe->flags, this is obviously
unsafe. For example mmap can set UPROBE_COPY_INSN while register
tries to set UPROBE_RUN_HANDLER, the latter can also race with
can_skip_sstep() which clears UPROBE_SKIP_SSTEP.
Change this code to use bitops.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Diffstat (limited to 'kernel')
| -rw-r--r-- | kernel/events/uprobes.c | 28 |
1 files changed, 14 insertions, 14 deletions
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 7f62b30c4172..c92651d619ca 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c | |||
| @@ -79,11 +79,11 @@ static struct mutex uprobes_mmap_mutex[UPROBES_HASH_SZ]; | |||
| 79 | static atomic_t uprobe_events = ATOMIC_INIT(0); | 79 | static atomic_t uprobe_events = ATOMIC_INIT(0); |
| 80 | 80 | ||
| 81 | /* Have a copy of original instruction */ | 81 | /* Have a copy of original instruction */ |
| 82 | #define UPROBE_COPY_INSN 0x1 | 82 | #define UPROBE_COPY_INSN 0 |
| 83 | /* Dont run handlers when first register/ last unregister in progress*/ | 83 | /* Dont run handlers when first register/ last unregister in progress*/ |
| 84 | #define UPROBE_RUN_HANDLER 0x2 | 84 | #define UPROBE_RUN_HANDLER 1 |
| 85 | /* Can skip singlestep */ | 85 | /* Can skip singlestep */ |
| 86 | #define UPROBE_SKIP_SSTEP 0x4 | 86 | #define UPROBE_SKIP_SSTEP 2 |
| 87 | 87 | ||
| 88 | struct uprobe { | 88 | struct uprobe { |
| 89 | struct rb_node rb_node; /* node in the rb tree */ | 89 | struct rb_node rb_node; /* node in the rb tree */ |
| @@ -94,7 +94,7 @@ struct uprobe { | |||
| 94 | struct uprobe_consumer *consumers; | 94 | struct uprobe_consumer *consumers; |
| 95 | struct inode *inode; /* Also hold a ref to inode */ | 95 | struct inode *inode; /* Also hold a ref to inode */ |
| 96 | loff_t offset; | 96 | loff_t offset; |
| 97 | int flags; | 97 | unsigned long flags; |
| 98 | struct arch_uprobe arch; | 98 | struct arch_uprobe arch; |
| 99 | }; | 99 | }; |
| 100 | 100 | ||
| @@ -423,7 +423,7 @@ static struct uprobe *insert_uprobe(struct uprobe *uprobe) | |||
| 423 | spin_unlock(&uprobes_treelock); | 423 | spin_unlock(&uprobes_treelock); |
| 424 | 424 | ||
| 425 | /* For now assume that the instruction need not be single-stepped */ | 425 | /* For now assume that the instruction need not be single-stepped */ |
| 426 | uprobe->flags |= UPROBE_SKIP_SSTEP; | 426 | __set_bit(UPROBE_SKIP_SSTEP, &uprobe->flags); |
| 427 | 427 | ||
| 428 | return u; | 428 | return u; |
| 429 | } | 429 | } |
| @@ -466,7 +466,7 @@ static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs) | |||
| 466 | { | 466 | { |
| 467 | struct uprobe_consumer *uc; | 467 | struct uprobe_consumer *uc; |
| 468 | 468 | ||
| 469 | if (!(uprobe->flags & UPROBE_RUN_HANDLER)) | 469 | if (!test_bit(UPROBE_RUN_HANDLER, &uprobe->flags)) |
| 470 | return; | 470 | return; |
| 471 | 471 | ||
| 472 | down_read(&uprobe->consumer_rwsem); | 472 | down_read(&uprobe->consumer_rwsem); |
| @@ -577,11 +577,11 @@ static int prepare_uprobe(struct uprobe *uprobe, struct file *file, | |||
| 577 | { | 577 | { |
| 578 | int ret = 0; | 578 | int ret = 0; |
| 579 | 579 | ||
| 580 | if (uprobe->flags & UPROBE_COPY_INSN) | 580 | if (test_bit(UPROBE_COPY_INSN, &uprobe->flags)) |
| 581 | return ret; | 581 | return ret; |
| 582 | 582 | ||
| 583 | mutex_lock(&uprobe->copy_mutex); | 583 | mutex_lock(&uprobe->copy_mutex); |
| 584 | if (uprobe->flags & UPROBE_COPY_INSN) | 584 | if (test_bit(UPROBE_COPY_INSN, &uprobe->flags)) |
| 585 | goto out; | 585 | goto out; |
| 586 | 586 | ||
| 587 | ret = copy_insn(uprobe, file); | 587 | ret = copy_insn(uprobe, file); |
| @@ -601,7 +601,7 @@ static int prepare_uprobe(struct uprobe *uprobe, struct file *file, | |||
| 601 | UPROBE_SWBP_INSN_SIZE > PAGE_SIZE); | 601 | UPROBE_SWBP_INSN_SIZE > PAGE_SIZE); |
| 602 | 602 | ||
| 603 | smp_wmb(); /* pairs with rmb() in find_active_uprobe() */ | 603 | smp_wmb(); /* pairs with rmb() in find_active_uprobe() */ |
| 604 | uprobe->flags |= UPROBE_COPY_INSN; | 604 | set_bit(UPROBE_COPY_INSN, &uprobe->flags); |
| 605 | 605 | ||
| 606 | out: | 606 | out: |
| 607 | mutex_unlock(&uprobe->copy_mutex); | 607 | mutex_unlock(&uprobe->copy_mutex); |
| @@ -852,7 +852,7 @@ int uprobe_register(struct inode *inode, loff_t offset, struct uprobe_consumer * | |||
| 852 | uprobe->consumers = NULL; | 852 | uprobe->consumers = NULL; |
| 853 | __uprobe_unregister(uprobe); | 853 | __uprobe_unregister(uprobe); |
| 854 | } else { | 854 | } else { |
| 855 | uprobe->flags |= UPROBE_RUN_HANDLER; | 855 | set_bit(UPROBE_RUN_HANDLER, &uprobe->flags); |
| 856 | } | 856 | } |
| 857 | } | 857 | } |
| 858 | 858 | ||
| @@ -885,7 +885,7 @@ void uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consume | |||
| 885 | if (consumer_del(uprobe, uc)) { | 885 | if (consumer_del(uprobe, uc)) { |
| 886 | if (!uprobe->consumers) { | 886 | if (!uprobe->consumers) { |
| 887 | __uprobe_unregister(uprobe); | 887 | __uprobe_unregister(uprobe); |
| 888 | uprobe->flags &= ~UPROBE_RUN_HANDLER; | 888 | clear_bit(UPROBE_RUN_HANDLER, &uprobe->flags); |
| 889 | } | 889 | } |
| 890 | } | 890 | } |
| 891 | 891 | ||
| @@ -1346,10 +1346,10 @@ bool uprobe_deny_signal(void) | |||
| 1346 | */ | 1346 | */ |
| 1347 | static bool can_skip_sstep(struct uprobe *uprobe, struct pt_regs *regs) | 1347 | static bool can_skip_sstep(struct uprobe *uprobe, struct pt_regs *regs) |
| 1348 | { | 1348 | { |
| 1349 | if (uprobe->flags & UPROBE_SKIP_SSTEP) { | 1349 | if (test_bit(UPROBE_SKIP_SSTEP, &uprobe->flags)) { |
| 1350 | if (arch_uprobe_skip_sstep(&uprobe->arch, regs)) | 1350 | if (arch_uprobe_skip_sstep(&uprobe->arch, regs)) |
| 1351 | return true; | 1351 | return true; |
| 1352 | uprobe->flags &= ~UPROBE_SKIP_SSTEP; | 1352 | clear_bit(UPROBE_SKIP_SSTEP, &uprobe->flags); |
| 1353 | } | 1353 | } |
| 1354 | return false; | 1354 | return false; |
| 1355 | } | 1355 | } |
| @@ -1473,7 +1473,7 @@ static void handle_swbp(struct pt_regs *regs) | |||
| 1473 | * new and not-yet-analyzed uprobe at the same address, restart. | 1473 | * new and not-yet-analyzed uprobe at the same address, restart. |
| 1474 | */ | 1474 | */ |
| 1475 | smp_rmb(); /* pairs with wmb() in install_breakpoint() */ | 1475 | smp_rmb(); /* pairs with wmb() in install_breakpoint() */ |
| 1476 | if (unlikely(!(uprobe->flags & UPROBE_COPY_INSN))) | 1476 | if (unlikely(!test_bit(UPROBE_COPY_INSN, &uprobe->flags))) |
| 1477 | goto restart; | 1477 | goto restart; |
| 1478 | 1478 | ||
| 1479 | utask = current->utask; | 1479 | utask = current->utask; |
