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

[Zev Weiss]

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@xxxxxxxxxxxxx>
> ---
> 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