| 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: <D9DV2VOCWEK3.TQ96Z41CV0P4@gmail.com>
Date: Wed, 23 Apr 2025 05:05:35 -0300
From: "Kurt Borja" <kuurtb@...il.com>
To: Thomas Weißschuh <linux@...ssschuh.net>, "Hans de
Goede" <hdegoede@...hat.com>, Ilpo Järvinen
<ilpo.jarvinen@...ux.intel.com>, "Armin Wolf" <W_Armin@....de>
Cc: <platform-driver-x86@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
"Joshua Grisham" <josh@...huagrisham.com>
Subject: Re: [PATCH 1/2] platform/x86: firmware_attributes_class: Provide a
highlevel interface
On Tue Jan 7, 2025 at 2:05 PM -03, Thomas Weißschuh wrote:
> Currently each user of firmware_attributes_class has to manually set up
> kobjects, devices, etc.
> Provide a higher level API which takes care of the low-level details.
>
> Signed-off-by: Thomas Weißschuh <linux@...ssschuh.net>
> ---
> drivers/platform/x86/firmware_attributes_class.c | 146 +++++++++++++++++++++++
> drivers/platform/x86/firmware_attributes_class.h | 37 ++++++
> 2 files changed, 183 insertions(+)
>
> diff --git a/drivers/platform/x86/firmware_attributes_class.c b/drivers/platform/x86/firmware_attributes_class.c
> index 736e96c186d9dc6d945517f090e9af903e93bbf4..70ceae5215820098b017bfda991a3c2a7824c98e 100644
> --- a/drivers/platform/x86/firmware_attributes_class.c
> +++ b/drivers/platform/x86/firmware_attributes_class.c
> @@ -2,6 +2,9 @@
>
> /* Firmware attributes class helper module */
>
> +#include <linux/device/class.h>
> +#include <linux/device.h>
> +#include <linux/kobject.h>
> #include <linux/module.h>
> #include "firmware_attributes_class.h"
>
> @@ -22,6 +25,149 @@ static __exit void fw_attributes_class_exit(void)
> }
> module_exit(fw_attributes_class_exit);
>
> +static ssize_t fw_attributes_sysfs_show(struct kobject *kobj, struct attribute *attr, char *buf)
> +{
> + struct firmware_attributes_device *fwadev = to_firmware_attribute_device(kobj);
> + const struct firmware_attribute *fw_attr = to_firmware_attribute(attr);
> +
> + if (!fw_attr->show)
> + return -EIO;
> +
> + return fw_attr->show(fwadev, fw_attr, buf);
> +}
> +
> +static ssize_t fw_attributes_sysfs_store(struct kobject *kobj, struct attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct firmware_attributes_device *fwadev = to_firmware_attribute_device(kobj);
> + const struct firmware_attribute *fw_attr = to_firmware_attribute(attr);
> +
> + if (!fw_attr->store)
> + return -EIO;
> +
> + return fw_attr->store(fwadev, fw_attr, buf, count);
> +}
> +
> +static const struct sysfs_ops fw_attributes_sysfs_ops = {
> + .show = fw_attributes_sysfs_show,
> + .store = fw_attributes_sysfs_store,
> +};
> +
> +static void fw_attributes_attr_release(struct kobject *kobj)
> +{
> + struct firmware_attributes_device *fwadev = to_firmware_attribute_device(kobj);
> + struct device *cdev;
> +
> + cdev = fwadev->dev;
> +
> + kfree(fwadev);
> + device_unregister(cdev);
> +}
> +
> +static const struct kobj_type fw_attributes_attr_type = {
> + .sysfs_ops = &fw_attributes_sysfs_ops,
> + .release = fw_attributes_attr_release,
> +};
> +
> +DEFINE_FREE(firmware_attributes_device_unregister, struct firmware_attributes_device *,
> + if (_T) firmware_attributes_device_unregister(_T))
> +
> +struct firmware_attributes_device *
> +firmware_attributes_device_register(struct device *parent, const char *name,
> + const struct attribute_group **groups, void *data)
> +{
> + struct firmware_attributes_device *fwadev = NULL;
> + struct device *cdev = NULL;
> + int ret;
> +
> + fwadev = kzalloc(sizeof(*fwadev), GFP_KERNEL);
> + if (!fwadev)
> + return ERR_PTR(-ENOMEM);
> +
> + cdev = device_create(&firmware_attributes_class, parent, MKDEV(0, 0), "%s", name);
> + if (IS_ERR(cdev))
> + return ERR_CAST(cdev);
> +
> + fwadev->data = data;
> + fwadev->dev = cdev;
> +
> + ret = kobject_init_and_add(&fwadev->attributes, &fw_attributes_attr_type, &cdev->kobj,
> + "attributes");
> + if (ret) {
> + device_del(cdev);
> + return ERR_PTR(ret);
> + }
> +
> + if (groups) {
> + ret = sysfs_create_groups(&fwadev->attributes, groups);
> + if (ret) {
> + firmware_attributes_device_unregister(fwadev);
> + return ERR_PTR(ret);
> + }
> +
> + kobject_uevent(&fwadev->dev->kobj, KOBJ_CHANGE);
> + }
> +
> + return fwadev;
> +}
> +EXPORT_SYMBOL_GPL(firmware_attributes_device_register);
> +
> +void firmware_attributes_device_unregister(struct firmware_attributes_device *fwadev)
> +{
> + kobject_del(&fwadev->attributes);
> + kobject_put(&fwadev->attributes);
> +}
> +EXPORT_SYMBOL_GPL(firmware_attributes_device_unregister);
> +
> +static void devm_firmware_attributes_device_release(void *data)
> +{
> + struct firmware_attributes_device *fwadev = data;
> +
> + firmware_attributes_device_unregister(fwadev);
> +}
> +
> +struct firmware_attributes_device *
> +devm_firmware_attributes_device_register(struct device *parent, const char *name,
> + const struct attribute_group **groups, void *data)
> +{
> + struct firmware_attributes_device *fwadev;
> + int ret;
> +
> + fwadev = firmware_attributes_device_register(parent, name, groups, data);
> + if (IS_ERR(fwadev))
> + return fwadev;
> +
> + ret = devm_add_action_or_reset(parent, devm_firmware_attributes_device_release, fwadev);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return fwadev;
> +}
> +EXPORT_SYMBOL_GPL(devm_firmware_attributes_device_register);
> +
> +static ssize_t firmware_attributes_type_show(struct firmware_attributes_device *fwadev,
> + const struct firmware_attribute *attr, char *buf)
> +{
> + if (attr == &_firmware_attribute_type_string)
> + return sysfs_emit(buf, "string\n");
> + else if (attr == &_firmware_attribute_type_enumeration)
> + return sysfs_emit(buf, "enumeration\n");
> + else if (attr == &_firmware_attribute_type_integer)
> + return sysfs_emit(buf, "integer\n");
> + else
> + return -EIO;
> +}
> +
> +#define __FW_TYPE_ATTR __ATTR(type, 0444, firmware_attributes_type_show, NULL)
> +
> +const struct firmware_attribute _firmware_attribute_type_string = __FW_TYPE_ATTR;
> +EXPORT_SYMBOL_GPL(_firmware_attribute_type_string);
> +const struct firmware_attribute _firmware_attribute_type_enumeration = __FW_TYPE_ATTR;
> +EXPORT_SYMBOL_GPL(_firmware_attribute_type_enumeration);
> +const struct firmware_attribute _firmware_attribute_type_integer = __FW_TYPE_ATTR;
> +EXPORT_SYMBOL_GPL(_firmware_attribute_type_integer);
> +
> MODULE_AUTHOR("Mark Pearson <markpearson@...ovo.com>");
> +MODULE_AUTHOR("Thomas Weißschuh <linux@...ssschuh.net>");
> MODULE_DESCRIPTION("Firmware attributes class helper module");
> MODULE_LICENSE("GPL");
> diff --git a/drivers/platform/x86/firmware_attributes_class.h b/drivers/platform/x86/firmware_attributes_class.h
> index d27abe54fcf9812a2f0868eec5426bbc8e7eb21c..66837ad9f65b8ca501dee73f48c01f2710d86bf5 100644
> --- a/drivers/platform/x86/firmware_attributes_class.h
> +++ b/drivers/platform/x86/firmware_attributes_class.h
> @@ -5,8 +5,45 @@
> #ifndef FW_ATTR_CLASS_H
> #define FW_ATTR_CLASS_H
>
> +#include <linux/device.h>
> #include <linux/device/class.h>
> +#include <linux/sysfs.h>
>
> extern const struct class firmware_attributes_class;
>
> +struct firmware_attributes_device {
> + struct device *dev;
> + struct kobject attributes;
> + void *data;
> +};
> +
> +struct firmware_attribute {
> + struct attribute attr;
> + ssize_t (*show)(struct firmware_attributes_device *fwadev,
> + const struct firmware_attribute *attr, char *buf);
> + ssize_t (*store)(struct firmware_attributes_device *fwadev,
> + const struct firmware_attribute *attr, const char *buf, size_t count);
> +};
> +
> +#define to_firmware_attribute(_a) container_of_const(_a, struct firmware_attribute, attr)
> +#define to_firmware_attribute_device(_s) \
> + container_of_const(_s, struct firmware_attributes_device, attributes)
> +
> +extern const struct firmware_attribute _firmware_attribute_type_string;
> +#define firmware_attribute_type_string ((struct attribute *)&_firmware_attribute_type_string.attr)
> +extern const struct firmware_attribute _firmware_attribute_type_enumeration;
> +#define firmware_attribute_type_enumeration ((struct attribute *)&_firmware_attribute_type_enumeration.attr)
> +extern const struct firmware_attribute _firmware_attribute_type_integer;
> +#define firmware_attribute_type_integer ((struct attribute *)&_firmware_attribute_type_integer.attr)
> +
> +struct firmware_attributes_device * __must_check
> +firmware_attributes_device_register(struct device *parent, const char *name,
> + const struct attribute_group **groups, void *data);
> +
> +void firmware_attributes_device_unregister(struct firmware_attributes_device *fwadev);
> +
> +struct firmware_attributes_device * __must_check
> +devm_firmware_attributes_device_register(struct device *parent, const char *name,
> + const struct attribute_group **groups, void *data);
> +
> #endif /* FW_ATTR_CLASS_H */
Hi Thomas,
Are you still working on this patchset?
If you don't mind I can take it from here. I'm planning on fixing a
couple memory leaks of this patch and extend it a bit more with some
helper macros for ABI compliant drivers. I might drop the test driver
though.
I ask this because I want to use this class in another series I'm
to do, and I think this patch is a great starting point.
Let me know what you think!
--
~ Kurt
Powered by blists - more mailing lists