linux-kernel - Re: [RFC PATCH 2/2] riscv: Introduce support for hardware break/watchpoints

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: <CAPd4WewhGXdE0KUzrnLGgK9cDjz35TFvr_fqYHc5yVzKTFCdHQ@mail.gmail.com>
Date: Mon, 12 May 2025 10:45:49 +0530
From: Himanshu Chauhan <hchauhan@...tanamicro.com>
To: Qingfang Deng <dqfext@...il.com>
Cc: linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	Charlie Jenkins <charlie@...osinc.com>, paul.walmsley@...ive.com, palmer@...belt.com, 
	aou@...s.berkeley.edu
Subject: Re: [RFC PATCH 2/2] riscv: Introduce support for hardware break/watchpoints

Hi Qingfang,

Thanks for pointing it out! That explains the difference in behaviour.
I will fix it.

Regards
Himanshu

On Mon, May 12, 2025 at 9:01 AM Qingfang Deng <dqfext@...il.com> wrote:
>
> Hi, Himanshu and Charlie,
>
> > +static int arch_smp_setup_sbi_shmem(unsigned int cpu)
> > +{
> > +     struct sbi_dbtr_shmem_entry *dbtr_shmem;
> > +     unsigned long shmem_pa;
> > +     struct sbiret ret;
> > +     int rc;
> > +
> > +     dbtr_shmem = per_cpu_ptr(sbi_dbtr_shmem, cpu);
> > +     if (!dbtr_shmem) {
> > +             pr_err("Invalid per-cpu shared memory for debug triggers\n");
> > +             return -ENODEV;
> > +     }
> > +
> > +     shmem_pa = __pa(dbtr_shmem);
> > +
> > +     ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
> > +                     (!MEM_LO(shmem_pa) ? 0xFFFFFFFFUL : MEM_LO(shmem_pa)),
> > +                     (!MEM_HI(shmem_pa) ? 0xFFFFFFFFUL : MEM_HI(shmem_pa)),
> > +                      0, 0, 0, 0);
> > +
> > +     if (ret.error) {
> > +             switch(ret.error) {
> > +             case SBI_ERR_DENIED:
> > +                     pr_warn("%s: Access denied for shared memory at %lx\n",
> > +                             __func__, shmem_pa);
> > +                     rc = -EPERM;
> > +                     break;
> > +
> > +             case SBI_ERR_INVALID_PARAM:
> > +             case SBI_ERR_INVALID_ADDRESS:
> > +                     pr_warn("%s: Invalid address parameter (%lu)\n",
> > +                             __func__, ret.error);
> > +                     rc = -EINVAL;
> > +                     break;
> > +
> > +             case SBI_ERR_ALREADY_AVAILABLE:
> > +                     pr_warn("%s: Shared memory is already set\n",
> > +                             __func__);
> > +                     rc = -EADDRINUSE;
> > +                     break;
> > +
> > +             case SBI_ERR_FAILURE:
> > +                     pr_err("%s: Internal sdtrig state error\n",
> > +                            __func__);
> > +                     rc = -ENXIO;
> > +                     break;
> > +
> > +             default:
> > +                     pr_warn("%s: Unknown error %lu\n", __func__, ret.error);
> > +                     rc = -ENXIO;
> > +                     break;
> > +             }
> > +     }
> > +
> > +     pr_warn("CPU %d: HW Breakpoint shared memory registered.\n", cpu);
>
> This is printed unconditionally, even if there is an error above.
>
> > +
> > +     return rc;
>
> If ret.error is 0, rc is uninitialized here, which may explain the error
> that Charlie came across.
>
> Regards,
>
> -- Qingfang