linux-kernel - Re: [PATCH v2 2/3] coresight: catu: Prevent concurrent PERF and SYSFS mode enablement

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: <CALJ9ZPPp_DhLJcdjxmSTvkEspqYOK0mYkMaBF7ZieArJpWJOnw@mail.gmail.com>
Date: Thu, 3 Apr 2025 10:58:28 -0700
From: Yabin Cui <yabinc@...gle.com>
To: Leo Yan <leo.yan@....com>
Cc: Suzuki K Poulose <suzuki.poulose@....com>, Mike Leach <mike.leach@...aro.org>, 
	James Clark <james.clark@...aro.org>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, coresight@...ts.linaro.org, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] coresight: catu: Prevent concurrent PERF and SYSFS
 mode enablement

On Wed, Apr 2, 2025 at 6:47 AM Leo Yan <leo.yan@....com> wrote:
>
> On Tue, Apr 01, 2025 at 06:18:31PM -0700, Yabin Cui wrote:
> > A CATU device can be enabled for either PERF mode or SYSFS mode, but
> > not both simultaneously. Consider the following race condition:
> >
> > CPU 1 enables CATU for PERF mode.
> > CPU 2 enables CATU for SYSFS mode. It only increases refcnt.
> > CPU 2 enables ETR for SYSFS mode.
> > CPU 1 fails to enable ETR for PERF mode.
> >
> > This results in a CATU being enabled in PERF mode and an ETR enabled
> > in SYSFS mode, leading to unknown behavior.
> >
> > To prevent this situation, this patch checks enabled mode of a
> > CATU device before increasing its refcnt.
>
> Yes.  We have also observed the same issue.  I am currently working on
> refactoring the Arm CoreSight locking, my plan is to use coresight_path
> to maintain mode.
>
> Given it is uncommon case to use two modes concurrently in Arm
> CoreSight, I assume this is not an urgent issue.  Could we defer this
> patch?  My understanding is that this issue will be prevented with
> checking the path mode, rather than checking modes in seperate modules.

Sure, I am fine with withdrawing this patch, as there are other plans to address
the issue.

>
> To be clear, I am fine with other two patches in the series.
>
> Thanks,
> Leo
>
> > Signed-off-by: Yabin Cui <yabinc@...gle.com>
> > Suggested-by: James Clark <james.clark@...aro.org>
> > Suggested-by: Leo Yan <leo.yan@....com>
> > ---
> >  drivers/hwtracing/coresight/coresight-catu.c | 6 +++++-
> >  drivers/hwtracing/coresight/coresight-catu.h | 1 +
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
> > index b1d490dd7249..0caf3a3e75ce 100644
> > --- a/drivers/hwtracing/coresight/coresight-catu.c
> > +++ b/drivers/hwtracing/coresight/coresight-catu.c
> > @@ -466,7 +466,10 @@ static int catu_enable(struct coresight_device *csdev, enum cs_mode mode,
> >               CS_UNLOCK(catu_drvdata->base);
> >               rc = catu_enable_hw(catu_drvdata, mode, data);
> >               CS_LOCK(catu_drvdata->base);
> > -     }
> > +             if (!rc)
> > +                     catu_drvdata->mode = mode;
> > +     } else if (catu_drvdata->mode != mode)
> > +             rc = -EBUSY;
> >       if (!rc)
> >               csdev->refcnt++;
> >       return rc;
> > @@ -499,6 +502,7 @@ static int catu_disable(struct coresight_device *csdev, void *__unused)
> >               CS_UNLOCK(catu_drvdata->base);
> >               rc = catu_disable_hw(catu_drvdata);
> >               CS_LOCK(catu_drvdata->base);
> > +             catu_drvdata->mode = CS_MODE_DISABLED;
> >       } else {
> >               rc = -EBUSY;
> >       }
> > diff --git a/drivers/hwtracing/coresight/coresight-catu.h b/drivers/hwtracing/coresight/coresight-catu.h
> > index 755776cd19c5..ea406464f0a8 100644
> > --- a/drivers/hwtracing/coresight/coresight-catu.h
> > +++ b/drivers/hwtracing/coresight/coresight-catu.h
> > @@ -66,6 +66,7 @@ struct catu_drvdata {
> >       struct coresight_device *csdev;
> >       int irq;
> >       raw_spinlock_t spinlock;
> > +     enum cs_mode mode;
> >  };
> >
> >  #define CATU_REG32(name, offset)                                     \
> > --
> > 2.49.0.472.ge94155a9ec-goog
> >