lkml.org 
[lkml]   [2022]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [syzbot] KASAN: use-after-free Read in task_work_run (2)
On Thu, 27 Oct 2022 at 05:03, Hillf Danton <hdanton@sina.com> wrote:
>
> On 26 Oct 2022 11:29:35 -0700
> > syzbot has found a reproducer for the following issue on:
> >
> > HEAD commit: 88619e77b33d net: stmmac: rk3588: Allow multiple gmac cont..
> > git tree: bpf
> > console output: https://syzkaller.appspot.com/x/log.txt?x=1646d6f2880000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=a66c6c673fb555e8
> > dashboard link: https://syzkaller.appspot.com/bug?extid=9228d6098455bb209ec8
> > compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12bc425e880000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1126516e880000
>
> Grab another hold on event upon adding task work in bid to fix uaf.
>
> #syz test https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git 88619e77b33d
>
> --- x/kernel/events/core.c
> +++ c/kernel/events/core.c
> @@ -2291,6 +2291,7 @@ event_sched_out(struct perf_event *event
> !event->pending_work) {
> event->pending_work = 1;
> dec = false;
> + atomic_long_inc(&event->refcount);
> task_work_add(current, &event->pending_task, TWA_RESUME);
> }
> if (dec)
> @@ -6561,6 +6562,8 @@ static void perf_pending_task(struct cal
> struct perf_event *event = container_of(head, struct perf_event, pending_task);
> int rctx;
>
> + if (event->state == PERF_EVENT_STATE_DEAD)
> + goto out;
> /*
> * If we 'fail' here, that's OK, it means recursion is already disabled
> * and we won't recurse 'further'.
> @@ -6577,6 +6580,8 @@ static void perf_pending_task(struct cal
> if (rctx >= 0)
> perf_swevent_put_recursion_context(rctx);
> preempt_enable_notrace();
> +out:
> + put_event(event);
> }
>
> #ifdef CONFIG_GUEST_PERF_EVENTS

I'm not convinced this is what we want - while we could prolong the
lifetime of an event, but if we're concurrently killing the event
somewhere, we might as well cancel the task work (and potentially just
skip a pending SIGTRAP). Your change most likely results in similar
behaviour due to the DEAD check, although it prolongs the event's
lifetime unnecessarily.

\
 
 \ /
  Last update: 2022-11-23 12:16    [W:0.093 / U:0.668 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site