linux-kernel - Re: [PATCH v3 56/57] perf: Simplify perf_pmu_output

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wh6JEk7wYECcMdbXHf5ST8PAkOyUXhE8x2kqT6to+Gn9Q@mail.gmail.com>
Date:   Mon, 12 Jun 2023 09:19:19 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     keescook@...omium.org, gregkh@...uxfoundation.org,
        pbonzini@...hat.com, masahiroy@...nel.org, nathan@...nel.org,
        ndesaulniers@...gle.com, nicolas@...sle.eu,
        catalin.marinas@....com, will@...nel.org, vkoul@...nel.org,
        trix@...hat.com, ojeda@...nel.org, mingo@...hat.com,
        longman@...hat.com, boqun.feng@...il.com, dennis@...nel.org,
        tj@...nel.org, cl@...ux.com, acme@...nel.org, mark.rutland@....com,
        alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
        namhyung@...nel.org, irogers@...gle.com, adrian.hunter@...el.com,
        juri.lelli@...hat.com, vincent.guittot@...aro.org,
        dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
        mgorman@...e.de, bristot@...hat.com, vschneid@...hat.com,
        paulmck@...nel.org, frederic@...nel.org, quic_neeraju@...cinc.com,
        joel@...lfernandes.org, josh@...htriplett.org,
        mathieu.desnoyers@...icios.com, jiangshanlai@...il.com,
        rientjes@...gle.com, vbabka@...e.cz, roman.gushchin@...ux.dev,
        42.hyeyoo@...il.com, apw@...onical.com, joe@...ches.com,
        dwaipayanray1@...il.com, lukas.bulwahn@...il.com,
        john.johansen@...onical.com, paul@...l-moore.com,
        jmorris@...ei.org, serge@...lyn.com, linux-kbuild@...r.kernel.org,
        linux-kernel@...r.kernel.org, dmaengine@...r.kernel.org,
        llvm@...ts.linux.dev, linux-perf-users@...r.kernel.org,
        rcu@...r.kernel.org, linux-security-module@...r.kernel.org,
        tglx@...utronix.de, ravi.bangoria@....com, error27@...il.com,
        luc.vanoostenryck@...il.com
Subject: Re: [PATCH v3 56/57] perf: Simplify perf_pmu_output_stop()

This patch looks completely broken to me.

You now do

                if (err == -EAGAIN)
                        goto restart;

*within* the RCU-guarded section, and the "goto restart" will guard it again.

So no. Sending out a series of 57 patches that can have these kinds of
bugs in it is not ok. By patch 56 (which just happened to come in
fairly early for me), all sane peoples eyes have glazed over and they
don't react to this kind of breakage any more.

                Linus

On Mon, Jun 12, 2023 at 2:39 AM Peter Zijlstra <peterz@...radead.org> wrote:
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7977,7 +7977,8 @@ static void perf_pmu_output_stop(struct
>         int err, cpu;
>
>  restart:
> -       rcu_read_lock();
> +       /* cannot have a label in front of a decl */;
> +       guard(rcu)();
>         list_for_each_entry_rcu(iter, &event->rb->event_list, rb_entry) {
>                 /*
>                  * For per-CPU events, we need to make sure that neither they
> @@ -7993,12 +7994,9 @@ static void perf_pmu_output_stop(struct
>                         continue;
>
>                 err = cpu_function_call(cpu, __perf_pmu_output_stop, event);
> -               if (err == -EAGAIN) {
> -                       rcu_read_unlock();
> +               if (err == -EAGAIN)
>                         goto restart;
> -               }
>         }
> -       rcu_read_unlock();
>  }