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
| ||
|
Message-ID: <CAJ9a7VhA0b_o_aYJisfey0XK7FadNfxkUapTQc3nE611RiyVRg@mail.gmail.com> Date: Tue, 21 Mar 2023 15:12:10 +0000 From: Mike Leach <mike.leach@...aro.org> To: James Clark <james.clark@....com> Cc: coresight@...ts.linaro.org, Mathieu Poirier <mathieu.poirier@...aro.org>, Suzuki K Poulose <suzuki.poulose@....com>, Leo Yan <leo.yan@...aro.org>, Alexander Shishkin <alexander.shishkin@...ux.intel.com>, linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v2 0/9] coresight: Fix CTI module refcount leak by making it a helper device Hi James On Fri, 10 Mar 2023 at 16:06, James Clark <james.clark@....com> wrote: > > Changes since v1: > > * Don't dereference handle in tmc_etr_get_buffer() when not in perf mode. > * Fix some W=1 warnings > * Add a commit to rename child/output in terms of local/remote > > ------------------- > > Currently there is a refcount leak in CTI when using system wide mode > or tracing multithreaded applications. See the last commit for a > reproducer. This prevents the module from being unloaded. > > Historically there have been a few issues and fixes attempted around > here which have resulted in some extra logic and a member to keep > track of CTI being enabled 'struct coresight_device->ect_enabled'. > The fix in commit 665c157e0204 ("coresight: cti: Fix hang in > cti_disable_hw()") was also related to CTI having its own > enable/disable path which came later than other devices. > > If we make CTI a helper device and enable helper devices adjacent to > the path we get very similar enable/disable behavior to now, but with > more reuse of the existing reference counting logic in the coresight > core code. This also affects CATU which can have a little bit of > its hard coded enable/disable code removed. > > Enabling CATU on the generic path does require that input connections > are tracked so that it can get its associated ETR buffer. > > Applies to coresight/next (669c4614236a7) but also requires the > realloc_array patch here [1]. > > Also available in full here [2]. > > [1]: https://lore.kernel.org/linux-arm-kernel/20230306152723.3090195-1-james.clark@arm.com/ > [2]: https://gitlab.arm.com/linux-arm/linux-jc/-/tree/james-cs-cti-module-refcount-fix-v2 > > James Clark (9): > coresight: Use enum type for cs_mode wherever possible > coresight: Change name of pdata->conns > coresight: Rename nr_outports to nr_outconns > coresight: Rename connection members to allow for input connections > coresight: Dynamically add connections > coresight: Store in-connections as well as out-connections > coresight: Refactor out buffer allocation function for ETR > coresight: Enable and disable helper devices adjacent to the path > coresight: Fix CTI module refcount leak by making it a helper device > > drivers/hwtracing/coresight/coresight-catu.c | 34 +- > drivers/hwtracing/coresight/coresight-core.c | 312 +++++++++++------- > .../hwtracing/coresight/coresight-cti-core.c | 56 ++-- > .../hwtracing/coresight/coresight-cti-sysfs.c | 4 +- > drivers/hwtracing/coresight/coresight-cti.h | 4 +- > drivers/hwtracing/coresight/coresight-etb10.c | 3 +- > .../coresight/coresight-etm3x-core.c | 6 +- > .../coresight/coresight-etm4x-core.c | 6 +- > .../hwtracing/coresight/coresight-platform.c | 178 +++++++--- > drivers/hwtracing/coresight/coresight-priv.h | 9 +- > drivers/hwtracing/coresight/coresight-stm.c | 6 +- > drivers/hwtracing/coresight/coresight-sysfs.c | 9 +- > .../hwtracing/coresight/coresight-tmc-etf.c | 2 +- > .../hwtracing/coresight/coresight-tmc-etr.c | 89 ++--- > drivers/hwtracing/coresight/coresight-tmc.h | 2 + > drivers/hwtracing/coresight/coresight-tpdm.c | 4 +- > drivers/hwtracing/coresight/coresight-tpiu.c | 3 +- > drivers/hwtracing/coresight/coresight-trbe.c | 3 +- > drivers/hwtracing/coresight/ultrasoc-smb.c | 3 +- > drivers/hwtracing/coresight/ultrasoc-smb.h | 2 +- > include/linux/coresight.h | 109 +++--- > 21 files changed, 530 insertions(+), 314 deletions(-) > > -- > 2.34.1 > Looking at this overall - given that the only use of the in_conn is to reference the connecting device from the helper, i.e. coresight-catu.c:405: tmp = csdev->pdata->in_conns[i].remote_dev; would it not be simpler to : a) in coresight_connection add a field: struct coresight_device *origin_dev; which mimics the origin / target model we already have in coresight_sysfs_link then b) the in_conns could simply be references to out_conn object from origin_dev, rather than a complete coresight_connection with reversed values, thus simplifying the in_conns handling code, and removing the unused reversed feilds in the current in_conn object. e.g. tmp = csdev->pdata->in_conns[i]->origin_dev The remainder of the code would remain much the same, just adjusted for in_conns as refs rather than independent conn objects Regards Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK
Powered by blists - more mailing lists