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] [day] [month] [year] [list]
Message-ID: <526b1130-afec-4c75-8d86-74c5b7e272fe@rivosinc.com>
Date: Wed, 28 Aug 2024 10:13:54 -0700
From: Atish Patra <atishp@...osinc.com>
To: Alexandre Ghiti <alexghiti@...osinc.com>, Will Deacon <will@...nel.org>
Cc: Atish Patra <atishp@...shpatra.org>, Anup Patel <anup@...infault.org>,
 Mark Rutland <mark.rutland@....com>, Paul Walmsley
 <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>,
 Albert Ou <aou@...s.berkeley.edu>, linux-riscv@...ts.infradead.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 Nam Cao <namcao@...utronix.de>
Subject: Re: [PATCH -fixes] drivers: perf: Fix smp_processor_id() use in
 preemptible code

On 8/28/24 5:36 AM, Alexandre Ghiti wrote:
> Hi Will,
> 
> On Tue, Aug 27, 2024 at 2:53 PM Will Deacon <will@...nel.org> wrote:
>>
>> On Mon, Aug 26, 2024 at 06:52:10PM +0200, Alexandre Ghiti wrote:
>>> As reported in [1], the use of smp_processor_id() in
>>> pmu_sbi_device_probe() must be protected by disabling the preemption, so
>>> simple use get_cpu()/put_cpu() instead.
>>>
>>> Reported-by: Nam Cao <namcao@...utronix.de>
>>> Closes: https://lore.kernel.org/linux-riscv/20240820074925.ReMKUPP3@linutronix.de/ [1]
>>> Signed-off-by: Alexandre Ghiti <alexghiti@...osinc.com>
>>> ---
>>>   drivers/perf/riscv_pmu_sbi.c | 7 ++++++-
>>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
>>> index 31a17a56eb3b..25b1b699b3e2 100644
>>> --- a/drivers/perf/riscv_pmu_sbi.c
>>> +++ b/drivers/perf/riscv_pmu_sbi.c
>>> @@ -1373,11 +1373,15 @@ static int pmu_sbi_device_probe(struct platform_device *pdev)
>>>
>>>        /* SBI PMU Snapsphot is only available in SBI v2.0 */
>>>        if (sbi_v2_available) {
>>> +             int cpu;
>>> +
>>>                ret = pmu_sbi_snapshot_alloc(pmu);
>>>                if (ret)
>>>                        goto out_unregister;
>>>
>>> -             ret = pmu_sbi_snapshot_setup(pmu, smp_processor_id());
>>> +             cpu = get_cpu();
>>> +
>>> +             ret = pmu_sbi_snapshot_setup(pmu, cpu);
>>>                if (ret) {
>>>                        /* Snapshot is an optional feature. Continue if not available */
>>>                        pmu_sbi_snapshot_free(pmu);
>>> @@ -1391,6 +1395,7 @@ static int pmu_sbi_device_probe(struct platform_device *pdev)
>>>                         */
>>>                        static_branch_enable(&sbi_pmu_snapshot_available);
>>>                }
>>> +             put_cpu();
>>
>> Are you sure it's safe to enable the static key with preemption disabled?
>> I thought that could block on a mutex.
> 

Thanks Will for pointing that out.

> Yep, it seems you're right, thanks for jumping in.
> 
> I'm discussing with Atish how to fix that differently, I'll be back
> with another version very soon.
> 

Looking at the driver core framework code, I am wondering if a probe 
function can be preempted to run on a different cpu. If it can only be 
preempted by higher priority kernel threads or interrupts but is 
guaranteed to run on the same cpu again, we can just use the 
raw_smp_processor_id.

However, if there is no guarantee then we can just invoke 
get_cpu/put_cpu around pmu_sbi_snapshot_setup.

> Thanks again,
> 
> Alex
> 
>>
>> Will
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ