linux-kernel - Re: [PATCH] platform/x86: wmi-bmof: Make use of .bin

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ad05e251-37bd-0cc2-8b11-a859e453f476@linux.intel.com>
Date: Tue, 10 Dec 2024 16:47:38 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Armin Wolf <W_Armin@....de>
cc: linux@...ssschuh.net, Hans de Goede <hdegoede@...hat.com>, 
    platform-driver-x86@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] platform/x86: wmi-bmof: Make use of .bin_size()
 callback

On Fri, 6 Dec 2024, Armin Wolf wrote:

> Until now the wmi-bmof driver had to allocate the binary sysfs
> attribute dynamically since its size depends on the bmof buffer
> returned by the firmware.
> 
> Use the new .bin_size() callback to avoid having to do this memory
> allocation.
> 
> Tested on a Asus Prime B650-Plus.
> 
> Signed-off-by: Armin Wolf <W_Armin@....de>
> ---
>  drivers/platform/x86/wmi-bmof.c | 75 +++++++++++++++++----------------
>  1 file changed, 38 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/platform/x86/wmi-bmof.c b/drivers/platform/x86/wmi-bmof.c
> index df6f0ae6e6c7..3e33da36da8a 100644
> --- a/drivers/platform/x86/wmi-bmof.c
> +++ b/drivers/platform/x86/wmi-bmof.c
> @@ -20,66 +20,66 @@
> 
>  #define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
> 
> -struct bmof_priv {
> -	union acpi_object *bmofdata;
> -	struct bin_attribute bmof_bin_attr;
> -};
> -
> -static ssize_t read_bmof(struct file *filp, struct kobject *kobj, struct bin_attribute *attr,
> +static ssize_t bmof_read(struct file *filp, struct kobject *kobj, const struct bin_attribute *attr,
>  			 char *buf, loff_t off, size_t count)
>  {
> -	struct bmof_priv *priv = container_of(attr, struct bmof_priv, bmof_bin_attr);
> +	struct device *dev = kobj_to_dev(kobj);
> +	union acpi_object *obj = dev_get_drvdata(dev);
> 
> -	return memory_read_from_buffer(buf, count, &off, priv->bmofdata->buffer.pointer,
> -				       priv->bmofdata->buffer.length);
> +	return memory_read_from_buffer(buf, count, &off, obj->buffer.pointer, obj->buffer.length);
>  }
> 
> -static int wmi_bmof_probe(struct wmi_device *wdev, const void *context)
> +static const BIN_ATTR_ADMIN_RO(bmof, 0);
> +
> +static const struct bin_attribute * const bmof_attrs[] = {
> +	&bin_attr_bmof,
> +	NULL
> +};
> +
> +static size_t bmof_bin_size(struct kobject *kobj, const struct bin_attribute *attr, int n)
>  {
> -	struct bmof_priv *priv;
> -	int ret;
> +	struct device *dev = kobj_to_dev(kobj);
> +	union acpi_object *obj = dev_get_drvdata(dev);
> +
> +	return obj->buffer.length;
> +}
> 
> -	priv = devm_kzalloc(&wdev->dev, sizeof(struct bmof_priv), GFP_KERNEL);
> -	if (!priv)
> -		return -ENOMEM;
> +static const struct attribute_group bmof_group = {
> +	.bin_size = bmof_bin_size,
> +	.bin_attrs_new = bmof_attrs,
> +};
> +
> +static const struct attribute_group *bmof_groups[] = {
> +	&bmof_group,
> +	NULL
> +};
> 
> -	dev_set_drvdata(&wdev->dev, priv);
> +static int wmi_bmof_probe(struct wmi_device *wdev, const void *context)
> +{
> +	union acpi_object *obj;
> 
> -	priv->bmofdata = wmidev_block_query(wdev, 0);
> -	if (!priv->bmofdata) {
> +	obj = wmidev_block_query(wdev, 0);
> +	if (!obj) {
>  		dev_err(&wdev->dev, "failed to read Binary MOF\n");
>  		return -EIO;
>  	}
> 
> -	if (priv->bmofdata->type != ACPI_TYPE_BUFFER) {
> +	if (obj->type != ACPI_TYPE_BUFFER) {
>  		dev_err(&wdev->dev, "Binary MOF is not a buffer\n");
> -		ret = -EIO;
> -		goto err_free;
> +		kfree(obj);
> +		return -EIO;
>  	}
> 
> -	sysfs_bin_attr_init(&priv->bmof_bin_attr);
> -	priv->bmof_bin_attr.attr.name = "bmof";
> -	priv->bmof_bin_attr.attr.mode = 0400;
> -	priv->bmof_bin_attr.read = read_bmof;
> -	priv->bmof_bin_attr.size = priv->bmofdata->buffer.length;
> -
> -	ret = device_create_bin_file(&wdev->dev, &priv->bmof_bin_attr);
> -	if (ret)
> -		goto err_free;
> +	dev_set_drvdata(&wdev->dev, obj);
> 
>  	return 0;
> -
> - err_free:
> -	kfree(priv->bmofdata);
> -	return ret;
>  }
> 
>  static void wmi_bmof_remove(struct wmi_device *wdev)
>  {
> -	struct bmof_priv *priv = dev_get_drvdata(&wdev->dev);
> +	union acpi_object *obj = dev_get_drvdata(&wdev->dev);
> 
> -	device_remove_bin_file(&wdev->dev, &priv->bmof_bin_attr);
> -	kfree(priv->bmofdata);
> +	kfree(obj);
>  }
> 
>  static const struct wmi_device_id wmi_bmof_id_table[] = {
> @@ -90,6 +90,7 @@ static const struct wmi_device_id wmi_bmof_id_table[] = {
>  static struct wmi_driver wmi_bmof_driver = {
>  	.driver = {
>  		.name = "wmi-bmof",
> +		.dev_groups = bmof_groups,
>  	},
>  	.probe = wmi_bmof_probe,
>  	.remove = wmi_bmof_remove,

So do I understand right that this meant to supercede the patch from 
Thomas?

-- 
 i.