Messages in this thread | | | Date | Thu, 28 Feb 2013 21:10:24 +0100 | From | Oleg Nesterov <> | Subject | Re: [RFC PATCH v3 4/6] uretprobes: return probe entry, prepare uretprobe |
| |
On 02/28, Anton Arapov wrote: > > +static void prepare_uretprobe(struct uprobe *uprobe, struct pt_regs *regs) > +{ > + struct return_uprobe_i *ri; > + struct uprobe_task *utask; > + struct xol_area *area; > + unsigned long rp_trampoline_vaddr = 0; > + uprobe_opcode_t insn = UPROBE_SWBP_INSN; > + > + area = get_xol_area(); > + if (area) > + rp_trampoline_vaddr = area->rp_trampoline_vaddr; > + if (!rp_trampoline_vaddr) { > + rp_trampoline_vaddr = xol_get_insn_slot(&insn); > + if (!rp_trampoline_vaddr) > + return; > + } > + area->rp_trampoline_vaddr = rp_trampoline_vaddr;
This is called under down_read(), so 2 threads can race with each other and use the different rp_trampoline_vaddr's if ->rp_trampoline_vaddr was NULL.
And again, I think ->rp_trampoline_vaddr is simply unneeded, see my reply to 3/6.
> static void handler_chain(struct uprobe *uprobe, struct pt_regs *regs) > { > + int rc = 0; > struct uprobe_consumer *uc; > int remove = UPROBE_HANDLER_REMOVE; > > down_read(&uprobe->register_rwsem); > for (uc = uprobe->consumers; uc; uc = uc->next) { > - int rc = uc->handler(uc, regs); > + if (uc->handler) > + rc = uc->handler(uc, regs); > + > + if (uc->rp_handler) > + prepare_uretprobe(uprobe, regs); /* put bp at return */
Hmm. I didn't read this series yet. But at first glance I am not sure prepare_uretprobe() should be called every time we see ->rp_handler != NULL, there could be multiple consumers...
Oleg.
| |