* [PATCH 1/2] platform/x86: think-lmi: Fix check for admin password being set
@ 2021-06-09 15:17 Hans de Goede
2021-06-09 15:17 ` [PATCH 2/2] platform/x86: think-lmi: Avoid potential read before start of the buffer Hans de Goede
2021-06-09 15:19 ` [PATCH 1/2] platform/x86: think-lmi: Fix check for admin password being set Hans de Goede
0 siblings, 2 replies; 4+ messages in thread
From: Hans de Goede @ 2021-06-09 15:17 UTC (permalink / raw
To: Mark Gross, Andy Shevchenko
Cc: Hans de Goede, platform-driver-x86, Mark Pearson, Dan Carpenter,
coverity-bot
tlmi_priv.pwd_admin->password is an array (not a pointer), so the correct
way to check for the password being set is to check for
tlmi_priv.pwd_admin->password[0] != 0.
For the second check, replace the check with checking that auth_str is
set instead.
Cc: Mark Pearson <markpearson@lenovo.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
Addresses-Coverity-ID: 1505158 ("NO_EFFECT")
Fixes: a7314b3b1d8a ("platform/x86: think-lmi: Add WMI interface support on Lenovo platforms")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/think-lmi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
index c6413b906e4a..4881de4e669d 100644
--- a/drivers/platform/x86/think-lmi.c
+++ b/drivers/platform/x86/think-lmi.c
@@ -537,7 +537,7 @@ static ssize_t current_value_store(struct kobject *kobj,
p = strchrnul(new_setting, '\n');
*p = '\0';
- if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password) {
+ if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) {
auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s;",
tlmi_priv.pwd_admin->password,
encoding_options[tlmi_priv.pwd_admin->encoding],
@@ -563,7 +563,7 @@ static ssize_t current_value_store(struct kobject *kobj,
if (ret)
goto out;
- if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password)
+ if (auth_str)
ret = tlmi_save_bios_settings(auth_str);
else
ret = tlmi_save_bios_settings("");
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] platform/x86: think-lmi: Avoid potential read before start of the buffer
2021-06-09 15:17 [PATCH 1/2] platform/x86: think-lmi: Fix check for admin password being set Hans de Goede
@ 2021-06-09 15:17 ` Hans de Goede
2021-06-09 15:19 ` [PATCH 1/2] platform/x86: think-lmi: Fix check for admin password being set Hans de Goede
1 sibling, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2021-06-09 15:17 UTC (permalink / raw
To: Mark Gross, Andy Shevchenko
Cc: Hans de Goede, platform-driver-x86, Mark Pearson, Dan Carpenter
If length equals 0 then reading buf[length-1] will read before the start
of the buffer.
Avoid this by moving the length == 0 check up.
Cc: Mark Pearson <markpearson@lenovo.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/think-lmi.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
index 4881de4e669d..7771c9359449 100644
--- a/drivers/platform/x86/think-lmi.c
+++ b/drivers/platform/x86/think-lmi.c
@@ -443,10 +443,13 @@ static ssize_t kbdlang_store(struct kobject *kobj,
int length;
length = strlen(buf);
+ if (!length)
+ return -EINVAL;
+
if (buf[length-1] == '\n')
length--;
- if (!length || (length >= TLMI_LANG_MAXLEN))
+ if (length >= TLMI_LANG_MAXLEN)
return -EINVAL;
memcpy(setting->kbdlang, buf, length);
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] platform/x86: think-lmi: Fix check for admin password being set
2021-06-09 15:17 [PATCH 1/2] platform/x86: think-lmi: Fix check for admin password being set Hans de Goede
2021-06-09 15:17 ` [PATCH 2/2] platform/x86: think-lmi: Avoid potential read before start of the buffer Hans de Goede
@ 2021-06-09 15:19 ` Hans de Goede
2021-06-09 15:27 ` [External] " Mark Pearson
1 sibling, 1 reply; 4+ messages in thread
From: Hans de Goede @ 2021-06-09 15:19 UTC (permalink / raw
To: Mark Gross, Andy Shevchenko
Cc: platform-driver-x86, Mark Pearson, Dan Carpenter, coverity-bot
Hi,
On 6/9/21 5:17 PM, Hans de Goede wrote:
> tlmi_priv.pwd_admin->password is an array (not a pointer), so the correct
> way to check for the password being set is to check for
> tlmi_priv.pwd_admin->password[0] != 0.
>
> For the second check, replace the check with checking that auth_str is
> set instead.
>
> Cc: Mark Pearson <markpearson@lenovo.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
> Addresses-Coverity-ID: 1505158 ("NO_EFFECT")
> Fixes: a7314b3b1d8a ("platform/x86: think-lmi: Add WMI interface support on Lenovo platforms")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
I've added this series to my review-hans branch (soon to be pdx86/for-next) branch now.
Regards,
Hans
> ---
> drivers/platform/x86/think-lmi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
> index c6413b906e4a..4881de4e669d 100644
> --- a/drivers/platform/x86/think-lmi.c
> +++ b/drivers/platform/x86/think-lmi.c
> @@ -537,7 +537,7 @@ static ssize_t current_value_store(struct kobject *kobj,
> p = strchrnul(new_setting, '\n');
> *p = '\0';
>
> - if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password) {
> + if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) {
> auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s;",
> tlmi_priv.pwd_admin->password,
> encoding_options[tlmi_priv.pwd_admin->encoding],
> @@ -563,7 +563,7 @@ static ssize_t current_value_store(struct kobject *kobj,
> if (ret)
> goto out;
>
> - if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password)
> + if (auth_str)
> ret = tlmi_save_bios_settings(auth_str);
> else
> ret = tlmi_save_bios_settings("");
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [External] Re: [PATCH 1/2] platform/x86: think-lmi: Fix check for admin password being set
2021-06-09 15:19 ` [PATCH 1/2] platform/x86: think-lmi: Fix check for admin password being set Hans de Goede
@ 2021-06-09 15:27 ` Mark Pearson
0 siblings, 0 replies; 4+ messages in thread
From: Mark Pearson @ 2021-06-09 15:27 UTC (permalink / raw
To: Hans de Goede, Mark Gross, Andy Shevchenko
Cc: platform-driver-x86, Dan Carpenter, coverity-bot
On 2021-06-09 11:19 a.m., Hans de Goede wrote:
> Hi,
>
> On 6/9/21 5:17 PM, Hans de Goede wrote:
>> tlmi_priv.pwd_admin->password is an array (not a pointer), so the correct
>> way to check for the password being set is to check for
>> tlmi_priv.pwd_admin->password[0] != 0.
>>
>> For the second check, replace the check with checking that auth_str is
>> set instead.
>>
>> Cc: Mark Pearson <markpearson@lenovo.com>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
>> Addresses-Coverity-ID: 1505158 ("NO_EFFECT")
>> Fixes: a7314b3b1d8a ("platform/x86: think-lmi: Add WMI interface support on Lenovo platforms")
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> I've added this series to my review-hans branch (soon to be pdx86/for-next) branch now.
>
> Regards,
>
> Hans
>
Thanks Hans - I was just about to start looking at these. You're fast :)
I think you've fixed everything I saw flagged - let me know if there is
anything else I should look at and might have missed.
Mark
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-09 15:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-09 15:17 [PATCH 1/2] platform/x86: think-lmi: Fix check for admin password being set Hans de Goede
2021-06-09 15:17 ` [PATCH 2/2] platform/x86: think-lmi: Avoid potential read before start of the buffer Hans de Goede
2021-06-09 15:19 ` [PATCH 1/2] platform/x86: think-lmi: Fix check for admin password being set Hans de Goede
2021-06-09 15:27 ` [External] " Mark Pearson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.