Re: [PATCH] platform/x86: dell-wmi-sysman: Clean up security buffer helpers

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Mon, 9 Mar 2026, Thorsten Blum wrote:

> In calculate_security_buffer(), call strlen() once and use ALIGN() to
> round up to an even size.
> 
> In populate_security_buffer(), also avoid recomputing strlen(), rename
> the u32 pointer from 'seclen' to 'seclenp' to avoid confusion with the
> new length variable, and drop the memcpy() guard since calling it with
> size 0 is a no-op and therefore safe.
> 
> Use 'const char *' for the read-only source string in both helpers.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  .../dell/dell-wmi-sysman/dell-wmi-sysman.h    |  4 ++--
>  .../x86/dell/dell-wmi-sysman/sysman.c         | 20 ++++++++-----------
>  2 files changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h b/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h
> index 817ee7ba07ca..5278a93fdaf7 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/dell-wmi-sysman.h
> @@ -189,8 +189,8 @@ void exit_bios_attr_set_interface(void);
>  int init_bios_attr_set_interface(void);
>  int map_wmi_error(int error_code);
>  size_t calculate_string_buffer(const char *str);
> -size_t calculate_security_buffer(char *authentication);
> -void populate_security_buffer(char *buffer, char *authentication);
> +size_t calculate_security_buffer(const char *authentication);
> +void populate_security_buffer(char *buffer, const char *authentication);
>  ssize_t populate_string_buffer(char *buffer, size_t buffer_len, const char *str);
>  int set_new_password(const char *password_type, const char *new);
>  int init_bios_attr_pass_interface(void);
> diff --git a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> index 9dddab6c9397..aba9ef2cfc03 100644
> --- a/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> +++ b/drivers/platform/x86/dell/dell-wmi-sysman/sysman.c
> @@ -72,13 +72,9 @@ size_t calculate_string_buffer(const char *str)
>   *
>   * Currently only supported type is Admin password
>   */
> -size_t calculate_security_buffer(char *authentication)
> +size_t calculate_security_buffer(const char *authentication)
>  {
> -	if (strlen(authentication) > 0) {
> -		return (sizeof(u32) * 2) + strlen(authentication) +
> -			strlen(authentication) % 2;
> -	}
> -	return sizeof(u32) * 2;
> +	return sizeof(u32) * 2 + ALIGN(strlen(authentication), 2);

You need to add an include for ALIGN().

Also header for strlen() is missing (a pre-existing problem).

>  }
>  
>  /**
> @@ -88,18 +84,18 @@ size_t calculate_security_buffer(char *authentication)
>   *
>   * Currently only supported type is PLAIN TEXT
>   */
> -void populate_security_buffer(char *buffer, char *authentication)
> +void populate_security_buffer(char *buffer, const char *authentication)
>  {
> +	size_t seclen = strlen(authentication);
>  	char *auth = buffer + sizeof(u32) * 2;
>  	u32 *sectype = (u32 *) buffer;
> -	u32 *seclen = sectype + 1;
> +	u32 *seclenp = sectype + 1;
> -	*sectype = strlen(authentication) > 0 ? 1 : 0;
> -	*seclen = strlen(authentication);
> +	*sectype = !!seclen;
> +	*seclenp = seclen;
>  
>  	/* plain text */
> -	if (strlen(authentication) > 0)
> -		memcpy(auth, authentication, *seclen);
> +	memcpy(auth, authentication, seclen);

Logicwise this change seem fine to me but I wonder why is buffer char *
(again, this is a pre-existing problem). It seems u8 * would be more 
appropriate given how it's handled here as something else than characters.


Also, you don't need resend if your patch is still listed in the 
patchwork, it's not lost as long as it remains in patchwork and I'll 
eventually get to it.

-- 
 i.