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; |