On 02/22, Prashant Bhole wrote:
> I encountered following BUG caught by KASAN with recent kernels when trying
> out [BCC project] bcc/testing/python/test_usdt2.py
> Tried with v4.12, it was reproducible.
> --- KASAN log ---
> BUG: KASAN: use-after-free in uprobe_perf_close+0x118/0x1a0
> Read of size 4 at addr ffff8800bb2db4cc by task test_usdt2.py/1265
> CPU: 2 PID: 1265 Comm: test_usdt2.py Not tainted 4.16.0-rc2-next-20180220+
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1.fc26
> Call Trace:
> ? uprobe_perf_close+0x118/0x1a0
> Freed by task 1265:
Hmm. this looks strange, I do not see no __put_task_struct/free_task in this
trace... OK, nevermind.
> After debugging, found that uprobe_perf_close() is called after task has
> been terminated and uprobe_perf_close() tries to access task_struct of the
> terminated process.
Oh. You can't imagine how much I forgot this code ;) I will recheck, but at
first glance you are right. We can't rely on _free_event()->put_ctx() which
does put_task_struct() after event->destroy(), the exiting task does
put_task_struct(current) itself and sets child_ctx->task = TASK_TOMBSTONE in
In short, nothing protects event->hw.target. But uprobe_perf_open() should be
safe, perf_init_event() is called when the caller has the additional reference.
I am wondering if this was wrong from the very beginning or it was broken later,
but I won't even try to check.
> static int uprobe_perf_close(struct trace_uprobe *tu, struct perf_event
> + int err = 0;
> bool done;
> @@ -1054,9 +1055,12 @@ static int uprobe_perf_close(struct trace_uprobe *tu,
> struct perf_event *event)
> if (!done)
> - return uprobe_apply(tu->inode, tu->offset, &tu->consumer, false);
> + err = uprobe_apply(tu->inode, tu->offset, &tu->consumer, false);
> - return 0;
> + if (event->hw.target)
> + put_task_struct(event->hw.target);
> + return err;
No need to delay put_task_struct(), you can call it right after "done = ..."
in the "if (event->hw.target)" block and simplify this change.
However. Probably this is the simplest fix, but it doesn't look nice. We do not
really need task_struct, we need its ->mm. PF_EXITING check can be removed, this
is a minor optimization.
perhaps we can add _something_ like
struct mm_struct *uprobe_event_mm(event)
// needs rcu_read_lock/READ_ONCE/etc
if (event->ctx &&
event->ctx->task && event->ctx->task != TASK_TOMBSTONE)
and use it instead of event->hw.target->mm ... Not sure.
And. What about other users of event->hw.target? Say, task_bp_pinned(). It
dereference this pointer, How can we trust the result of "iter->hw.target ==
if hw.target can be freed and then re-alloced?
This all makes me think that we should change (fix) kernel/events/core.c...