Messages in this thread | | | Subject | Re: uprobes/perf: KASAN: use-after-free in uprobe_perf_close | From | Prashant Bhole <> | Date | Mon, 9 Apr 2018 19:00:06 +0900 |
| |
On 4/9/2018 4:38 PM, Peter Zijlstra wrote: > On Tue, Mar 06, 2018 at 06:49:10PM +0900, Prashant Bhole wrote: >> Sorry for late reply. I tried these changes. It didn't fix the problem. With > > He, sorry for completely forgetting about this one :/ > >> these changes, the use-after-free access of task_struct occurs at >> _free_event() for the last remaining event. >> >> In your changes, I tried keeping get/put_task_struct() in >> perf_alloc_context()/put_ctx() intact and The problem did not occur. Changes >> are mentioned below. > > Yes, I think you're right in that this is the cleanest solution; it adds > reference counting to the exact pointer we're using. > >> diff --git a/kernel/events/core.c b/kernel/events/core.c >> index c98cce4ceebd..65889d2b5ae2 100644 >> --- a/kernel/events/core.c >> +++ b/kernel/events/core.c >> @@ -4109,6 +4109,8 @@ static void _free_event(struct perf_event *event) >> >> if (event->ctx) >> put_ctx(event->ctx); >> + if (event->hw.target) >> + put_task_struct(event->hw.target); >> >> exclusive_event_destroy(event); >> module_put(event->pmu->module); >> @@ -9593,6 +9595,7 @@ perf_event_alloc(struct perf_event_attr *attr, int >> cpu, >> * and we cannot use the ctx information because we need the >> * pmu before we get a ctx. >> */ >> + get_task_struct(task); >> event->hw.target = task; >> } >> >> @@ -9708,6 +9711,8 @@ perf_event_alloc(struct perf_event_attr *attr, int >> cpu, >> perf_detach_cgroup(event); >> if (event->ns) >> put_pid_ns(event->ns); >> + if (task) > > Should this not too be 'event->hw.target', for consistency and clarity?
Yes, I am sending a patch with this change. Thanks.
-Prashant
| |