linux-kernel - Re: [PATCH v2 1/2] tracing: Simplify & fix saved

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: <CAJWu+oqLVE9aE_=atqbXMx9+uz1NB3M+SxOg6vjOTirS0HA1Pg@mail.gmail.com>
Date:   Thu, 1 Jul 2021 14:07:21 -0400
From:   Joel Fernandes <joelaf@...gle.com>
To:     Paul Burton <paulburton@...gle.com>
Cc:     linux-kernel@...r.kernel.org, Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>, stable@...r.kernel.org
Subject: Re: [PATCH v2 1/2] tracing: Simplify & fix saved_tgids logic

On Thu, Jul 1, 2021 at 1:24 PM Paul Burton <paulburton@...gle.com> wrote:
>
> The tgid_map array records a mapping from pid to tgid, where the index
> of an entry within the array is the pid & the value stored at that index
> is the tgid.
>
> The saved_tgids_next() function iterates over pointers into the tgid_map
> array & dereferences the pointers which results in the tgid, but then it
> passes that dereferenced value to trace_find_tgid() which treats it as a
> pid & does a further lookup within the tgid_map array. It seems likely
> that the intent here was to skip over entries in tgid_map for which the
> recorded tgid is zero, but instead we end up skipping over entries for
> which the thread group leader hasn't yet had its own tgid recorded in
> tgid_map.
>
> A minimal fix would be to remove the call to trace_find_tgid, turning:
>
>   if (trace_find_tgid(*ptr))
>
> into:
>
>   if (*ptr)
>
> ..but it seems like this logic can be much simpler if we simply let
> seq_read() iterate over the whole tgid_map array & filter out empty
> entries by returning SEQ_SKIP from saved_tgids_show(). Here we take that
> approach, removing the incorrect logic here entirely.
>
> Signed-off-by: Paul Burton <paulburton@...gle.com>
> Fixes: d914ba37d714 ("tracing: Add support for recording tgid of tasks")
> Cc: Steven Rostedt <rostedt@...dmis.org>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Joel Fernandes <joelaf@...gle.com>

Reviewed-by: Joel Fernandes (Google) <joel@...lfernandes.org>