linux-kernel - Re: [PATCH v1 3/8] software node: Show properties and their values in sysfs

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: <YGAmB2Nwph6pArXc@kroah.com>
Date:   Sun, 28 Mar 2021 08:45:27 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Daniel Scally <djrscally@...il.com>, linux-kernel@...r.kernel.org,
        linux-media@...r.kernel.org, linux-acpi@...r.kernel.org,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Yong Zhi <yong.zhi@...el.com>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Bingbu Cao <bingbu.cao@...el.com>,
        Tianshu Qiu <tian.shu.qiu@...el.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Subject: Re: [PATCH v1 3/8] software node: Show properties and their values
 in sysfs

On Sun, Mar 28, 2021 at 12:20:07AM +0200, Andy Shevchenko wrote:
> It's very convenient to see what properties and their values
> are currently being assigned in the registered software nodes.
> 
> Show properties and their values in sysfs.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> ---
>  drivers/base/swnode.c | 137 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 132 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 19aa44bc2628..d7fe1a887d2d 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -10,6 +10,7 @@
>  #include <linux/kernel.h>
>  #include <linux/property.h>
>  #include <linux/slab.h>
> +#include <linux/sysfs.h>
>  
>  struct swnode {
>  	int id;
> @@ -17,6 +18,10 @@ struct swnode {
>  	struct fwnode_handle fwnode;
>  	const struct software_node *node;
>  
> +	/* properties in sysfs */
> +	struct kobj_attribute *property_attrs;
> +	struct attribute_group property_group;
> +
>  	/* hierarchy */
>  	struct ida child_ids;
>  	struct list_head entry;
> @@ -25,6 +30,7 @@ struct swnode {
>  
>  	unsigned int allocated:1;
>  	unsigned int managed:1;
> +	unsigned int properties:1;
>  };
>  
>  static DEFINE_IDA(swnode_root_ids);
> @@ -299,6 +305,18 @@ static int property_entry_copy_data(struct property_entry *dst,
>  	return 0;
>  }
>  
> +static int property_entries_count(const struct property_entry *properties)
> +{
> +	int n = 0;
> +
> +	if (properties) {
> +		while (properties[n].name)
> +			n++;
> +	}
> +
> +	return n;
> +}
> +
>  /**
>   * property_entries_dup - duplicate array of properties
>   * @properties: array of properties to copy
> @@ -310,15 +328,13 @@ struct property_entry *
>  property_entries_dup(const struct property_entry *properties)
>  {
>  	struct property_entry *p;
> -	int i, n = 0;
> +	int i, n;
>  	int ret;
>  
> -	if (!properties)
> +	n = property_entries_count(properties);
> +	if (n == 0)
>  		return NULL;
>  
> -	while (properties[n].name)
> -		n++;
> -
>  	p = kcalloc(n + 1, sizeof(*p), GFP_KERNEL);
>  	if (!p)
>  		return ERR_PTR(-ENOMEM);
> @@ -746,6 +762,108 @@ static void software_node_free(const struct software_node *node)
>  	kfree(node);
>  }
>  
> +static ssize_t
> +swnode_property_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
> +{
> +	struct swnode *swnode = kobj_to_swnode(kobj);
> +	const struct property_entry *prop;
> +	const void *pointer;
> +	ssize_t len = 0;
> +	int i;
> +
> +	prop = property_entry_get(swnode->node->properties, attr->attr.name);
> +	if (!prop)
> +		return -EINVAL;
> +
> +	/* We can't fail here, because it means boolean property */
> +	pointer = property_get_pointer(prop);
> +	if (!pointer)
> +		return sysfs_emit(buf, "\n");
> +
> +	switch (prop->type) {
> +	case DEV_PROP_U8:
> +		for (i = 0; i < prop->length / sizeof(u8); i++)
> +			len += sysfs_emit_at(buf, len, "%u,", ((u8 *)pointer)[i]);

No, sysfs is "one value per file", and that is not what you are showing
here at all :(

Also, there is no Documentation/ABI/ entries for your new sysfs files,
so that means we couldn't take this patcheset anyway :(

greg k-h