linux-kernel - Re: [PATCH] perf: Fix sys_perf_event

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: <7bf54a3f-07f7-5471-f6c2-88d3d9bc7a5e@amd.com>
Date:   Thu, 17 Nov 2022 16:03:54 +0530
From:   Ravi Bangoria <ravi.bangoria@....com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Peter Zijlstra <peterz@...radead.org>, nslusarek@....net,
        Linus Torvalds <torvalds@...ux-foundation.org>, x86@...nel.org,
        linux-kernel@...r.kernel.org, Ravi Bangoria <ravi.bangoria@....com>
Subject: Re: [PATCH] perf: Fix sys_perf_event_open() race against self

Hi Boris,

On 17-Nov-22 4:00 AM, Borislav Petkov wrote:
> On Mon, May 23, 2022 at 04:03:29PM +0530, Ravi Bangoria wrote:
>> On 21-May-22 12:08 AM, Peter Zijlstra wrote:
>>>
>>> Norbert reported that it's possible to race sys_perf_event_open() such
>>> that the looser ends up in another context from the group leader,
>>> triggering many WARNs.
>>>
>>> The move_group case checks for races against itself, but the
>>> !move_group case doesn't, seemingly relying on the previous
>>> group_leader->ctx == ctx check. However, that check is racy due to not
>>> holding any locks at that time.
>>>
>>> Therefore, re-check the result after acquiring locks and bailing
>>> if they no longer match.
>>>
>>> Additionally, clarify the not_move_group case from the
>>> move_group-vs-move_group race.
>>
>> Tested-by: Ravi Bangoria <ravi.bangoria@....com>
>>
>> Below is a quick test to reproduce the issue. It triggers WARN_ON()
>> as normal user. No warnings with the patch.
> 
> Shouldn't this test be in tools/perf/tests/ or so?

The issue was because of multiple event contexts involved. However, it's
no longer the case with reworked event context:

  https://git.kernel.org/tip/tip/c/bd27568117664b

Thanks,
Ravi