linux-kernel - Re: [PATCH v15 03/13] hp-bioscfg: bioscfg

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: <CAOOmCE-GWjcncveifa5=T4O3tt1wkZKM0og-7ZUMUQSdr6d5cQ@mail.gmail.com>
Date:   Wed, 31 May 2023 15:09:15 -0500
From:   Jorge Lopez <jorgealtxwork@...il.com>
To:     Thomas Weißschuh <thomas@...ch.de>
Cc:     hdegoede@...hat.com, platform-driver-x86@...r.kernel.org,
        linux-kernel@...r.kernel.org, ilpo.jarvinen@...ux.intel.com
Subject: Re: [PATCH v15 03/13] hp-bioscfg: bioscfg

On Fri, May 26, 2023 at 10:28 AM Thomas Weißschuh <thomas@...ch.de> wrote:
>
> On 2023-05-19 15:12:50-0500, Jorge Lopez wrote:
>
> <snip>
>
> > ---
> >  drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 988 +++++++++++++++++++
> >  1 file changed, 988 insertions(+)
> >  create mode 100644 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> >
> > diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> > new file mode 100644
> > index 000000000000..fe0be40c8dee
> > --- /dev/null
> > +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> > @@ -0,0 +1,988 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Common methods for use with hp-bioscfg driver
> > + *
> > + *  Copyright (c) 2022 HP Development Company, L.P.
> > + */
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/fs.h>
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/wmi.h>
> > +#include "bioscfg.h"
> > +#include "../../firmware_attributes_class.h"
> > +#include <linux/nls.h>
> > +#include <linux/errno.h>
> > +
> > +MODULE_AUTHOR("Jorge Lopez <jorge.lopez2@...com>");
> > +MODULE_DESCRIPTION("HP BIOS Configuration Driver");
> > +MODULE_LICENSE("GPL");
> > +
> > +struct bioscfg_priv bioscfg_drv = {
> > +     .mutex = __MUTEX_INITIALIZER(bioscfg_drv.mutex),
> > +};
> > +
> > +static struct class *fw_attr_class;
> > +
> > +ssize_t display_name_language_code_show(struct kobject *kobj,
> > +                                     struct kobj_attribute *attr,
> > +                                     char *buf)
> > +{
> > +     return sysfs_emit(buf, "%s\n", bioscfg_drv.display_name_language_code);
> > +}
>
> You probably can also define the "struct attribute" for this callback
> right here.
> Then there don't have to be attribute definitions everywhere.
>
> As the language code is always the same anyways this can just print the
> constant. Also remove the variables and explicit setting from all the
> attributes.

Done!

>
> > +
> > +int hp_get_integer_from_buffer(u8 **buffer, u32 *buffer_size, u32 *integer)
> > +{
> > +     int *ptr = PTR_ALIGN((int *)*buffer, 4);
>
> 4 -> sizeof(int)

Done!

>
> > +
> > +     /* Ensure there is enough space remaining to read the integer */
> > +     if (*buffer_size < sizeof(int))
> > +             return -EINVAL;
> > +
> > +     *integer = *(ptr++);
> > +     *buffer = (u8 *)ptr;
> > +     *buffer_size -= sizeof(int);
> > +
> > +     return 0;
> > +}
> > +

<snip>

> > +/**
> > + * hp_init_bios_attributes() - Initialize all attributes for a type
> > + * @attr_type: The attribute type to initialize
> > + * @guid: The WMI GUID associated with this type to initialize
> > + *
> > + * Initialiaze all 5 types of attributes: enumeration, integer,
>
> "Initialize"

Done!

>
> > + * string, password, ordered list  object.  Populates each attrbute types
> > + * respective properties under sysfs files
> > + */
> > +static int hp_init_bios_attributes(enum hp_wmi_data_type attr_type, const char *guid)
> > +{
> > +     union acpi_object *obj = NULL;
> > +     int min_elements;
> > +
> > +     /* instance_id needs to be reset for each type GUID
> > +      * also, instance IDs are unique within GUID but not across
> > +      */
> > +     int instance_id = 0;
> > +     int retval = 0;
>
> More common would be just "ret" instead of "retval".
> Same everywhere else.

Done!

<snip>

> > +     fw_attributes_class_put();
> > +     hp_exit_attr_set_interface();
> > +}
> > +
> > +module_init(hp_init);
> > +module_exit(hp_exit);
> > --
> > 2.34.1
> >