.../dell/dell-wmi-sysman/dell-wmi-sysman.h | 4 ++-- .../x86/dell/dell-wmi-sysman/sysman.c | 20 ++++++++----------- 2 files changed, 10 insertions(+), 14 deletions(-)
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);
}
/**
@@ -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);
}
/**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.
On Tue, Mar 31, 2026 at 03:46:11PM +0300, Ilpo Järvinen wrote: > 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(-) > > > > [...] > > 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. Yes, I agree. However, using u8 * would also require changing the call sites in biosattr-interface.c and passwordattr-interface.c. Happy to change it, but it would probably be better as a follow-up patch. > 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 wasn't aware of the patchwork instance - is it this one? https://patchwork.kernel.org/project/platform-driver-x86/list/ I see 67 patches in total, but this one is not part of the list, afaict. Thanks, Thorsten
On Tue, 31 Mar 2026, Thorsten Blum wrote: > On Tue, Mar 31, 2026 at 03:46:11PM +0300, Ilpo Järvinen wrote: > > 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(-) > > > > > > [...] > > > > 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. > > Yes, I agree. However, using u8 * would also require changing the call > sites in biosattr-interface.c and passwordattr-interface.c. Happy to > change it, but it would probably be better as a follow-up patch. A follow-up patch is fine. > > 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 wasn't aware of the patchwork instance - is it this one? > > https://patchwork.kernel.org/project/platform-driver-x86/list/ Correct. You can find that URL from MAINTAINERS. > I see 67 patches in total, but this one is not part of the list, afaict. It was there until today but I marked it changes required after noting it lacks the header so I'm expecting a new version. -- i.
© 2016 - 2026 Red Hat, Inc.