[v11,06/14] HP BIOSCFG driver - passwdobj-attributes
Commit Message
HP BIOS Configuration driver purpose is to provide a driver supporting
the latest sysfs class firmware attributes framework allowing the user
to change BIOS settings and security solutions on HP Inc.’s commercial
notebooks.
Many features of HP Commercial notebooks can be managed using Windows
Management Instrumentation (WMI). WMI is an implementation of Web-Based
Enterprise Management (WBEM) that provides a standards-based interface
for changing and monitoring system settings. HP BIOSCFG driver provides
a native Linux solution and the exposed features facilitates the
migration to Linux environments.
The Linux security features to be provided in hp-bioscfg driver enables
managing the BIOS settings and security solutions via sysfs, a virtual
filesystem that can be used by user-mode applications. The new
documentation cover HP-specific firmware sysfs attributes such Secure
Platform Management and Sure Start. Each section provides security
feature description and identifies sysfs directories and files exposed
by the driver.
Many HP Commercial notebooks include a feature called Secure Platform
Management (SPM), which replaces older password-based BIOS settings
management with public key cryptography. PC secure product management
begins when a target system is provisioned with cryptographic keys
that are used to ensure the integrity of communications between system
management utilities and the BIOS.
HP Commercial notebooks have several BIOS settings that control its
behaviour and capabilities, many of which are related to security.
To prevent unauthorized changes to these settings, the system can
be configured to use a cryptographic signature-based authorization
string that the BIOS will use to verify authorization to modify the
setting.
Linux Security components are under development and not published yet.
The only linux component is the driver (hp bioscfg) at this time.
Other published security components are under Windows.
Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>
---
Based on the latest platform-drivers-x86.git/for-next
---
.../x86/hp/hp-bioscfg/passwdobj-attributes.c | 669 ++++++++++++++++++
1 file changed, 669 insertions(+)
create mode 100644 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
Comments
On 2023-04-20 11:54:46-0500, Jorge Lopez wrote:
> ---
> .../x86/hp/hp-bioscfg/passwdobj-attributes.c | 669 ++++++++++++++++++
> 1 file changed, 669 insertions(+)
> create mode 100644 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
>
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> new file mode 100644
> index 000000000000..c03b3a71e9c4
> --- /dev/null
> +++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> @@ -0,0 +1,669 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Functions corresponding to password object type attributes under
> + * BIOS PASSWORD for use with hp-bioscfg driver.
> + *
> + * Copyright (c) 2022 HP Development Company, L.P.
> + */
> +
> +#include "bioscfg.h"
> +#include <asm-generic/posix_types.h>
> +
> +GET_INSTANCE_ID(password);
> +/*
> + * Clear all passwords copied to memory for a particular
> + * authentication instance
> + */
> +int clear_passwords(const int instance)
> +{
> + if (!bioscfg_drv.password_data[instance].is_enabled)
> + return 0;
> +
> + memset(bioscfg_drv.password_data[instance].current_password,
> + 0, sizeof(bioscfg_drv.password_data[instance].current_password));
> + memset(bioscfg_drv.password_data[instance].new_password,
> + 0, sizeof(bioscfg_drv.password_data[instance].new_password));
> +
> + return 0;
> +}
> +
> +/*
> + * Clear all credentials copied to memory for both Power-ON and Setup
> + * BIOS instances
> + */
> +int clear_all_credentials(void)
> +{
> + int instance;
> +
> + /* clear all passwords */
> + for (instance = 0; instance < bioscfg_drv.password_instances_count; instance++)
> + clear_passwords(instance);
> +
> + /* clear auth_token */
> + kfree(bioscfg_drv.spm_data.auth_token);
> + bioscfg_drv.spm_data.auth_token = NULL;
> +
> + return 0;
> +}
> +
> +int get_password_instance_for_type(const char *name)
> +{
> + int count = bioscfg_drv.password_instances_count;
> + int instance;
> +
> + for (instance = 0; instance < count; instance++) {
> + if (strcmp(bioscfg_drv.password_data[instance].common.display_name, name) == 0)
> + return instance;
> + }
> + return -EINVAL;
> +}
> +
> +int validate_password_input(int instance_id, const char *buf)
> +{
> + int length;
> +
> + length = strlen(buf);
> + if (buf[length-1] == '\n')
> + length--;
> +
> + if (length > MAX_PASSWD_SIZE)
> + return INVALID_BIOS_AUTH;
> +
> + if (bioscfg_drv.password_data[instance_id].min_password_length > length ||
> + bioscfg_drv.password_data[instance_id].max_password_length < length)
> + return INVALID_BIOS_AUTH;
> + return SUCCESS;
> +}
> +
> +int password_is_set(const char *name)
bool is_password_set(const char *name)
> +{
> + int id;
> +
> + id = get_password_instance_for_type(name);
> + if (id < 0)
> + return 0;
> +
> + return bioscfg_drv.password_data[id].is_enabled;
> +}
> +
> +ATTRIBUTE_N_PROPERTY_SHOW(is_enabled, password);
> +static struct kobj_attribute password_is_password_set = __ATTR_RO(is_enabled);
> +
> +static ssize_t current_password_store(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + char *p, *buf_cp;
> + int id, ret = 0;
> +
> + buf_cp = kstrdup(buf, GFP_KERNEL);
> + if (!buf_cp) {
> + ret = -ENOMEM;
> + goto exit_password;
> + }
> +
> + p = memchr(buf_cp, '\n', count);
> +
> + if (p != NULL)
> + *p = '\0';
This will also accept input like "foo\nbar" and truncate away the "bar".
For something like a password it seems errorprone to try to munge the
value.
> +
> + id = get_password_instance_id(kobj);
> +
> + if (id >= 0)
> + ret = validate_password_input(id, buf_cp);
> +
> + if (!ret) {
> + strscpy(bioscfg_drv.password_data[id].current_password,
> + buf_cp,
> + sizeof(bioscfg_drv.password_data[id].current_password));
> + /*
> + * set pending reboot flag depending on
> + * "RequiresPhysicalPresence" value
> + */
> + if (bioscfg_drv.password_data[id].common.requires_physical_presence)
> + bioscfg_drv.pending_reboot = true;
Just setting this to true does not emit the necessary KOBJ_CHANGE event
on the class dev kobj which is necessary for userspace to be able to
react.
> + }
> +
> +exit_password:
> + kfree(buf_cp);
> + return ret ? ret : count;
> +}
> +static struct kobj_attribute password_current_password = __ATTR_WO(current_password);
> +
> +static ssize_t new_password_store(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + char *p, *buf_cp = NULL;
> + int id = 0;
> + int ret = -EIO;
> +
> + buf_cp = kstrdup(buf, GFP_KERNEL);
> + if (!buf_cp) {
> + ret = -ENOMEM;
> + goto exit_password;
> + }
> +
> + p = memchr(buf_cp, '\n', count);
> +
> + if (p != NULL)
> + *p = '\0';
Same as above.
> +
> + id = get_password_instance_id(kobj);
> +
> + if (id >= 0)
> + ret = validate_password_input(id, buf_cp);
> +
> + if (!ret)
> + strscpy(bioscfg_drv.password_data[id].new_password,
> + buf_cp,
> + sizeof(bioscfg_drv.password_data[id].new_password));
> +
> + if (!ret)
> + ret = hp_set_attribute(kobj->name, buf_cp);
> +
> +exit_password:
> + /*
> + * Regardless of the results both new and current passwords
> + * will be set to zero and avoid security issues
> + */
> + clear_passwords(id);
> +
> + kfree(buf_cp);
> + return ret ? ret : count;
> +}
> +
> +static struct kobj_attribute password_new_password = __ATTR_WO(new_password);
> +
> +
> +ATTRIBUTE_N_PROPERTY_SHOW(min_password_length, password);
> +static struct kobj_attribute password_min_password_length = __ATTR_RO(min_password_length);
> +
> +ATTRIBUTE_N_PROPERTY_SHOW(max_password_length, password);
> +static struct kobj_attribute password_max_password_length = __ATTR_RO(max_password_length);
> +
> +static ssize_t role_show(struct kobject *kobj, struct kobj_attribute *attr,
> + char *buf)
> +{
> + if (strcmp(kobj->name, SETUP_PASSWD) == 0)
> + return sysfs_emit(buf, "%s\n", BIOS_ADMIN);
> +
> + if (strcmp(kobj->name, POWER_ON_PASSWD) == 0)
> + return sysfs_emit(buf, "%s\n", POWER_ON);
> +
> + return -EIO;
> +}
> +static struct kobj_attribute password_role = __ATTR_RO(role);
> +
> +static ssize_t mechanism_show(struct kobject *kobj, struct kobj_attribute *attr,
> + char *buf)
> +{
> + int i = get_password_instance_id(kobj);
> +
> + if (i < 0)
> + return i;
> +
> + if (bioscfg_drv.password_data[i].mechanism != PASSWORD)
> + return -EINVAL;
> +
> + return sysfs_emit(buf, "%s\n", PASSWD_MECHANISM_TYPES);
> +}
> +static struct kobj_attribute password_mechanism = __ATTR_RO(mechanism);
> +
> +static ssize_t type_show(struct kobject *kobj, struct kobj_attribute *attr,
> + char *buf)
> +{
> + return sysfs_emit(buf, "password\n");
> +}
> +static struct kobj_attribute password_type = __ATTR_RO(type);
> +
> +ATTRIBUTE_S_COMMON_PROPERTY_SHOW(display_name, password);
> +static struct kobj_attribute password_display_name =
> + __ATTR_RO(display_name);
> +
> +ATTRIBUTE_S_COMMON_PROPERTY_SHOW(display_name_language_code, password);
> +static struct kobj_attribute password_display_langcode =
> + __ATTR_RO(display_name_language_code);
> +
> +ATTRIBUTE_N_COMMON_PROPERTY_SHOW(prerequisites_size, password);
> +static struct kobj_attribute password_prerequisites_size_val =
> + __ATTR_RO(prerequisites_size);
> +
> +ATTRIBUTE_V_COMMON_PROPERTY_SHOW(prerequisites, password);
> +static struct kobj_attribute password_prerequisites_val =
> + __ATTR_RO(prerequisites);
> +
> +ATTRIBUTE_N_PROPERTY_SHOW(encodings_size, password);
> +static struct kobj_attribute password_encodings_size_val =
> + __ATTR_RO(encodings_size);
As before, these size attribute are fairly pointless for userspace as
they can't be relied on.
> +
> +ATTRIBUTE_VALUES_PROPERTY_SHOW(encodings, password);
> +static struct kobj_attribute password_encodings_val =
> + __ATTR_RO(encodings);
> +
> +
> +static struct attribute *password_attrs[] = {
> + &password_is_password_set.attr,
> + &password_min_password_length.attr,
> + &password_max_password_length.attr,
> + &password_current_password.attr,
> + &password_new_password.attr,
> + &password_role.attr,
> + &password_mechanism.attr,
> + &password_type.attr,
> + &password_display_name.attr,
> + &password_display_langcode.attr,
> + &password_prerequisites_size_val.attr,
> + &password_prerequisites_val.attr,
> + &password_encodings_val.attr,
> + &password_encodings_size_val.attr,
> + NULL
> +};
Many of these attributes are not documented.
> +
> +static const struct attribute_group bios_password_attr_group = {
> + .attrs = password_attrs
> +};
> +
> +static const struct attribute_group system_password_attr_group = {
> + .attrs = password_attrs
> +};
These groups are the same, are both needed?
> +
> +int alloc_password_data(void)
> +{
> + int ret = 0;
> +
> + bioscfg_drv.password_instances_count = get_instance_count(HP_WMI_BIOS_PASSWORD_GUID);
> + bioscfg_drv.password_data = kcalloc(bioscfg_drv.password_instances_count,
> + sizeof(struct password_data), GFP_KERNEL);
sizeof(bioscfg_drv.password_data)
> + if (!bioscfg_drv.password_data) {
> + bioscfg_drv.password_instances_count = 0;
> + ret = -ENOMEM;
> + }
> +
> + return ret;
> +}
> +
> +/*
> + * populate_password_package_data -
> + * Populate all properties for an instance under password attribute
> + *
> + * @password_obj: ACPI object with password data
> + * @instance_id: The instance to enumerate
> + * @attr_name_kobj: The parent kernel object
> + */
> +int populate_password_package_data(union acpi_object *password_obj, int instance_id,
> + struct kobject *attr_name_kobj)
> +{
> + bioscfg_drv.password_data[instance_id].attr_name_kobj = attr_name_kobj;
> +
> + populate_password_elements_from_package(password_obj,
> + password_obj->package.count,
> + instance_id);
> +
> + if (strcmp(attr_name_kobj->name, "Setup Password") == 0) {
SETUP_PASSWD
> + /* Save system authentication instance for easy access */
> + return sysfs_create_group(attr_name_kobj, &bios_password_attr_group);
> + }
> +
> + return sysfs_create_group(attr_name_kobj, &system_password_attr_group);
> +}
> +
> +/* Expected Values types associated with each element */
> +static const acpi_object_type expected_password_types[] = {
> + [NAME] = ACPI_TYPE_STRING,
> + [VALUE] = ACPI_TYPE_STRING,
> + [PATH] = ACPI_TYPE_STRING,
> + [IS_READONLY] = ACPI_TYPE_INTEGER,
> + [DISPLAY_IN_UI] = ACPI_TYPE_INTEGER,
> + [REQUIRES_PHYSICAL_PRESENCE] = ACPI_TYPE_INTEGER,
> + [SEQUENCE] = ACPI_TYPE_INTEGER,
> + [PREREQUISITES_SIZE] = ACPI_TYPE_INTEGER,
> + [PREREQUISITES] = ACPI_TYPE_STRING,
> + [SECURITY_LEVEL] = ACPI_TYPE_INTEGER,
> + [PSWD_MIN_LENGTH] = ACPI_TYPE_INTEGER,
> + [PSWD_MAX_LENGTH] = ACPI_TYPE_INTEGER,
> + [PSWD_SIZE] = ACPI_TYPE_INTEGER,
> + [PSWD_ENCODINGS] = ACPI_TYPE_STRING,
> + [PSWD_IS_SET] = ACPI_TYPE_INTEGER
> +};
> +
> +
> +int populate_password_elements_from_package(union acpi_object *password_obj,
> + int password_obj_count,
> + int instance_id)
> +{
> + char *str_value = NULL;
> + int value_len;
> + int ret = 0;
> + u32 size = 0;
> + u32 int_value;
> + int elem = 0;
> + int reqs;
> + int eloc;
> + int pos_values;
> +
> +
> + if (!password_obj)
> + return -EINVAL;
> +
> + strscpy(bioscfg_drv.password_data[instance_id].common.display_name_language_code,
> + LANG_CODE_STR,
> + sizeof(bioscfg_drv.password_data[instance_id].common.display_name_language_code));
> +
> + for (elem = 1, eloc = 1; elem < password_obj_count; elem++, eloc++) {
> +
> + /* ONLY look at the first PASSWORD_ELEM_CNT elements */
> + if (eloc == PASSWORD_ELEM_CNT)
> + goto exit_package;
> +
> + switch (password_obj[elem].type) {
> + case ACPI_TYPE_STRING:
> +
> + if (PREREQUISITES != elem && PSWD_ENCODINGS != elem) {
> + ret = convert_hexstr_to_str(password_obj[elem].string.pointer,
> + password_obj[elem].string.length,
> + &str_value, &value_len);
> + if (ret)
> + continue;
> + }
> + break;
> + case ACPI_TYPE_INTEGER:
> + int_value = (u32)password_obj[elem].integer.value;
> + break;
> + default:
> + pr_warn("Unsupported object type [%d]\n", password_obj[elem].type);
> + continue;
> + }
> +
> + /* Check that both expected and read object type match */
> + if (expected_password_types[eloc] != password_obj[elem].type) {
> + pr_err("Error expected type %d for elem %d, but got type %d instead\n",
> + expected_password_types[eloc], elem, password_obj[elem].type);
> + return -EIO;
> + }
> +
> + /* Assign appropriate element value to corresponding field*/
> + switch (eloc) {
> + case VALUE:
> + break;
> + case PATH:
> + strscpy(bioscfg_drv.password_data[instance_id].common.path, str_value,
> + sizeof(bioscfg_drv.password_data[instance_id].common.path));
> + break;
> + case IS_READONLY:
> + bioscfg_drv.password_data[instance_id].common.is_readonly = int_value;
> + break;
> + case DISPLAY_IN_UI:
> + bioscfg_drv.password_data[instance_id].common.display_in_ui = int_value;
> + break;
> + case REQUIRES_PHYSICAL_PRESENCE:
> + bioscfg_drv.password_data[instance_id].common.requires_physical_presence = int_value;
> + break;
> + case SEQUENCE:
> + bioscfg_drv.password_data[instance_id].common.sequence = int_value;
> + break;
> + case PREREQUISITES_SIZE:
> + bioscfg_drv.password_data[instance_id].common.prerequisites_size = int_value;
> + if (int_value > MAX_PREREQUISITES_SIZE)
> + pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
> + /*
> + * This HACK is needed to keep the expected
> + * element list pointing to the right obj[elem].type
> + * when the size is zero. PREREQUISITES
> + * object is omitted by BIOS when the size is
> + * zero.
> + */
> + if (int_value == 0)
> + eloc++;
> + break;
> + case PREREQUISITES:
> + size = bioscfg_drv.password_data[instance_id].common.prerequisites_size;
> +
> + for (reqs = 0; reqs < size; reqs++) {
> + ret = convert_hexstr_to_str(password_obj[elem + reqs].string.pointer,
> + password_obj[elem + reqs].string.length,
> + &str_value, &value_len);
> +
> + if (ret)
> + break;
> +
> + strscpy(bioscfg_drv.password_data[instance_id].common.prerequisites[reqs],
> + str_value,
> + sizeof(bioscfg_drv.password_data[instance_id].common.prerequisites[reqs]));
> +
> + kfree(str_value);
> + }
> + break;
> +
> + case SECURITY_LEVEL:
> + bioscfg_drv.password_data[instance_id].common.security_level = int_value;
> + break;
> +
> + case PSWD_MIN_LENGTH:
> + bioscfg_drv.password_data[instance_id].min_password_length = int_value;
> + break;
> + case PSWD_MAX_LENGTH:
> + bioscfg_drv.password_data[instance_id].max_password_length = int_value;
> + break;
> + case PSWD_SIZE:
> + bioscfg_drv.password_data[instance_id].encodings_size = int_value;
> + if (int_value > MAX_ENCODINGS_SIZE)
> + pr_warn("Password Encoding size value exceeded the maximum number of elements supported or data may be malformed\n");
> +
> + /*
> + * This HACK is needed to keep the expected
> + * element list pointing to the right obj[elem].type
> + * when the size is zero. PSWD_ENCODINGS
> + * object is omitted by BIOS when the size is
> + * zero.
> + */
> + if (int_value == 0)
> + eloc++;
> + break;
> +
> + case PSWD_ENCODINGS:
> + size = bioscfg_drv.password_data[instance_id].encodings_size;
> +
> + for (pos_values = 0; pos_values < size && pos_values < MAX_ENCODINGS_SIZE; pos_values++) {
> + ret = convert_hexstr_to_str(password_obj[elem + pos_values].string.pointer,
> + password_obj[elem + pos_values].string.length,
> + &str_value, &value_len);
> + if (ret)
> + break;
> +
> + strscpy(bioscfg_drv.password_data[instance_id].encodings[pos_values],
> + str_value,
> + sizeof(bioscfg_drv.password_data[instance_id].encodings[pos_values]));
> + kfree(str_value);
> + }
> + break;
> + case PSWD_IS_SET:
> + bioscfg_drv.password_data[instance_id].is_enabled = int_value;
> + break;
> +
> + default:
> + pr_warn("Invalid element: %d found in Password attribute or data may be malformed\n", elem);
> + break;
> + }
> + kfree(str_value);
> + }
> +
> +exit_package:
> + kfree(str_value);
> + return 0;
> +}
> +
> +/*
> + * populate_password_buffer_data -
> + * Populate all properties for an instance under password object attribute
> + *
> + * @buffer_ptr: Buffer pointer
> + * @buffer_size: Buffer size
> + * @instance_id: The instance to enumerate
> + * @attr_name_kobj: The parent kernel object
> + */
> +int populate_password_buffer_data(u8 *buffer_ptr, u32 *buffer_size, int instance_id,
> + struct kobject *attr_name_kobj)
> +{
> + bioscfg_drv.password_data[instance_id].attr_name_kobj = attr_name_kobj;
> +
> + /* Populate Password attributes */
> + populate_password_elements_from_buffer(buffer_ptr, buffer_size,
> + instance_id);
> + friendly_user_name_update(bioscfg_drv.password_data[instance_id].common.path,
> + attr_name_kobj->name,
> + bioscfg_drv.password_data[instance_id].common.display_name,
> + sizeof(bioscfg_drv.password_data[instance_id].common.display_name));
> + if (strcmp(attr_name_kobj->name, "Setup Password") == 0)
> + return sysfs_create_group(attr_name_kobj, &bios_password_attr_group);
> +
> + return sysfs_create_group(attr_name_kobj, &system_password_attr_group);
> +}
> +
> +int populate_password_elements_from_buffer(u8 *buffer_ptr, u32 *buffer_size,
> + int instance_id)
> +{
> + int ret;
> + char *dst = NULL;
> + int elem;
> + int reqs;
> + int integer;
> + int size = 0;
> + int values;
> + int dst_size = *buffer_size / sizeof(u16);
> +
> + dst = kcalloc(dst_size, sizeof(char), GFP_KERNEL);
> + if (!dst)
> + return -ENOMEM;
> +
> + elem = 0;
> + strscpy(bioscfg_drv.password_data[instance_id].common.display_name_language_code,
> + LANG_CODE_STR,
> + sizeof(bioscfg_drv.password_data[instance_id].common.display_name_language_code));
> +
> + for (elem = 1; elem < 3; elem++) {
> +
> + ret = get_string_from_buffer(&buffer_ptr, buffer_size, dst, dst_size);
> + if (ret < 0)
> + continue;
> +
> + switch (elem) {
> + case VALUE:
> + strscpy(bioscfg_drv.password_data[instance_id].current_password,
> + dst, sizeof(bioscfg_drv.password_data[instance_id].current_password));
> + break;
> + case PATH:
> + strscpy(bioscfg_drv.password_data[instance_id].common.path, dst,
> + sizeof(bioscfg_drv.password_data[instance_id].common.path));
> + break;
> + default:
> + pr_warn("Invalid element: %d found in Password attribute or data may be malformed\n", elem);
> + break;
> + }
> + }
> +
> + for (elem = 3; elem < PASSWORD_ELEM_CNT; elem++) {
> +
> + if (elem != PREREQUISITES && elem != PSWD_ENCODINGS) {
> + ret = get_integer_from_buffer((int **)&buffer_ptr, buffer_size, (int *)&integer);
> + if (ret)
> + continue;
> + }
> +
> + switch (elem) {
> + case IS_READONLY:
> + bioscfg_drv.password_data[instance_id].common.is_readonly = integer;
> + break;
> + case DISPLAY_IN_UI:
> + bioscfg_drv.password_data[instance_id].common.display_in_ui = integer;
> + break;
> + case REQUIRES_PHYSICAL_PRESENCE:
> + bioscfg_drv.password_data[instance_id].common.requires_physical_presence = integer;
> + break;
> + case SEQUENCE:
> + bioscfg_drv.password_data[instance_id].common.sequence = integer;
> + break;
> + case PREREQUISITES_SIZE:
> + bioscfg_drv.password_data[instance_id].common.prerequisites_size = integer;
> + if (integer > MAX_PREREQUISITES_SIZE)
> + pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
> +
> + // PREREQUISITES:
> + elem++;
> + size = bioscfg_drv.password_data[instance_id].common.prerequisites_size;
> + for (reqs = 0; reqs < size && reqs > MAX_PREREQUISITES_SIZE; reqs++) {
> + ret = get_string_from_buffer(&buffer_ptr, buffer_size, dst, dst_size);
> + if (ret < 0)
> + continue;
> +
> + strscpy(bioscfg_drv.password_data[instance_id].common.prerequisites[reqs],
> + dst,
> + sizeof(bioscfg_drv.password_data[instance_id].common.prerequisites[reqs]));
> + }
> + break;
> + case SECURITY_LEVEL:
> + bioscfg_drv.password_data[instance_id].common.security_level = integer;
> + break;
> +
> + case PSWD_MIN_LENGTH:
> + bioscfg_drv.password_data[instance_id].min_password_length = integer;
> + break;
> + case PSWD_MAX_LENGTH:
> + bioscfg_drv.password_data[instance_id].max_password_length = integer;
> + break;
> + case PSWD_SIZE:
> + bioscfg_drv.password_data[instance_id].encodings_size = integer;
> + if (integer > MAX_ENCODINGS_SIZE)
> + pr_warn("Password Encoding size value exceeded the maximum number of elements supported or data may be malformed\n");
> +
> + // PSWD_ENCODINGS:
> + elem++;
> + size = bioscfg_drv.password_data[instance_id].encodings_size;
> + for (values = 0; values < size && values < MAX_ENCODINGS_SIZE; values++) {
> + ret = get_string_from_buffer(&buffer_ptr, buffer_size, dst, dst_size);
> + if (ret < 0)
> + continue;
> +
> + strscpy(bioscfg_drv.password_data[instance_id].encodings[values],
> + dst,
> + sizeof(bioscfg_drv.password_data[instance_id].encodings[values]));
> +
> + }
> + break;
> + case PSWD_IS_SET:
> + bioscfg_drv.password_data[instance_id].is_enabled = integer;
> + break;
> + default:
> + pr_warn("Invalid element: %d found in Password attribute or data may be malformed\n", elem);
> + break;
> + }
> + }
> + kfree(dst);
> + return 0;
> +}
> +
> +/*
> + * exit_password_attributes() - Clear all attribute data
> + *
> + * Clears all data allocated for this group of attributes
> + */
> +void exit_password_attributes(void)
> +{
> + int instance_id;
> +
> + for (instance_id = 0; instance_id < bioscfg_drv.password_instances_count; instance_id++) {
> + struct kobject *attr_name_kobj = bioscfg_drv.password_data[instance_id].attr_name_kobj;
> +
> + if (attr_name_kobj) {
> + if (strcmp(attr_name_kobj->name, SETUP_PASSWD) == 0)
> + sysfs_remove_group(attr_name_kobj,
> + &bios_password_attr_group);
> + else
> + sysfs_remove_group(attr_name_kobj,
> + &system_password_attr_group);
> + }
> + }
> + bioscfg_drv.password_instances_count = 0;
> + kfree(bioscfg_drv.password_data);
> + bioscfg_drv.password_data = NULL;
> +}
> --
> 2.34.1
>
Hi,
On 4/23/23 11:07, thomas@t-8ch.de wrote:
> On 2023-04-20 11:54:46-0500, Jorge Lopez wrote:
>> ---
>> .../x86/hp/hp-bioscfg/passwdobj-attributes.c | 669 ++++++++++++++++++
>> 1 file changed, 669 insertions(+)
>> create mode 100644 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
>>
>> diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
>> new file mode 100644
>> index 000000000000..c03b3a71e9c4
>> --- /dev/null
>> +++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
<snip>
>> +static ssize_t current_password_store(struct kobject *kobj,
>> + struct kobj_attribute *attr,
>> + const char *buf, size_t count)
>> +{
>> + char *p, *buf_cp;
>> + int id, ret = 0;
>> +
>> + buf_cp = kstrdup(buf, GFP_KERNEL);
>> + if (!buf_cp) {
>> + ret = -ENOMEM;
>> + goto exit_password;
>> + }
>> +
>> + p = memchr(buf_cp, '\n', count);
>> +
>> + if (p != NULL)
>> + *p = '\0';
>
> This will also accept input like "foo\nbar" and truncate away the "bar".
That is true, but stripping '\n' at the end is a pretty standard
pattern for sysfs attr store functions since users will e.g.
often do:
echo one-string-out-of-a-few-valid-strings > /sys/.../some-enum-attr
Where to actually write the real valid string the user should do:
echo -n one-string-out-of-a-few-valid-strings > /sys/.../some-enum-attr
See e.g.:
drivers/platform/x86/dell/dell-wmi-sysman/passobj-attributes.c new_password_store()
which does the exact same thing.
The stripping of '\n' is often taken care of by various kernel
helpers for sysfs attr.
> For something like a password it seems errorprone to try to munge the
> value.
Almost all password input dialogs including the actual BIOS password
input dialog will consider the enter key / a new-line to mean
"end-of-password, please validate the password inputted so far"
So I don't think this is really a problem. With that said we
could make this more robust (and maybe also change the Dell
code to match) by doing:
len = strlen(buf_cp);
if (len && buf_cp[len - 1] == '\n')
buf_cp[len - 1] = 0;
To ensure that we only ever strip a leading '\n'
and not one in the middle of the buffer.
Regards,
Hans
On Sun, Apr 23, 2023 at 4:07 AM <thomas@t-8ch.de> wrote:
>
> On 2023-04-20 11:54:46-0500, Jorge Lopez wrote:
> > ---
> > .../x86/hp/hp-bioscfg/passwdobj-attributes.c | 669 ++++++++++++++++++
> > 1 file changed, 669 insertions(+)
> > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> >
> > diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> > new file mode 100644
> > index 000000000000..c03b3a71e9c4
> > --- /dev/null
> > +++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> > @@ -0,0 +1,669 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
<snip>
> > +int validate_password_input(int instance_id, const char *buf)
> > +{
> > + int length;
> > +
> > + length = strlen(buf);
> > + if (buf[length-1] == '\n')
> > + length--;
> > +
> > + if (length > MAX_PASSWD_SIZE)
> > + return INVALID_BIOS_AUTH;
> > +
> > + if (bioscfg_drv.password_data[instance_id].min_password_length > length ||
> > + bioscfg_drv.password_data[instance_id].max_password_length < length)
> > + return INVALID_BIOS_AUTH;
> > + return SUCCESS;
> > +}
> > +
> > +int password_is_set(const char *name)
>
> bool is_password_set(const char *name)
Function is invoked nowhere. It will be removed.
>
> > +{
> > + int id;
> > +
> > + id = get_password_instance_for_type(name);
> > + if (id < 0)
> > + return 0;
> > +
> > + return bioscfg_drv.password_data[id].is_enabled;
> > +}
> > +
> > +ATTRIBUTE_N_PROPERTY_SHOW(is_enabled, password);
> > +static struct kobj_attribute password_is_password_set = __ATTR_RO(is_enabled);
> > +
> > +static ssize_t current_password_store(struct kobject *kobj,
> > + struct kobj_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + char *p, *buf_cp;
> > + int id, ret = 0;
> > +
> > + buf_cp = kstrdup(buf, GFP_KERNEL);
> > + if (!buf_cp) {
> > + ret = -ENOMEM;
> > + goto exit_password;
> > + }
> > +
> > + p = memchr(buf_cp, '\n', count);
> > +
> > + if (p != NULL)
> > + *p = '\0';
>
> This will also accept input like "foo\nbar" and truncate away the "bar".
>
> For something like a password it seems errorprone to try to munge the
> value.
This is an expected behavior. If the user enters '\n' as part of the
password, the buffer data will be truncated since only one line per
sysfs file is permitted.
>
> > +
> > + id = get_password_instance_id(kobj);
> > +
> > + if (id >= 0)
> > + ret = validate_password_input(id, buf_cp);
> > +
> > + if (!ret) {
> > + strscpy(bioscfg_drv.password_data[id].current_password,
> > + buf_cp,
> > + sizeof(bioscfg_drv.password_data[id].current_password));
> > + /*
> > + * set pending reboot flag depending on
> > + * "RequiresPhysicalPresence" value
> > + */
> > + if (bioscfg_drv.password_data[id].common.requires_physical_presence)
> > + bioscfg_drv.pending_reboot = true;
>
> Just setting this to true does not emit the necessary KOBJ_CHANGE event
> on the class dev kobj which is necessary for userspace to be able to
> react.
This feature was added outside of the original design specification to
be used at a later time.
Changes to the value to true does not emit a KOBJ_CHANGE event.
>
> > + }
> > +
> > +exit_password:
> > + kfree(buf_cp);
> > + return ret ? ret : count;
> > +}
> > +static struct kobj_attribute password_current_password = __ATTR_WO(current_password);
> > +
> > +static ssize_t new_password_store(struct kobject *kobj,
> > + struct kobj_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + char *p, *buf_cp = NULL;
> > + int id = 0;
> > + int ret = -EIO;
> > +
> > + buf_cp = kstrdup(buf, GFP_KERNEL);
> > + if (!buf_cp) {
> > + ret = -ENOMEM;
> > + goto exit_password;
> > + }
> > +
> > + p = memchr(buf_cp, '\n', count);
> > +
> > + if (p != NULL)
> > + *p = '\0';
>
> Same as above.
This is an expected behavior. If the user enters '\n' as part of the
password, the buffer data will be truncated since only one line per
sysfs file is permitted.
>
> > +
> > + id = get_password_instance_id(kobj);
> > +
> > + if (id >= 0)
> > + ret = validate_password_input(id, buf_cp);
> > +
> > + if (!ret)
> > + strscpy(bioscfg_drv.password_data[id].new_password,
> > + buf_cp,
> > + sizeof(bioscfg_drv.password_data[id].new_password));
> > +
> > + if (!ret)
> > + ret = hp_set_attribute(kobj->name, buf_cp);
> > +
> > +exit_password:
> > + /*
> > + * Regardless of the results both new and current passwords
> > + * will be set to zero and avoid security issues
> > + */
> > + clear_passwords(id);
> > +
> > + kfree(buf_cp);
> > + return ret ? ret : count;
> > +}
> > +
> > +static struct kobj_attribute password_new_password = __ATTR_WO(new_password);
> > +
> > +
> > +ATTRIBUTE_N_PROPERTY_SHOW(min_password_length, password);
> > +static struct kobj_attribute password_min_password_length = __ATTR_RO(min_password_length);
> > +
> > +ATTRIBUTE_N_PROPERTY_SHOW(max_password_length, password);
> > +static struct kobj_attribute password_max_password_length = __ATTR_RO(max_password_length);
> > +
> > +static ssize_t role_show(struct kobject *kobj, struct kobj_attribute *attr,
> > + char *buf)
> > +{
> > + if (strcmp(kobj->name, SETUP_PASSWD) == 0)
> > + return sysfs_emit(buf, "%s\n", BIOS_ADMIN);
> > +
> > + if (strcmp(kobj->name, POWER_ON_PASSWD) == 0)
> > + return sysfs_emit(buf, "%s\n", POWER_ON);
> > +
> > + return -EIO;
> > +}
> > +static struct kobj_attribute password_role = __ATTR_RO(role);
> > +
> > +static ssize_t mechanism_show(struct kobject *kobj, struct kobj_attribute *attr,
> > + char *buf)
> > +{
> > + int i = get_password_instance_id(kobj);
> > +
> > + if (i < 0)
> > + return i;
> > +
> > + if (bioscfg_drv.password_data[i].mechanism != PASSWORD)
> > + return -EINVAL;
> > +
> > + return sysfs_emit(buf, "%s\n", PASSWD_MECHANISM_TYPES);
> > +}
> > +static struct kobj_attribute password_mechanism = __ATTR_RO(mechanism);
> > +
> > +static ssize_t type_show(struct kobject *kobj, struct kobj_attribute *attr,
> > + char *buf)
> > +{
> > + return sysfs_emit(buf, "password\n");
> > +}
> > +static struct kobj_attribute password_type = __ATTR_RO(type);
> > +
> > +ATTRIBUTE_S_COMMON_PROPERTY_SHOW(display_name, password);
> > +static struct kobj_attribute password_display_name =
> > + __ATTR_RO(display_name);
> > +
> > +ATTRIBUTE_S_COMMON_PROPERTY_SHOW(display_name_language_code, password);
> > +static struct kobj_attribute password_display_langcode =
> > + __ATTR_RO(display_name_language_code);
> > +
> > +ATTRIBUTE_N_COMMON_PROPERTY_SHOW(prerequisites_size, password);
> > +static struct kobj_attribute password_prerequisites_size_val =
> > + __ATTR_RO(prerequisites_size);
> > +
> > +ATTRIBUTE_V_COMMON_PROPERTY_SHOW(prerequisites, password);
> > +static struct kobj_attribute password_prerequisites_val =
> > + __ATTR_RO(prerequisites);
> > +
> > +ATTRIBUTE_N_PROPERTY_SHOW(encodings_size, password);
> > +static struct kobj_attribute password_encodings_size_val =
> > + __ATTR_RO(encodings_size);
>
> As before, these size attribute are fairly pointless for userspace as
> they can't be relied on.
I will remove the attribute from being reported in sysfs but they will
be kept as part of the driver internal data
>
> > +
> > +ATTRIBUTE_VALUES_PROPERTY_SHOW(encodings, password);
> > +static struct kobj_attribute password_encodings_val =
> > + __ATTR_RO(encodings);
> > +
> > +
> > +static struct attribute *password_attrs[] = {
> > + &password_is_password_set.attr,
> > + &password_min_password_length.attr,
> > + &password_max_password_length.attr,
> > + &password_current_password.attr,
> > + &password_new_password.attr,
> > + &password_role.attr,
> > + &password_mechanism.attr,
> > + &password_type.attr,
> > + &password_display_name.attr,
> > + &password_display_langcode.attr,
> > + &password_prerequisites_size_val.attr,
> > + &password_prerequisites_val.attr,
> > + &password_encodings_val.attr,
> > + &password_encodings_size_val.attr,
> > + NULL
> > +};
>
> Many of these attributes are not documented.
Those attributes are documented under authentication section, lines 150-329
What: /sys/class/firmware-attributes/*/authentication/
Date: February 2021
KernelVersion: 5.11
Contact: Divya Bharathi <Divya.Bharathi@Dell.com>,
Prasanth KSR <prasanth.ksr@dell.com>
Dell.Client.Kernel@dell.com
Description:
Devices support various authentication mechanisms which can be exposed
as a separate configuration object.
>
> > +
> > +static const struct attribute_group bios_password_attr_group = {
> > + .attrs = password_attrs
> > +};
> > +
> > +static const struct attribute_group system_password_attr_group = {
> > + .attrs = password_attrs
> > +};
>
> These groups are the same, are both needed?
Yes. They will show under 'Setup Password' and 'Power-on password'
>
> > +
> > +int alloc_password_data(void)
> > +{
> > + int ret = 0;
> > +
> > + bioscfg_drv.password_instances_count = get_instance_count(HP_WMI_BIOS_PASSWORD_GUID);
> > + bioscfg_drv.password_data = kcalloc(bioscfg_drv.password_instances_count,
> > + sizeof(struct password_data), GFP_KERNEL);
>
> sizeof(bioscfg_drv.password_data)
>
> > + if (!bioscfg_drv.password_data) {
> > + bioscfg_drv.password_instances_count = 0;
> > + ret = -ENOMEM;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +/*
> > + * populate_password_package_data -
> > + * Populate all properties for an instance under password attribute
> > + *
> > + * @password_obj: ACPI object with password data
> > + * @instance_id: The instance to enumerate
> > + * @attr_name_kobj: The parent kernel object
> > + */
> > +int populate_password_package_data(union acpi_object *password_obj, int instance_id,
> > + struct kobject *attr_name_kobj)
> > +{
> > + bioscfg_drv.password_data[instance_id].attr_name_kobj = attr_name_kobj;
> > +
> > + populate_password_elements_from_package(password_obj,
> > + password_obj->package.count,
> > + instance_id);
> > +
> > + if (strcmp(attr_name_kobj->name, "Setup Password") == 0) {
>
> SETUP_PASSWD
Done!
>
<snip>
On 2023-05-04 15:29:21-0500, Jorge Lopez wrote:
> On Sun, Apr 23, 2023 at 4:07 AM <thomas@t-8ch.de> wrote:
> >
> > On 2023-04-20 11:54:46-0500, Jorge Lopez wrote:
> > > ---
> > > .../x86/hp/hp-bioscfg/passwdobj-attributes.c | 669 ++++++++++++++++++
> > > 1 file changed, 669 insertions(+)
> > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> > >
> > > diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> > > new file mode 100644
> > > index 000000000000..c03b3a71e9c4
> > > --- /dev/null
> > > +++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> > > @@ -0,0 +1,669 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
>
> <snip>
>
> > > +int validate_password_input(int instance_id, const char *buf)
> > > +{
> > > + int length;
> > > +
> > > + length = strlen(buf);
> > > + if (buf[length-1] == '\n')
> > > + length--;
> > > +
> > > + if (length > MAX_PASSWD_SIZE)
> > > + return INVALID_BIOS_AUTH;
> > > +
> > > + if (bioscfg_drv.password_data[instance_id].min_password_length > length ||
> > > + bioscfg_drv.password_data[instance_id].max_password_length < length)
> > > + return INVALID_BIOS_AUTH;
> > > + return SUCCESS;
> > > +}
> > > +
> > > +int password_is_set(const char *name)
> >
> > bool is_password_set(const char *name)
>
> Function is invoked nowhere. It will be removed.
> >
> > > +{
> > > + int id;
> > > +
> > > + id = get_password_instance_for_type(name);
> > > + if (id < 0)
> > > + return 0;
> > > +
> > > + return bioscfg_drv.password_data[id].is_enabled;
> > > +}
> > > +
> > > +ATTRIBUTE_N_PROPERTY_SHOW(is_enabled, password);
> > > +static struct kobj_attribute password_is_password_set = __ATTR_RO(is_enabled);
> > > +
> > > +static ssize_t current_password_store(struct kobject *kobj,
> > > + struct kobj_attribute *attr,
> > > + const char *buf, size_t count)
> > > +{
> > > + char *p, *buf_cp;
> > > + int id, ret = 0;
> > > +
> > > + buf_cp = kstrdup(buf, GFP_KERNEL);
> > > + if (!buf_cp) {
> > > + ret = -ENOMEM;
> > > + goto exit_password;
> > > + }
> > > +
> > > + p = memchr(buf_cp, '\n', count);
> > > +
> > > + if (p != NULL)
> > > + *p = '\0';
> >
> > This will also accept input like "foo\nbar" and truncate away the "bar".
> >
> > For something like a password it seems errorprone to try to munge the
> > value.
>
> This is an expected behavior. If the user enters '\n' as part of the
> password, the buffer data will be truncated since only one line per
> sysfs file is permitted.
As discussed in another patch this would silently truncate the input on
the first newline character; even if it is not the last character of the
input string.
This should use the same helper as the other files to only strip a
newline at the end of the input.
> >
> > > +
> > > + id = get_password_instance_id(kobj);
> > > +
> > > + if (id >= 0)
> > > + ret = validate_password_input(id, buf_cp);
> > > +
> > > + if (!ret) {
> > > + strscpy(bioscfg_drv.password_data[id].current_password,
> > > + buf_cp,
> > > + sizeof(bioscfg_drv.password_data[id].current_password));
> > > + /*
> > > + * set pending reboot flag depending on
> > > + * "RequiresPhysicalPresence" value
> > > + */
> > > + if (bioscfg_drv.password_data[id].common.requires_physical_presence)
> > > + bioscfg_drv.pending_reboot = true;
> >
> > Just setting this to true does not emit the necessary KOBJ_CHANGE event
> > on the class dev kobj which is necessary for userspace to be able to
> > react.
>
> This feature was added outside of the original design specification to
> be used at a later time.
> Changes to the value to true does not emit a KOBJ_CHANGE event.
This contradicts the documentation:
A read-only attribute reads 1 if a reboot is necessary to apply
pending BIOS attribute changes. Also, an uevent_KOBJ_CHANGE is
generated when it changes to 1.
This will confuse userspace, there are generic userspace applications
waiting for those events.
If there is a reason for not emitting them it should be good and
documented.
Also according to the docs the authentication attributes should
KOBJ_CHANGE events. I think this also affects this driver and should be
implemented.
> >
> > > + }
> > > +
> > > +exit_password:
> > > + kfree(buf_cp);
> > > + return ret ? ret : count;
> > > +}
> > > +static struct kobj_attribute password_current_password = __ATTR_WO(current_password);
> > > +
> > > +static ssize_t new_password_store(struct kobject *kobj,
> > > + struct kobj_attribute *attr,
> > > + const char *buf, size_t count)
> > > +{
> > > + char *p, *buf_cp = NULL;
> > > + int id = 0;
> > > + int ret = -EIO;
> > > +
> > > + buf_cp = kstrdup(buf, GFP_KERNEL);
> > > + if (!buf_cp) {
> > > + ret = -ENOMEM;
> > > + goto exit_password;
> > > + }
> > > +
> > > + p = memchr(buf_cp, '\n', count);
> > > +
> > > + if (p != NULL)
> > > + *p = '\0';
> >
> > Same as above.
>
> This is an expected behavior. If the user enters '\n' as part of the
> password, the buffer data will be truncated since only one line per
> sysfs file is permitted.
If a user accidentally presses enter before entering a password this
may set the password to the empty string; surprising.
This should really use the helper.
> >
> > > +
> > > + id = get_password_instance_id(kobj);
> > > +
> > > + if (id >= 0)
> > > + ret = validate_password_input(id, buf_cp);
> > > +
> > > + if (!ret)
> > > + strscpy(bioscfg_drv.password_data[id].new_password,
> > > + buf_cp,
> > > + sizeof(bioscfg_drv.password_data[id].new_password));
> > > +
> > > + if (!ret)
> > > + ret = hp_set_attribute(kobj->name, buf_cp);
> > > +
> > > +exit_password:
> > > + /*
> > > + * Regardless of the results both new and current passwords
> > > + * will be set to zero and avoid security issues
> > > + */
> > > + clear_passwords(id);
> > > +
> > > + kfree(buf_cp);
> > > + return ret ? ret : count;
> > > +}
> > > +
> > > +static struct kobj_attribute password_new_password = __ATTR_WO(new_password);
> > > +
> > > +
> > > +ATTRIBUTE_N_PROPERTY_SHOW(min_password_length, password);
> > > +static struct kobj_attribute password_min_password_length = __ATTR_RO(min_password_length);
> > > +
> > > +ATTRIBUTE_N_PROPERTY_SHOW(max_password_length, password);
> > > +static struct kobj_attribute password_max_password_length = __ATTR_RO(max_password_length);
> > > +
> > > +static ssize_t role_show(struct kobject *kobj, struct kobj_attribute *attr,
> > > + char *buf)
> > > +{
> > > + if (strcmp(kobj->name, SETUP_PASSWD) == 0)
> > > + return sysfs_emit(buf, "%s\n", BIOS_ADMIN);
> > > +
> > > + if (strcmp(kobj->name, POWER_ON_PASSWD) == 0)
> > > + return sysfs_emit(buf, "%s\n", POWER_ON);
> > > +
> > > + return -EIO;
> > > +}
> > > +static struct kobj_attribute password_role = __ATTR_RO(role);
> > > +
> > > +static ssize_t mechanism_show(struct kobject *kobj, struct kobj_attribute *attr,
> > > + char *buf)
> > > +{
> > > + int i = get_password_instance_id(kobj);
> > > +
> > > + if (i < 0)
> > > + return i;
> > > +
> > > + if (bioscfg_drv.password_data[i].mechanism != PASSWORD)
> > > + return -EINVAL;
> > > +
> > > + return sysfs_emit(buf, "%s\n", PASSWD_MECHANISM_TYPES);
> > > +}
> > > +static struct kobj_attribute password_mechanism = __ATTR_RO(mechanism);
> > > +
> > > +static ssize_t type_show(struct kobject *kobj, struct kobj_attribute *attr,
> > > + char *buf)
> > > +{
> > > + return sysfs_emit(buf, "password\n");
> > > +}
> > > +static struct kobj_attribute password_type = __ATTR_RO(type);
> > > +
> > > +ATTRIBUTE_S_COMMON_PROPERTY_SHOW(display_name, password);
> > > +static struct kobj_attribute password_display_name =
> > > + __ATTR_RO(display_name);
> > > +
> > > +ATTRIBUTE_S_COMMON_PROPERTY_SHOW(display_name_language_code, password);
> > > +static struct kobj_attribute password_display_langcode =
> > > + __ATTR_RO(display_name_language_code);
> > > +
> > > +ATTRIBUTE_N_COMMON_PROPERTY_SHOW(prerequisites_size, password);
> > > +static struct kobj_attribute password_prerequisites_size_val =
> > > + __ATTR_RO(prerequisites_size);
> > > +
> > > +ATTRIBUTE_V_COMMON_PROPERTY_SHOW(prerequisites, password);
> > > +static struct kobj_attribute password_prerequisites_val =
> > > + __ATTR_RO(prerequisites);
> > > +
> > > +ATTRIBUTE_N_PROPERTY_SHOW(encodings_size, password);
> > > +static struct kobj_attribute password_encodings_size_val =
> > > + __ATTR_RO(encodings_size);
> >
> > As before, these size attribute are fairly pointless for userspace as
> > they can't be relied on.
>
> I will remove the attribute from being reported in sysfs but they will
> be kept as part of the driver internal data
>
> >
> > > +
> > > +ATTRIBUTE_VALUES_PROPERTY_SHOW(encodings, password);
> > > +static struct kobj_attribute password_encodings_val =
> > > + __ATTR_RO(encodings);
> > > +
> > > +
> > > +static struct attribute *password_attrs[] = {
> > > + &password_is_password_set.attr,
> > > + &password_min_password_length.attr,
> > > + &password_max_password_length.attr,
> > > + &password_current_password.attr,
> > > + &password_new_password.attr,
> > > + &password_role.attr,
> > > + &password_mechanism.attr,
> > > + &password_type.attr,
> > > + &password_display_name.attr,
> > > + &password_display_langcode.attr,
> > > + &password_prerequisites_size_val.attr,
> > > + &password_prerequisites_val.attr,
> > > + &password_encodings_val.attr,
> > > + &password_encodings_size_val.attr,
> > > + NULL
> > > +};
> >
> > Many of these attributes are not documented.
>
> Those attributes are documented under authentication section, lines 150-329
>
> What: /sys/class/firmware-attributes/*/authentication/
> Date: February 2021
> KernelVersion: 5.11
> Contact: Divya Bharathi <Divya.Bharathi@Dell.com>,
> Prasanth KSR <prasanth.ksr@dell.com>
> Dell.Client.Kernel@dell.com
> Description:
> Devices support various authentication mechanisms which can be exposed
> as a separate configuration object.
If I read that correctly the authentication attributes are not "normal"
attributes.
So they don't need "type", "display_name", "display_langcode".
Do they need prerequisites?
>
>
> >
> > > +
> > > +static const struct attribute_group bios_password_attr_group = {
> > > + .attrs = password_attrs
> > > +};
> > > +
> > > +static const struct attribute_group system_password_attr_group = {
> > > + .attrs = password_attrs
> > > +};
> >
> > These groups are the same, are both needed?
>
> Yes. They will show under 'Setup Password' and 'Power-on password'
These are identical constant structures. It should be possible to have
only one and use it for both usecases.
<snip>
On Thu, May 4, 2023 at 3:59 PM Thomas Weißschuh <thomas@t-8ch.de> wrote:
>
> On 2023-05-04 15:29:21-0500, Jorge Lopez wrote:
> > On Sun, Apr 23, 2023 at 4:07 AM <thomas@t-8ch.de> wrote:
> > >
> > > On 2023-04-20 11:54:46-0500, Jorge Lopez wrote:
> > > > ---
> > > > .../x86/hp/hp-bioscfg/passwdobj-attributes.c | 669 ++++++++++++++++++
> > > > 1 file changed, 669 insertions(+)
> > > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> > > >
> > > > diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> > > > new file mode 100644
> > > > index 000000000000..c03b3a71e9c4
> > > > --- /dev/null
> > > > +++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> > > > @@ -0,0 +1,669 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> >
> > <snip>
> >
> > > > +int validate_password_input(int instance_id, const char *buf)
> > > > +{
> > > > + int length;
> > > > +
> > > > + length = strlen(buf);
> > > > + if (buf[length-1] == '\n')
> > > > + length--;
> > > > +
> > > > + if (length > MAX_PASSWD_SIZE)
> > > > + return INVALID_BIOS_AUTH;
> > > > +
> > > > + if (bioscfg_drv.password_data[instance_id].min_password_length > length ||
> > > > + bioscfg_drv.password_data[instance_id].max_password_length < length)
> > > > + return INVALID_BIOS_AUTH;
> > > > + return SUCCESS;
> > > > +}
> > > > +
> > > > +int password_is_set(const char *name)
> > >
> > > bool is_password_set(const char *name)
> >
> > Function is invoked nowhere. It will be removed.
> > >
> > > > +{
> > > > + int id;
> > > > +
> > > > + id = get_password_instance_for_type(name);
> > > > + if (id < 0)
> > > > + return 0;
> > > > +
> > > > + return bioscfg_drv.password_data[id].is_enabled;
> > > > +}
> > > > +
> > > > +ATTRIBUTE_N_PROPERTY_SHOW(is_enabled, password);
> > > > +static struct kobj_attribute password_is_password_set = __ATTR_RO(is_enabled);
> > > > +
> > > > +static ssize_t current_password_store(struct kobject *kobj,
> > > > + struct kobj_attribute *attr,
> > > > + const char *buf, size_t count)
> > > > +{
> > > > + char *p, *buf_cp;
> > > > + int id, ret = 0;
> > > > +
> > > > + buf_cp = kstrdup(buf, GFP_KERNEL);
> > > > + if (!buf_cp) {
> > > > + ret = -ENOMEM;
> > > > + goto exit_password;
> > > > + }
> > > > +
> > > > + p = memchr(buf_cp, '\n', count);
> > > > +
> > > > + if (p != NULL)
> > > > + *p = '\0';
> > >
> > > This will also accept input like "foo\nbar" and truncate away the "bar".
> > >
> > > For something like a password it seems errorprone to try to munge the
> > > value.
> >
> > This is an expected behavior. If the user enters '\n' as part of the
> > password, the buffer data will be truncated since only one line per
> > sysfs file is permitted.
>
> As discussed in another patch this would silently truncate the input on
> the first newline character; even if it is not the last character of the
> input string.
>
> This should use the same helper as the other files to only strip a
> newline at the end of the input.
>
Done! enforce_single_line_input() function is used.
> > >
> > > > +
> > > > + id = get_password_instance_id(kobj);
> > > > +
> > > > + if (id >= 0)
> > > > + ret = validate_password_input(id, buf_cp);
> > > > +
> > > > + if (!ret) {
> > > > + strscpy(bioscfg_drv.password_data[id].current_password,
> > > > + buf_cp,
> > > > + sizeof(bioscfg_drv.password_data[id].current_password));
> > > > + /*
> > > > + * set pending reboot flag depending on
> > > > + * "RequiresPhysicalPresence" value
> > > > + */
> > > > + if (bioscfg_drv.password_data[id].common.requires_physical_presence)
> > > > + bioscfg_drv.pending_reboot = true;
> > >
> > > Just setting this to true does not emit the necessary KOBJ_CHANGE event
> > > on the class dev kobj which is necessary for userspace to be able to
> > > react.
> >
> > This feature was added outside of the original design specification to
> > be used at a later time.
> > Changes to the value to true does not emit a KOBJ_CHANGE event.
>
> This contradicts the documentation:
>
> A read-only attribute reads 1 if a reboot is necessary to apply
> pending BIOS attribute changes. Also, an uevent_KOBJ_CHANGE is
> generated when it changes to 1.
>
> This will confuse userspace, there are generic userspace applications
> waiting for those events.
> If there is a reason for not emitting them it should be good and
> documented.
>
> Also according to the docs the authentication attributes should
> KOBJ_CHANGE events. I think this also affects this driver and should be
> implemented.
>
HP-BIOSCFG initially is not intended for the use of fwupd tool nor was
it tested.
This does not mean the driver will interface with fwupd and other
tools in the future.
> > >
> > > > + }
> > > > +
> > > > +exit_password:
> > > > + kfree(buf_cp);
> > > > + return ret ? ret : count;
> > > > +}
> > > > +static struct kobj_attribute password_current_password = __ATTR_WO(current_password);
> > > > +
> > > > +static ssize_t new_password_store(struct kobject *kobj,
> > > > + struct kobj_attribute *attr,
> > > > + const char *buf, size_t count)
> > > > +{
> > > > + char *p, *buf_cp = NULL;
> > > > + int id = 0;
> > > > + int ret = -EIO;
> > > > +
> > > > + buf_cp = kstrdup(buf, GFP_KERNEL);
> > > > + if (!buf_cp) {
> > > > + ret = -ENOMEM;
> > > > + goto exit_password;
> > > > + }
> > > > +
> > > > + p = memchr(buf_cp, '\n', count);
> > > > +
> > > > + if (p != NULL)
> > > > + *p = '\0';
> > >
> > > Same as above.
> >
> > This is an expected behavior. If the user enters '\n' as part of the
> > password, the buffer data will be truncated since only one line per
> > sysfs file is permitted.
>
> If a user accidentally presses enter before entering a password this
> may set the password to the empty string; surprising.
>
> This should really use the helper.
Done! enforce_single_line_input() function is used.
>
> > >
<snip>
> > > > +
> > > > +ATTRIBUTE_V_COMMON_PROPERTY_SHOW(prerequisites, password);
> > > > +static struct kobj_attribute password_prerequisites_val =
> > > > + __ATTR_RO(prerequisites);
> > > > +
> > > > +ATTRIBUTE_N_PROPERTY_SHOW(encodings_size, password);
> > > > +static struct kobj_attribute password_encodings_size_val =
> > > > + __ATTR_RO(encodings_size);
> > >
> > > As before, these size attribute are fairly pointless for userspace as
> > > they can't be relied on.
> >
> > I will remove the attribute from being reported in sysfs but they will
> > be kept as part of the driver internal data
> >
> > >
> > > > +
> > > > +ATTRIBUTE_VALUES_PROPERTY_SHOW(encodings, password);
> > > > +static struct kobj_attribute password_encodings_val =
> > > > + __ATTR_RO(encodings);
> > > > +
> > > > +
> > > > +static struct attribute *password_attrs[] = {
> > > > + &password_is_password_set.attr,
> > > > + &password_min_password_length.attr,
> > > > + &password_max_password_length.attr,
> > > > + &password_current_password.attr,
> > > > + &password_new_password.attr,
> > > > + &password_role.attr,
> > > > + &password_mechanism.attr,
> > > > + &password_type.attr,
> > > > + &password_display_name.attr,
> > > > + &password_display_langcode.attr,
> > > > + &password_prerequisites_size_val.attr,
> > > > + &password_prerequisites_val.attr,
> > > > + &password_encodings_val.attr,
> > > > + &password_encodings_size_val.attr,
> > > > + NULL
> > > > +};
> > >
> > > Many of these attributes are not documented.
> >
> > Those attributes are documented under authentication section, lines 150-329
> >
> > What: /sys/class/firmware-attributes/*/authentication/
> > Date: February 2021
> > KernelVersion: 5.11
> > Contact: Divya Bharathi <Divya.Bharathi@Dell.com>,
> > Prasanth KSR <prasanth.ksr@dell.com>
> > Dell.Client.Kernel@dell.com
> > Description:
> > Devices support various authentication mechanisms which can be exposed
> > as a separate configuration object.
>
> If I read that correctly the authentication attributes are not "normal"
> attributes.
> So they don't need "type", "display_name", "display_langcode".
>
Type, display_name, and display_langcode are required and default settings.
See documentation lines 15-52
type:
A file that can be read to obtain the type of attribute.
This attribute is mandatory.
display_name:
A file that can be read to obtain a user friendly
description of the at <attr>
display_name_language_code:
A file that can be read to obtain
the IETF language tag corresponding to the
"display_name" of the <attr>
> Do they need prerequisites?
Prerequisites is optional and not documented. I will remove it from
the list of items reported within sysfs
>
> >
> >
> > >
> > > > +
> > > > +static const struct attribute_group bios_password_attr_group = {
> > > > + .attrs = password_attrs
> > > > +};
> > > > +
> > > > +static const struct attribute_group system_password_attr_group = {
> > > > + .attrs = password_attrs
> > > > +};
> > >
> > > These groups are the same, are both needed?
> >
> > Yes. They will show under 'Setup Password' and 'Power-on password'
>
> These are identical constant structures. It should be possible to have
> only one and use it for both usecases.
>
Both 'Setup Password' and 'Power-on password' need to coexist hence
the reason for keeping them separate.
Both attributes share the same helper routines. Unifying both
passwords into a single structure adds unnecessary complexity.
> <snip>
On 2023-05-04 16:34:06-0500, Jorge Lopez wrote:
> On Thu, May 4, 2023 at 3:59 PM Thomas Weißschuh <thomas@t-8ch.de> wrote:
> >
> > On 2023-05-04 15:29:21-0500, Jorge Lopez wrote:
> > > On Sun, Apr 23, 2023 at 4:07 AM <thomas@t-8ch.de> wrote:
> > > >
> > > > On 2023-04-20 11:54:46-0500, Jorge Lopez wrote:
> > > > > ---
> > > > > .../x86/hp/hp-bioscfg/passwdobj-attributes.c | 669 ++++++++++++++++++
> > > > > 1 file changed, 669 insertions(+)
> > > > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
<snip>
> > > > > +
> > > > > + id = get_password_instance_id(kobj);
> > > > > +
> > > > > + if (id >= 0)
> > > > > + ret = validate_password_input(id, buf_cp);
> > > > > +
> > > > > + if (!ret) {
> > > > > + strscpy(bioscfg_drv.password_data[id].current_password,
> > > > > + buf_cp,
> > > > > + sizeof(bioscfg_drv.password_data[id].current_password));
> > > > > + /*
> > > > > + * set pending reboot flag depending on
> > > > > + * "RequiresPhysicalPresence" value
> > > > > + */
> > > > > + if (bioscfg_drv.password_data[id].common.requires_physical_presence)
> > > > > + bioscfg_drv.pending_reboot = true;
> > > >
> > > > Just setting this to true does not emit the necessary KOBJ_CHANGE event
> > > > on the class dev kobj which is necessary for userspace to be able to
> > > > react.
> > >
> > > This feature was added outside of the original design specification to
> > > be used at a later time.
> > > Changes to the value to true does not emit a KOBJ_CHANGE event.
> >
> > This contradicts the documentation:
> >
> > A read-only attribute reads 1 if a reboot is necessary to apply
> > pending BIOS attribute changes. Also, an uevent_KOBJ_CHANGE is
> > generated when it changes to 1.
> >
> > This will confuse userspace, there are generic userspace applications
> > waiting for those events.
> > If there is a reason for not emitting them it should be good and
> > documented.
> >
> > Also according to the docs the authentication attributes should
> > KOBJ_CHANGE events. I think this also affects this driver and should be
> > implemented.
> >
>
> HP-BIOSCFG initially is not intended for the use of fwupd tool nor was
> it tested.
> This does not mean the driver will interface with fwupd and other
> tools in the future.
There are probably more tools than fwupd using this ABI.
The driver is implementing a well-known ABI and users of this ABI expect
it to work as documented.
Emitting these events seems straigtforward and simple.
Maybe Hans can give more guidance on it.
> > > >
> > > > > + }
> > > > > +
> > > > > +exit_password:
> > > > > + kfree(buf_cp);
> > > > > + return ret ? ret : count;
> > > > > +}
> > > > > +static struct kobj_attribute password_current_password = __ATTR_WO(current_password);
> > > > > +
> > > > > +static ssize_t new_password_store(struct kobject *kobj,
> > > > > + struct kobj_attribute *attr,
> > > > > + const char *buf, size_t count)
> > > > > +{
> > > > > + char *p, *buf_cp = NULL;
> > > > > + int id = 0;
> > > > > + int ret = -EIO;
> > > > > +
> > > > > + buf_cp = kstrdup(buf, GFP_KERNEL);
> > > > > + if (!buf_cp) {
> > > > > + ret = -ENOMEM;
> > > > > + goto exit_password;
> > > > > + }
> > > > > +
> > > > > + p = memchr(buf_cp, '\n', count);
> > > > > +
> > > > > + if (p != NULL)
> > > > > + *p = '\0';
> > > >
> > > > Same as above.
> > >
> > > This is an expected behavior. If the user enters '\n' as part of the
> > > password, the buffer data will be truncated since only one line per
> > > sysfs file is permitted.
> >
> > If a user accidentally presses enter before entering a password this
> > may set the password to the empty string; surprising.
> >
> > This should really use the helper.
>
> Done! enforce_single_line_input() function is used.
> >
> > > >
> <snip>
> > > > > +
> > > > > +ATTRIBUTE_V_COMMON_PROPERTY_SHOW(prerequisites, password);
> > > > > +static struct kobj_attribute password_prerequisites_val =
> > > > > + __ATTR_RO(prerequisites);
> > > > > +
> > > > > +ATTRIBUTE_N_PROPERTY_SHOW(encodings_size, password);
> > > > > +static struct kobj_attribute password_encodings_size_val =
> > > > > + __ATTR_RO(encodings_size);
> > > >
> > > > As before, these size attribute are fairly pointless for userspace as
> > > > they can't be relied on.
> > >
> > > I will remove the attribute from being reported in sysfs but they will
> > > be kept as part of the driver internal data
> > >
> > > >
> > > > > +
> > > > > +ATTRIBUTE_VALUES_PROPERTY_SHOW(encodings, password);
> > > > > +static struct kobj_attribute password_encodings_val =
> > > > > + __ATTR_RO(encodings);
> > > > > +
> > > > > +
> > > > > +static struct attribute *password_attrs[] = {
> > > > > + &password_is_password_set.attr,
> > > > > + &password_min_password_length.attr,
> > > > > + &password_max_password_length.attr,
> > > > > + &password_current_password.attr,
> > > > > + &password_new_password.attr,
> > > > > + &password_role.attr,
> > > > > + &password_mechanism.attr,
> > > > > + &password_type.attr,
> > > > > + &password_display_name.attr,
> > > > > + &password_display_langcode.attr,
> > > > > + &password_prerequisites_size_val.attr,
> > > > > + &password_prerequisites_val.attr,
> > > > > + &password_encodings_val.attr,
> > > > > + &password_encodings_size_val.attr,
> > > > > + NULL
> > > > > +};
> > > >
> > > > Many of these attributes are not documented.
> > >
> > > Those attributes are documented under authentication section, lines 150-329
> > >
> > > What: /sys/class/firmware-attributes/*/authentication/
> > > Date: February 2021
> > > KernelVersion: 5.11
> > > Contact: Divya Bharathi <Divya.Bharathi@Dell.com>,
> > > Prasanth KSR <prasanth.ksr@dell.com>
> > > Dell.Client.Kernel@dell.com
> > > Description:
> > > Devices support various authentication mechanisms which can be exposed
> > > as a separate configuration object.
> >
> > If I read that correctly the authentication attributes are not "normal"
> > attributes.
> > So they don't need "type", "display_name", "display_langcode".
> >
>
> Type, display_name, and display_langcode are required and default settings.
> See documentation lines 15-52
>
> type:
> A file that can be read to obtain the type of attribute.
> This attribute is mandatory.
>
> display_name:
> A file that can be read to obtain a user friendly
> description of the at <attr>
>
> display_name_language_code:
> A file that can be read to obtain
> the IETF language tag corresponding to the
> "display_name" of the <attr>
They are required for
/sys/class/firmware-attributes/*/attributes/*/
but here we implement
/sys/class/firmware-attributes/*/authentication/*/
which is a different ABI.
>
> > Do they need prerequisites?
>
> Prerequisites is optional and not documented. I will remove it from
> the list of items reported within sysfs
> >
> > >
> > >
> > > >
> > > > > +
> > > > > +static const struct attribute_group bios_password_attr_group = {
> > > > > + .attrs = password_attrs
> > > > > +};
> > > > > +
> > > > > +static const struct attribute_group system_password_attr_group = {
> > > > > + .attrs = password_attrs
> > > > > +};
> > > >
> > > > These groups are the same, are both needed?
> > >
> > > Yes. They will show under 'Setup Password' and 'Power-on password'
> >
> > These are identical constant structures. It should be possible to have
> > only one and use it for both usecases.
> >
>
> Both 'Setup Password' and 'Power-on password' need to coexist hence
> the reason for keeping them separate.
> Both attributes share the same helper routines. Unifying both
> passwords into a single structure adds unnecessary complexity.
They are already sharing the "password_attrs" array and all the
attributes listed in it.
It seems they could also share the attribute_group which does not really
contain any data.
On Thu, May 4, 2023 at 5:21 PM Thomas Weißschuh <thomas@t-8ch.de> wrote:
>
> On 2023-05-04 16:34:06-0500, Jorge Lopez wrote:
> > On Thu, May 4, 2023 at 3:59 PM Thomas Weißschuh <thomas@t-8ch.de> wrote:
> > >
> > > On 2023-05-04 15:29:21-0500, Jorge Lopez wrote:
> > > > On Sun, Apr 23, 2023 at 4:07 AM <thomas@t-8ch.de> wrote:
> > > > >
> > > > > On 2023-04-20 11:54:46-0500, Jorge Lopez wrote:
> > > > > > ---
> > > > > > .../x86/hp/hp-bioscfg/passwdobj-attributes.c | 669 ++++++++++++++++++
> > > > > > 1 file changed, 669 insertions(+)
> > > > > > create mode 100644 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
>
> <snip>
>
> > > > > > +
> > > > > > + id = get_password_instance_id(kobj);
> > > > > > +
> > > > > > + if (id >= 0)
> > > > > > + ret = validate_password_input(id, buf_cp);
> > > > > > +
> > > > > > + if (!ret) {
> > > > > > + strscpy(bioscfg_drv.password_data[id].current_password,
> > > > > > + buf_cp,
> > > > > > + sizeof(bioscfg_drv.password_data[id].current_password));
> > > > > > + /*
> > > > > > + * set pending reboot flag depending on
> > > > > > + * "RequiresPhysicalPresence" value
> > > > > > + */
> > > > > > + if (bioscfg_drv.password_data[id].common.requires_physical_presence)
> > > > > > + bioscfg_drv.pending_reboot = true;
> > > > >
> > > > > Just setting this to true does not emit the necessary KOBJ_CHANGE event
> > > > > on the class dev kobj which is necessary for userspace to be able to
> > > > > react.
> > > >
> > > > This feature was added outside of the original design specification to
> > > > be used at a later time.
> > > > Changes to the value to true does not emit a KOBJ_CHANGE event.
> > >
> > > This contradicts the documentation:
> > >
> > > A read-only attribute reads 1 if a reboot is necessary to apply
> > > pending BIOS attribute changes. Also, an uevent_KOBJ_CHANGE is
> > > generated when it changes to 1.
> > >
> > > This will confuse userspace, there are generic userspace applications
> > > waiting for those events.
> > > If there is a reason for not emitting them it should be good and
> > > documented.
> > >
> > > Also according to the docs the authentication attributes should
> > > KOBJ_CHANGE events. I think this also affects this driver and should be
> > > implemented.
> > >
> >
> > HP-BIOSCFG initially is not intended for the use of fwupd tool nor was
> > it tested.
> > This does not mean the driver will interface with fwupd and other
> > tools in the future.
>
> There are probably more tools than fwupd using this ABI.
>
> The driver is implementing a well-known ABI and users of this ABI expect
> it to work as documented.
>
> Emitting these events seems straigtforward and simple.
>
> Maybe Hans can give more guidance on it.
>
Let me see how I can implement it by following the sample of other drivers.
I will reachout to Hans as my last resource.
Done!
> > > > >
> > > > > > + }
> > > > > > +
> > > > > > +exit_password:
> > > > > > + kfree(buf_cp);
> > > > > > + return ret ? ret : count;
> > > > > > +}
> > > > > > +static struct kobj_attribute password_current_password = __ATTR_WO(current_password);
> > > > > > +
> > > > > > +static ssize_t new_password_store(struct kobject *kobj,
> > > > > > + struct kobj_attribute *attr,
> > > > > > + const char *buf, size_t count)
> > > > > > +{
> > > > > > + char *p, *buf_cp = NULL;
> > > > > > + int id = 0;
> > > > > > + int ret = -EIO;
> > > > > > +
> > > > > > + buf_cp = kstrdup(buf, GFP_KERNEL);
> > > > > > + if (!buf_cp) {
> > > > > > + ret = -ENOMEM;
> > > > > > + goto exit_password;
> > > > > > + }
> > > > > > +
> > > > > > + p = memchr(buf_cp, '\n', count);
> > > > > > +
> > > > > > + if (p != NULL)
> > > > > > + *p = '\0';
> > > > >
> > > > > Same as above.
> > > >
> > > > This is an expected behavior. If the user enters '\n' as part of the
> > > > password, the buffer data will be truncated since only one line per
> > > > sysfs file is permitted.
> > >
> > > If a user accidentally presses enter before entering a password this
> > > may set the password to the empty string; surprising.
> > >
> > > This should really use the helper.
> >
> > Done! enforce_single_line_input() function is used.
> > >
> > > > >
> > <snip>
> > > > > > +
> > > > > > +ATTRIBUTE_V_COMMON_PROPERTY_SHOW(prerequisites, password);
> > > > > > +static struct kobj_attribute password_prerequisites_val =
> > > > > > + __ATTR_RO(prerequisites);
> > > > > > +
> > > > > > +ATTRIBUTE_N_PROPERTY_SHOW(encodings_size, password);
> > > > > > +static struct kobj_attribute password_encodings_size_val =
> > > > > > + __ATTR_RO(encodings_size);
> > > > >
> > > > > As before, these size attribute are fairly pointless for userspace as
> > > > > they can't be relied on.
> > > >
> > > > I will remove the attribute from being reported in sysfs but they will
> > > > be kept as part of the driver internal data
> > > >
> > > > >
> > > > > > +
> > > > > > +ATTRIBUTE_VALUES_PROPERTY_SHOW(encodings, password);
> > > > > > +static struct kobj_attribute password_encodings_val =
> > > > > > + __ATTR_RO(encodings);
> > > > > > +
> > > > > > +
> > > > > > +static struct attribute *password_attrs[] = {
> > > > > > + &password_is_password_set.attr,
> > > > > > + &password_min_password_length.attr,
> > > > > > + &password_max_password_length.attr,
> > > > > > + &password_current_password.attr,
> > > > > > + &password_new_password.attr,
> > > > > > + &password_role.attr,
> > > > > > + &password_mechanism.attr,
> > > > > > + &password_type.attr,
> > > > > > + &password_display_name.attr,
> > > > > > + &password_display_langcode.attr,
> > > > > > + &password_prerequisites_size_val.attr,
> > > > > > + &password_prerequisites_val.attr,
> > > > > > + &password_encodings_val.attr,
> > > > > > + &password_encodings_size_val.attr,
> > > > > > + NULL
> > > > > > +};
> > > > >
> > > > > Many of these attributes are not documented.
> > > >
> > > > Those attributes are documented under authentication section, lines 150-329
> > > >
> > > > What: /sys/class/firmware-attributes/*/authentication/
> > > > Date: February 2021
> > > > KernelVersion: 5.11
> > > > Contact: Divya Bharathi <Divya.Bharathi@Dell.com>,
> > > > Prasanth KSR <prasanth.ksr@dell.com>
> > > > Dell.Client.Kernel@dell.com
> > > > Description:
> > > > Devices support various authentication mechanisms which can be exposed
> > > > as a separate configuration object.
> > >
> > > If I read that correctly the authentication attributes are not "normal"
> > > attributes.
> > > So they don't need "type", "display_name", "display_langcode".
> > >
> >
> > Type, display_name, and display_langcode are required and default settings.
> > See documentation lines 15-52
> >
> > type:
> > A file that can be read to obtain the type of attribute.
> > This attribute is mandatory.
> >
> > display_name:
> > A file that can be read to obtain a user friendly
> > description of the at <attr>
> >
> > display_name_language_code:
> > A file that can be read to obtain
> > the IETF language tag corresponding to the
> > "display_name" of the <attr>
>
> They are required for
>
> /sys/class/firmware-attributes/*/attributes/*/
>
> but here we implement
>
> /sys/class/firmware-attributes/*/authentication/*/
>
> which is a different ABI.
>
> >
> > > Do they need prerequisites?
> >
> > Prerequisites is optional and not documented. I will remove it from
> > the list of items reported within sysfs
> > >
> > > >
Fair enough. I will remove "type", "display_name" and "display_langcode".
The same three entries will be removed from spmobj-attributes (SPM)
which belong to authentication attributes.
> > > >
> > > > >
> > > > > > +
> > > > > > +static const struct attribute_group bios_password_attr_group = {
> > > > > > + .attrs = password_attrs
> > > > > > +};
> > > > > > +
> > > > > > +static const struct attribute_group system_password_attr_group = {
> > > > > > + .attrs = password_attrs
> > > > > > +};
> > > > >
> > > > > These groups are the same, are both needed?
> > > >
> > > > Yes. They will show under 'Setup Password' and 'Power-on password'
> > >
> > > These are identical constant structures. It should be possible to have
> > > only one and use it for both usecases.
> > >
> >
> > Both 'Setup Password' and 'Power-on password' need to coexist hence
> > the reason for keeping them separate.
> > Both attributes share the same helper routines. Unifying both
> > passwords into a single structure adds unnecessary complexity.
>
> They are already sharing the "password_attrs" array and all the
> attributes listed in it.
>
> It seems they could also share the attribute_group which does not really
> contain any data.
Ah. That makes sense. Thank you for the clarification.
Done!
new file mode 100644
@@ -0,0 +1,669 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Functions corresponding to password object type attributes under
+ * BIOS PASSWORD for use with hp-bioscfg driver.
+ *
+ * Copyright (c) 2022 HP Development Company, L.P.
+ */
+
+#include "bioscfg.h"
+#include <asm-generic/posix_types.h>
+
+GET_INSTANCE_ID(password);
+/*
+ * Clear all passwords copied to memory for a particular
+ * authentication instance
+ */
+int clear_passwords(const int instance)
+{
+ if (!bioscfg_drv.password_data[instance].is_enabled)
+ return 0;
+
+ memset(bioscfg_drv.password_data[instance].current_password,
+ 0, sizeof(bioscfg_drv.password_data[instance].current_password));
+ memset(bioscfg_drv.password_data[instance].new_password,
+ 0, sizeof(bioscfg_drv.password_data[instance].new_password));
+
+ return 0;
+}
+
+/*
+ * Clear all credentials copied to memory for both Power-ON and Setup
+ * BIOS instances
+ */
+int clear_all_credentials(void)
+{
+ int instance;
+
+ /* clear all passwords */
+ for (instance = 0; instance < bioscfg_drv.password_instances_count; instance++)
+ clear_passwords(instance);
+
+ /* clear auth_token */
+ kfree(bioscfg_drv.spm_data.auth_token);
+ bioscfg_drv.spm_data.auth_token = NULL;
+
+ return 0;
+}
+
+int get_password_instance_for_type(const char *name)
+{
+ int count = bioscfg_drv.password_instances_count;
+ int instance;
+
+ for (instance = 0; instance < count; instance++) {
+ if (strcmp(bioscfg_drv.password_data[instance].common.display_name, name) == 0)
+ return instance;
+ }
+ return -EINVAL;
+}
+
+int validate_password_input(int instance_id, const char *buf)
+{
+ int length;
+
+ length = strlen(buf);
+ if (buf[length-1] == '\n')
+ length--;
+
+ if (length > MAX_PASSWD_SIZE)
+ return INVALID_BIOS_AUTH;
+
+ if (bioscfg_drv.password_data[instance_id].min_password_length > length ||
+ bioscfg_drv.password_data[instance_id].max_password_length < length)
+ return INVALID_BIOS_AUTH;
+ return SUCCESS;
+}
+
+int password_is_set(const char *name)
+{
+ int id;
+
+ id = get_password_instance_for_type(name);
+ if (id < 0)
+ return 0;
+
+ return bioscfg_drv.password_data[id].is_enabled;
+}
+
+ATTRIBUTE_N_PROPERTY_SHOW(is_enabled, password);
+static struct kobj_attribute password_is_password_set = __ATTR_RO(is_enabled);
+
+static ssize_t current_password_store(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ char *p, *buf_cp;
+ int id, ret = 0;
+
+ buf_cp = kstrdup(buf, GFP_KERNEL);
+ if (!buf_cp) {
+ ret = -ENOMEM;
+ goto exit_password;
+ }
+
+ p = memchr(buf_cp, '\n', count);
+
+ if (p != NULL)
+ *p = '\0';
+
+ id = get_password_instance_id(kobj);
+
+ if (id >= 0)
+ ret = validate_password_input(id, buf_cp);
+
+ if (!ret) {
+ strscpy(bioscfg_drv.password_data[id].current_password,
+ buf_cp,
+ sizeof(bioscfg_drv.password_data[id].current_password));
+ /*
+ * set pending reboot flag depending on
+ * "RequiresPhysicalPresence" value
+ */
+ if (bioscfg_drv.password_data[id].common.requires_physical_presence)
+ bioscfg_drv.pending_reboot = true;
+ }
+
+exit_password:
+ kfree(buf_cp);
+ return ret ? ret : count;
+}
+static struct kobj_attribute password_current_password = __ATTR_WO(current_password);
+
+static ssize_t new_password_store(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ char *p, *buf_cp = NULL;
+ int id = 0;
+ int ret = -EIO;
+
+ buf_cp = kstrdup(buf, GFP_KERNEL);
+ if (!buf_cp) {
+ ret = -ENOMEM;
+ goto exit_password;
+ }
+
+ p = memchr(buf_cp, '\n', count);
+
+ if (p != NULL)
+ *p = '\0';
+
+ id = get_password_instance_id(kobj);
+
+ if (id >= 0)
+ ret = validate_password_input(id, buf_cp);
+
+ if (!ret)
+ strscpy(bioscfg_drv.password_data[id].new_password,
+ buf_cp,
+ sizeof(bioscfg_drv.password_data[id].new_password));
+
+ if (!ret)
+ ret = hp_set_attribute(kobj->name, buf_cp);
+
+exit_password:
+ /*
+ * Regardless of the results both new and current passwords
+ * will be set to zero and avoid security issues
+ */
+ clear_passwords(id);
+
+ kfree(buf_cp);
+ return ret ? ret : count;
+}
+
+static struct kobj_attribute password_new_password = __ATTR_WO(new_password);
+
+
+ATTRIBUTE_N_PROPERTY_SHOW(min_password_length, password);
+static struct kobj_attribute password_min_password_length = __ATTR_RO(min_password_length);
+
+ATTRIBUTE_N_PROPERTY_SHOW(max_password_length, password);
+static struct kobj_attribute password_max_password_length = __ATTR_RO(max_password_length);
+
+static ssize_t role_show(struct kobject *kobj, struct kobj_attribute *attr,
+ char *buf)
+{
+ if (strcmp(kobj->name, SETUP_PASSWD) == 0)
+ return sysfs_emit(buf, "%s\n", BIOS_ADMIN);
+
+ if (strcmp(kobj->name, POWER_ON_PASSWD) == 0)
+ return sysfs_emit(buf, "%s\n", POWER_ON);
+
+ return -EIO;
+}
+static struct kobj_attribute password_role = __ATTR_RO(role);
+
+static ssize_t mechanism_show(struct kobject *kobj, struct kobj_attribute *attr,
+ char *buf)
+{
+ int i = get_password_instance_id(kobj);
+
+ if (i < 0)
+ return i;
+
+ if (bioscfg_drv.password_data[i].mechanism != PASSWORD)
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%s\n", PASSWD_MECHANISM_TYPES);
+}
+static struct kobj_attribute password_mechanism = __ATTR_RO(mechanism);
+
+static ssize_t type_show(struct kobject *kobj, struct kobj_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "password\n");
+}
+static struct kobj_attribute password_type = __ATTR_RO(type);
+
+ATTRIBUTE_S_COMMON_PROPERTY_SHOW(display_name, password);
+static struct kobj_attribute password_display_name =
+ __ATTR_RO(display_name);
+
+ATTRIBUTE_S_COMMON_PROPERTY_SHOW(display_name_language_code, password);
+static struct kobj_attribute password_display_langcode =
+ __ATTR_RO(display_name_language_code);
+
+ATTRIBUTE_N_COMMON_PROPERTY_SHOW(prerequisites_size, password);
+static struct kobj_attribute password_prerequisites_size_val =
+ __ATTR_RO(prerequisites_size);
+
+ATTRIBUTE_V_COMMON_PROPERTY_SHOW(prerequisites, password);
+static struct kobj_attribute password_prerequisites_val =
+ __ATTR_RO(prerequisites);
+
+ATTRIBUTE_N_PROPERTY_SHOW(encodings_size, password);
+static struct kobj_attribute password_encodings_size_val =
+ __ATTR_RO(encodings_size);
+
+ATTRIBUTE_VALUES_PROPERTY_SHOW(encodings, password);
+static struct kobj_attribute password_encodings_val =
+ __ATTR_RO(encodings);
+
+
+static struct attribute *password_attrs[] = {
+ &password_is_password_set.attr,
+ &password_min_password_length.attr,
+ &password_max_password_length.attr,
+ &password_current_password.attr,
+ &password_new_password.attr,
+ &password_role.attr,
+ &password_mechanism.attr,
+ &password_type.attr,
+ &password_display_name.attr,
+ &password_display_langcode.attr,
+ &password_prerequisites_size_val.attr,
+ &password_prerequisites_val.attr,
+ &password_encodings_val.attr,
+ &password_encodings_size_val.attr,
+ NULL
+};
+
+static const struct attribute_group bios_password_attr_group = {
+ .attrs = password_attrs
+};
+
+static const struct attribute_group system_password_attr_group = {
+ .attrs = password_attrs
+};
+
+int alloc_password_data(void)
+{
+ int ret = 0;
+
+ bioscfg_drv.password_instances_count = get_instance_count(HP_WMI_BIOS_PASSWORD_GUID);
+ bioscfg_drv.password_data = kcalloc(bioscfg_drv.password_instances_count,
+ sizeof(struct password_data), GFP_KERNEL);
+ if (!bioscfg_drv.password_data) {
+ bioscfg_drv.password_instances_count = 0;
+ ret = -ENOMEM;
+ }
+
+ return ret;
+}
+
+/*
+ * populate_password_package_data -
+ * Populate all properties for an instance under password attribute
+ *
+ * @password_obj: ACPI object with password data
+ * @instance_id: The instance to enumerate
+ * @attr_name_kobj: The parent kernel object
+ */
+int populate_password_package_data(union acpi_object *password_obj, int instance_id,
+ struct kobject *attr_name_kobj)
+{
+ bioscfg_drv.password_data[instance_id].attr_name_kobj = attr_name_kobj;
+
+ populate_password_elements_from_package(password_obj,
+ password_obj->package.count,
+ instance_id);
+
+ if (strcmp(attr_name_kobj->name, "Setup Password") == 0) {
+ /* Save system authentication instance for easy access */
+ return sysfs_create_group(attr_name_kobj, &bios_password_attr_group);
+ }
+
+ return sysfs_create_group(attr_name_kobj, &system_password_attr_group);
+}
+
+/* Expected Values types associated with each element */
+static const acpi_object_type expected_password_types[] = {
+ [NAME] = ACPI_TYPE_STRING,
+ [VALUE] = ACPI_TYPE_STRING,
+ [PATH] = ACPI_TYPE_STRING,
+ [IS_READONLY] = ACPI_TYPE_INTEGER,
+ [DISPLAY_IN_UI] = ACPI_TYPE_INTEGER,
+ [REQUIRES_PHYSICAL_PRESENCE] = ACPI_TYPE_INTEGER,
+ [SEQUENCE] = ACPI_TYPE_INTEGER,
+ [PREREQUISITES_SIZE] = ACPI_TYPE_INTEGER,
+ [PREREQUISITES] = ACPI_TYPE_STRING,
+ [SECURITY_LEVEL] = ACPI_TYPE_INTEGER,
+ [PSWD_MIN_LENGTH] = ACPI_TYPE_INTEGER,
+ [PSWD_MAX_LENGTH] = ACPI_TYPE_INTEGER,
+ [PSWD_SIZE] = ACPI_TYPE_INTEGER,
+ [PSWD_ENCODINGS] = ACPI_TYPE_STRING,
+ [PSWD_IS_SET] = ACPI_TYPE_INTEGER
+};
+
+
+int populate_password_elements_from_package(union acpi_object *password_obj,
+ int password_obj_count,
+ int instance_id)
+{
+ char *str_value = NULL;
+ int value_len;
+ int ret = 0;
+ u32 size = 0;
+ u32 int_value;
+ int elem = 0;
+ int reqs;
+ int eloc;
+ int pos_values;
+
+
+ if (!password_obj)
+ return -EINVAL;
+
+ strscpy(bioscfg_drv.password_data[instance_id].common.display_name_language_code,
+ LANG_CODE_STR,
+ sizeof(bioscfg_drv.password_data[instance_id].common.display_name_language_code));
+
+ for (elem = 1, eloc = 1; elem < password_obj_count; elem++, eloc++) {
+
+ /* ONLY look at the first PASSWORD_ELEM_CNT elements */
+ if (eloc == PASSWORD_ELEM_CNT)
+ goto exit_package;
+
+ switch (password_obj[elem].type) {
+ case ACPI_TYPE_STRING:
+
+ if (PREREQUISITES != elem && PSWD_ENCODINGS != elem) {
+ ret = convert_hexstr_to_str(password_obj[elem].string.pointer,
+ password_obj[elem].string.length,
+ &str_value, &value_len);
+ if (ret)
+ continue;
+ }
+ break;
+ case ACPI_TYPE_INTEGER:
+ int_value = (u32)password_obj[elem].integer.value;
+ break;
+ default:
+ pr_warn("Unsupported object type [%d]\n", password_obj[elem].type);
+ continue;
+ }
+
+ /* Check that both expected and read object type match */
+ if (expected_password_types[eloc] != password_obj[elem].type) {
+ pr_err("Error expected type %d for elem %d, but got type %d instead\n",
+ expected_password_types[eloc], elem, password_obj[elem].type);
+ return -EIO;
+ }
+
+ /* Assign appropriate element value to corresponding field*/
+ switch (eloc) {
+ case VALUE:
+ break;
+ case PATH:
+ strscpy(bioscfg_drv.password_data[instance_id].common.path, str_value,
+ sizeof(bioscfg_drv.password_data[instance_id].common.path));
+ break;
+ case IS_READONLY:
+ bioscfg_drv.password_data[instance_id].common.is_readonly = int_value;
+ break;
+ case DISPLAY_IN_UI:
+ bioscfg_drv.password_data[instance_id].common.display_in_ui = int_value;
+ break;
+ case REQUIRES_PHYSICAL_PRESENCE:
+ bioscfg_drv.password_data[instance_id].common.requires_physical_presence = int_value;
+ break;
+ case SEQUENCE:
+ bioscfg_drv.password_data[instance_id].common.sequence = int_value;
+ break;
+ case PREREQUISITES_SIZE:
+ bioscfg_drv.password_data[instance_id].common.prerequisites_size = int_value;
+ if (int_value > MAX_PREREQUISITES_SIZE)
+ pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
+ /*
+ * This HACK is needed to keep the expected
+ * element list pointing to the right obj[elem].type
+ * when the size is zero. PREREQUISITES
+ * object is omitted by BIOS when the size is
+ * zero.
+ */
+ if (int_value == 0)
+ eloc++;
+ break;
+ case PREREQUISITES:
+ size = bioscfg_drv.password_data[instance_id].common.prerequisites_size;
+
+ for (reqs = 0; reqs < size; reqs++) {
+ ret = convert_hexstr_to_str(password_obj[elem + reqs].string.pointer,
+ password_obj[elem + reqs].string.length,
+ &str_value, &value_len);
+
+ if (ret)
+ break;
+
+ strscpy(bioscfg_drv.password_data[instance_id].common.prerequisites[reqs],
+ str_value,
+ sizeof(bioscfg_drv.password_data[instance_id].common.prerequisites[reqs]));
+
+ kfree(str_value);
+ }
+ break;
+
+ case SECURITY_LEVEL:
+ bioscfg_drv.password_data[instance_id].common.security_level = int_value;
+ break;
+
+ case PSWD_MIN_LENGTH:
+ bioscfg_drv.password_data[instance_id].min_password_length = int_value;
+ break;
+ case PSWD_MAX_LENGTH:
+ bioscfg_drv.password_data[instance_id].max_password_length = int_value;
+ break;
+ case PSWD_SIZE:
+ bioscfg_drv.password_data[instance_id].encodings_size = int_value;
+ if (int_value > MAX_ENCODINGS_SIZE)
+ pr_warn("Password Encoding size value exceeded the maximum number of elements supported or data may be malformed\n");
+
+ /*
+ * This HACK is needed to keep the expected
+ * element list pointing to the right obj[elem].type
+ * when the size is zero. PSWD_ENCODINGS
+ * object is omitted by BIOS when the size is
+ * zero.
+ */
+ if (int_value == 0)
+ eloc++;
+ break;
+
+ case PSWD_ENCODINGS:
+ size = bioscfg_drv.password_data[instance_id].encodings_size;
+
+ for (pos_values = 0; pos_values < size && pos_values < MAX_ENCODINGS_SIZE; pos_values++) {
+ ret = convert_hexstr_to_str(password_obj[elem + pos_values].string.pointer,
+ password_obj[elem + pos_values].string.length,
+ &str_value, &value_len);
+ if (ret)
+ break;
+
+ strscpy(bioscfg_drv.password_data[instance_id].encodings[pos_values],
+ str_value,
+ sizeof(bioscfg_drv.password_data[instance_id].encodings[pos_values]));
+ kfree(str_value);
+ }
+ break;
+ case PSWD_IS_SET:
+ bioscfg_drv.password_data[instance_id].is_enabled = int_value;
+ break;
+
+ default:
+ pr_warn("Invalid element: %d found in Password attribute or data may be malformed\n", elem);
+ break;
+ }
+ kfree(str_value);
+ }
+
+exit_package:
+ kfree(str_value);
+ return 0;
+}
+
+/*
+ * populate_password_buffer_data -
+ * Populate all properties for an instance under password object attribute
+ *
+ * @buffer_ptr: Buffer pointer
+ * @buffer_size: Buffer size
+ * @instance_id: The instance to enumerate
+ * @attr_name_kobj: The parent kernel object
+ */
+int populate_password_buffer_data(u8 *buffer_ptr, u32 *buffer_size, int instance_id,
+ struct kobject *attr_name_kobj)
+{
+ bioscfg_drv.password_data[instance_id].attr_name_kobj = attr_name_kobj;
+
+ /* Populate Password attributes */
+ populate_password_elements_from_buffer(buffer_ptr, buffer_size,
+ instance_id);
+ friendly_user_name_update(bioscfg_drv.password_data[instance_id].common.path,
+ attr_name_kobj->name,
+ bioscfg_drv.password_data[instance_id].common.display_name,
+ sizeof(bioscfg_drv.password_data[instance_id].common.display_name));
+ if (strcmp(attr_name_kobj->name, "Setup Password") == 0)
+ return sysfs_create_group(attr_name_kobj, &bios_password_attr_group);
+
+ return sysfs_create_group(attr_name_kobj, &system_password_attr_group);
+}
+
+int populate_password_elements_from_buffer(u8 *buffer_ptr, u32 *buffer_size,
+ int instance_id)
+{
+ int ret;
+ char *dst = NULL;
+ int elem;
+ int reqs;
+ int integer;
+ int size = 0;
+ int values;
+ int dst_size = *buffer_size / sizeof(u16);
+
+ dst = kcalloc(dst_size, sizeof(char), GFP_KERNEL);
+ if (!dst)
+ return -ENOMEM;
+
+ elem = 0;
+ strscpy(bioscfg_drv.password_data[instance_id].common.display_name_language_code,
+ LANG_CODE_STR,
+ sizeof(bioscfg_drv.password_data[instance_id].common.display_name_language_code));
+
+ for (elem = 1; elem < 3; elem++) {
+
+ ret = get_string_from_buffer(&buffer_ptr, buffer_size, dst, dst_size);
+ if (ret < 0)
+ continue;
+
+ switch (elem) {
+ case VALUE:
+ strscpy(bioscfg_drv.password_data[instance_id].current_password,
+ dst, sizeof(bioscfg_drv.password_data[instance_id].current_password));
+ break;
+ case PATH:
+ strscpy(bioscfg_drv.password_data[instance_id].common.path, dst,
+ sizeof(bioscfg_drv.password_data[instance_id].common.path));
+ break;
+ default:
+ pr_warn("Invalid element: %d found in Password attribute or data may be malformed\n", elem);
+ break;
+ }
+ }
+
+ for (elem = 3; elem < PASSWORD_ELEM_CNT; elem++) {
+
+ if (elem != PREREQUISITES && elem != PSWD_ENCODINGS) {
+ ret = get_integer_from_buffer((int **)&buffer_ptr, buffer_size, (int *)&integer);
+ if (ret)
+ continue;
+ }
+
+ switch (elem) {
+ case IS_READONLY:
+ bioscfg_drv.password_data[instance_id].common.is_readonly = integer;
+ break;
+ case DISPLAY_IN_UI:
+ bioscfg_drv.password_data[instance_id].common.display_in_ui = integer;
+ break;
+ case REQUIRES_PHYSICAL_PRESENCE:
+ bioscfg_drv.password_data[instance_id].common.requires_physical_presence = integer;
+ break;
+ case SEQUENCE:
+ bioscfg_drv.password_data[instance_id].common.sequence = integer;
+ break;
+ case PREREQUISITES_SIZE:
+ bioscfg_drv.password_data[instance_id].common.prerequisites_size = integer;
+ if (integer > MAX_PREREQUISITES_SIZE)
+ pr_warn("Prerequisites size value exceeded the maximum number of elements supported or data may be malformed\n");
+
+ // PREREQUISITES:
+ elem++;
+ size = bioscfg_drv.password_data[instance_id].common.prerequisites_size;
+ for (reqs = 0; reqs < size && reqs > MAX_PREREQUISITES_SIZE; reqs++) {
+ ret = get_string_from_buffer(&buffer_ptr, buffer_size, dst, dst_size);
+ if (ret < 0)
+ continue;
+
+ strscpy(bioscfg_drv.password_data[instance_id].common.prerequisites[reqs],
+ dst,
+ sizeof(bioscfg_drv.password_data[instance_id].common.prerequisites[reqs]));
+ }
+ break;
+ case SECURITY_LEVEL:
+ bioscfg_drv.password_data[instance_id].common.security_level = integer;
+ break;
+
+ case PSWD_MIN_LENGTH:
+ bioscfg_drv.password_data[instance_id].min_password_length = integer;
+ break;
+ case PSWD_MAX_LENGTH:
+ bioscfg_drv.password_data[instance_id].max_password_length = integer;
+ break;
+ case PSWD_SIZE:
+ bioscfg_drv.password_data[instance_id].encodings_size = integer;
+ if (integer > MAX_ENCODINGS_SIZE)
+ pr_warn("Password Encoding size value exceeded the maximum number of elements supported or data may be malformed\n");
+
+ // PSWD_ENCODINGS:
+ elem++;
+ size = bioscfg_drv.password_data[instance_id].encodings_size;
+ for (values = 0; values < size && values < MAX_ENCODINGS_SIZE; values++) {
+ ret = get_string_from_buffer(&buffer_ptr, buffer_size, dst, dst_size);
+ if (ret < 0)
+ continue;
+
+ strscpy(bioscfg_drv.password_data[instance_id].encodings[values],
+ dst,
+ sizeof(bioscfg_drv.password_data[instance_id].encodings[values]));
+
+ }
+ break;
+ case PSWD_IS_SET:
+ bioscfg_drv.password_data[instance_id].is_enabled = integer;
+ break;
+ default:
+ pr_warn("Invalid element: %d found in Password attribute or data may be malformed\n", elem);
+ break;
+ }
+ }
+ kfree(dst);
+ return 0;
+}
+
+/*
+ * exit_password_attributes() - Clear all attribute data
+ *
+ * Clears all data allocated for this group of attributes
+ */
+void exit_password_attributes(void)
+{
+ int instance_id;
+
+ for (instance_id = 0; instance_id < bioscfg_drv.password_instances_count; instance_id++) {
+ struct kobject *attr_name_kobj = bioscfg_drv.password_data[instance_id].attr_name_kobj;
+
+ if (attr_name_kobj) {
+ if (strcmp(attr_name_kobj->name, SETUP_PASSWD) == 0)
+ sysfs_remove_group(attr_name_kobj,
+ &bios_password_attr_group);
+ else
+ sysfs_remove_group(attr_name_kobj,
+ &system_password_attr_group);
+ }
+ }
+ bioscfg_drv.password_instances_count = 0;
+ kfree(bioscfg_drv.password_data);
+ bioscfg_drv.password_data = NULL;
+}