linux-kernel - Re: [PATCH 0/9] coresight: change some driver' spinlock type to raw_spinlock

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:
 <GV1PR08MB105210FEE951B6331F48CF6CCFB292@GV1PR08MB10521.eurprd08.prod.outlook.com>
Date: Thu, 28 Nov 2024 06:23:14 +0000
From: Yeo Reum Yun <YeoReum.Yun@....com>
To: James Clark <james.clark@...aro.org>, Suzuki Poulose
	<Suzuki.Poulose@....com>, "mike.leach@...aro.org" <mike.leach@...aro.org>
CC: "coresight@...ts.linaro.org" <coresight@...ts.linaro.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-rt-devel@...ts.linux.dev"
	<linux-rt-devel@...ts.linux.dev>, nd <nd@....com>,
	"alexander.shishkin@...ux.intel.com" <alexander.shishkin@...ux.intel.com>,
	"bigeasy@...utronix.de" <bigeasy@...utronix.de>, "clrkwllms@...nel.org"
	<clrkwllms@...nel.org>, "rostedt@...dmis.org" <rostedt@...dmis.org>
Subject: Re: [PATCH 0/9] coresight: change some driver' spinlock type to
 raw_spinlock_t

Hi James,

Thanks for your review.

> Somewhat related to this change, but not something that needs to be done
> now:

> All of the spinlocks for the syfs store/read functions don't need to be
> raw spinlocks, or spinlocks at all. They're never called from the
> scheduling code and can be locked by coresight_mutex instead.
> coresight_mutex is held on the sysfs enable/disable path when those
> values are actually used.

> Same point as here:
> https://lore.kernel.org/linux-arm-kernel/9a637e74-d81d-405c-bad0-c97ec1aa4b77@linaro.org/

> Probably needs a review of which values in each device might be shared
> between perf mode and sysfs mode when they shouldn't be, as there was
> one in the link above.

> I thought the only thing shared between sysfs and perf should be the
> 'mode' which is taken with a compare and swap without locking anyway.

I think it seems bit of hard while sysfs exports interface to modify configuration.
Think about one source enabling via "PERF" and other process try to call reset via "SYSFS"
wihtout locking enabled "PERF" event can have different configuration 
because "SYSFS" can change the configuration while applying "PERF" configuration configured by perf event.

(CPU0 - enabling some coresight device with PERF and configuration from perf-event
 CPU1 - reset configuration via "SYSFS")

I think it seems hard to shared only "mode" between PERF and SYSFS mode.

Suppose one soure device's mode is DISABLED, and
    - one process setting the configuration via SYSFS interface
    - one process enabling source device via PERF according to configuration saved in perf_event.

wihtout locking, how to prevent "SYSFS"'s change of configuration while it saw "DISABLED state"?
I think wihtout lock, It falls to enabled perf event with inconsistent configuration.

IMHO, only shared cannot be mode unless sysfs provides interface to config something
which can affects function of hardware.....

Thanks.