linux-kernel - Re: [PATCH 2/3] regulator: userspace-consumer: Add regulator event support

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: <5000a233-e992-455a-bca3-79aaa8713ff4@hatter.bewilderbeest.net>
Date:   Fri, 1 Sep 2023 02:27:28 -0700
From:   Zev Weiss <zev@...ilderbeest.net>
To:     Naresh Solanki <naresh.solanki@...ements.com>
Cc:     broonie@...nel.org, Liam Girdwood <lgirdwood@...il.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] regulator: userspace-consumer: Add regulator event
 support

On Thu, Aug 31, 2023 at 05:14:09AM PDT, Naresh Solanki wrote:
>Add sysfs attribute to track regulator events received from regulator
>notifier block handler.
>
>Signed-off-by: Naresh Solanki <Naresh.Solanki@...ements.com>
>---
> drivers/regulator/userspace-consumer.c | 54 +++++++++++++++++++++++++-
> 1 file changed, 53 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/regulator/userspace-consumer.c b/drivers/regulator/userspace-consumer.c
>index 97f075ed68c9..a936661d99cd 100644
>--- a/drivers/regulator/userspace-consumer.c
>+++ b/drivers/regulator/userspace-consumer.c
>@@ -29,6 +29,10 @@ struct userspace_consumer_data {
>
> 	int num_supplies;
> 	struct regulator_bulk_data *supplies;
>+
>+	struct kobject *kobj;
>+	struct notifier_block nb;
>+	unsigned long events;
> };
>
> static ssize_t name_show(struct device *dev,
>@@ -89,12 +93,30 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
> 	return count;
> }
>
>+static DEFINE_SPINLOCK(events_lock);
>+
>+static ssize_t events_show(struct device *dev,
>+			   struct device_attribute *attr, char *buf)
>+{
>+	struct userspace_consumer_data *data = dev_get_drvdata(dev);
>+	unsigned long e;
>+
>+	spin_lock(&events_lock);
>+	e = data->events;
>+	data->events = 0;

I still don't think this is a good solution for the problem.

I for one frequently examine things in sysfs using shell commands like 
'cat' and 'grep' and such, and I suspect I'm (very, very) far from alone 
in that.  With this design a user doing that could cause a monitoring 
daemon to miss events that it was expecting to receive via this file.  

I don't think we should be creating sysfs files that are secretly land 
mines that allow a curious user innocently peeking around in sysfs doing 
(they think) read-only operations to break things for other programs 
using those files.


Zev