linux-kernel - Re: [GIT PULL] tracing/uprobe: Fix uprobe_perf

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-=wjakjw6-rDzDDBsuMoDCqd+9ogifR_EE1F0K-jYek1CdA@mail.gmail.com>
Date:   Wed, 24 Nov 2021 10:27:27 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jiri Olsa <jolsa@...hat.com>,
        Masami Hiramatsu <mhiramat@...nel.org>
Subject: Re: [GIT PULL] tracing/uprobe: Fix uprobe_perf_open probes iteration

On Wed, Nov 24, 2021 at 7:10 AM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> tracing: Fix wrong uprobe variable in iterator

I've pulled this, but:

>         list_for_each_entry(pos, trace_probe_probe_list(tp), list) {
> +               tu = container_of(pos, struct trace_uprobe, tp);

honestly, the "list_for_each_entry()" followed by a "container_of()"
like this makes me think you used the wrong entry to walk the list in.

You actually don't want to ever use that

        struct trace_probe *pos;

at all, and I think you should remove it.

Instead, you should do something like

        list_for_each_entry(pu, trace_probe_probe_list(tp), tp.list) {

ie simply walk the list _as_ the uprobe entry, not as some
intermediate internal probe list entry only to convert to the uprobe.

Now, I may be entirely off my meds here, and maybe there is something
I'm missing, but I _think_ the attached patch should work, and avoid
all that indirection through 'pos' that you don't care about and that
seems to just have been a mistake.

Feel free to call me funny names for when I missed some detail.

Again - I *have* pulled your fix, and in fact the attached patch is
relative to your fix. That fix isn't _wrong_. I just think it's a bit
silly, and I think the cause of the bug in the first place was that
unnecessary intermediate pointer.

               Linus

View attachment "patch.diff" of type "text/x-patch" (1002 bytes)