When an attribute is being changed if the Admin account is enabled, or if a password
is being updated then multiple WMI calls are needed.
Add mutex protection to ensure no race conditions are introduced.
Fixes: b49f72e7f96d ("platform/x86: think-lmi: Certificate authentication support")
Signed-off-by: Mark Pearson <[email protected]>
---
Changes in v2:
- New commit added after review of other patches in series.
Changes in v3:
- Simplified mutex handling as recommended.
Changes in v4:
- This was the 5th patch in the series but moved to be first.
- Added Fixes tag.
- Improved commit information to be clearer.
drivers/platform/x86/think-lmi.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
index 1138f770149d..6cf77bc26b05 100644
--- a/drivers/platform/x86/think-lmi.c
+++ b/drivers/platform/x86/think-lmi.c
@@ -14,6 +14,7 @@
#include <linux/acpi.h>
#include <linux/errno.h>
#include <linux/fs.h>
+#include <linux/mutex.h>
#include <linux/string.h>
#include <linux/types.h>
#include <linux/dmi.h>
@@ -195,6 +196,7 @@ static const char * const level_options[] = {
};
static struct think_lmi tlmi_priv;
static struct class *fw_attr_class;
+static DEFINE_MUTEX(tlmi_mutex);
/* ------ Utility functions ------------*/
/* Strip out CR if one is present */
@@ -437,6 +439,9 @@ static ssize_t new_password_store(struct kobject *kobj,
/* Strip out CR if one is present, setting password won't work if it is present */
strip_cr(new_pwd);
+ /* Use lock in case multiple WMI operations needed */
+ mutex_lock(&tlmi_mutex);
+
pwdlen = strlen(new_pwd);
/* pwdlen == 0 is allowed to clear the password */
if (pwdlen && ((pwdlen < setting->minlen) || (pwdlen > setting->maxlen))) {
@@ -493,6 +498,7 @@ static ssize_t new_password_store(struct kobject *kobj,
kfree(auth_str);
}
out:
+ mutex_unlock(&tlmi_mutex);
kfree(new_pwd);
return ret ?: count;
}
@@ -981,6 +987,9 @@ static ssize_t current_value_store(struct kobject *kobj,
/* Strip out CR if one is present */
strip_cr(new_setting);
+ /* Use lock in case multiple WMI operations needed */
+ mutex_lock(&tlmi_mutex);
+
/* Check if certificate authentication is enabled and active */
if (tlmi_priv.certificate_support && tlmi_priv.pwd_admin->cert_installed) {
if (!tlmi_priv.pwd_admin->signature || !tlmi_priv.pwd_admin->save_signature) {
@@ -1039,6 +1048,7 @@ static ssize_t current_value_store(struct kobject *kobj,
kobject_uevent(&tlmi_priv.class_dev->kobj, KOBJ_CHANGE);
}
out:
+ mutex_unlock(&tlmi_mutex);
kfree(auth_str);
kfree(set_str);
kfree(new_setting);
--
2.40.1
Code clean up to use BIT macro as suggested.
Signed-off-by: Mark Pearson <[email protected]>
---
Changes in v4:
- New patch split out from previous patch #2.
drivers/platform/x86/think-lmi.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
index 564e3fc33cfb..e3be99b44ce0 100644
--- a/drivers/platform/x86/think-lmi.c
+++ b/drivers/platform/x86/think-lmi.c
@@ -169,11 +169,11 @@ MODULE_PARM_DESC(debug_support, "Enable debug command support");
*/
#define LENOVO_CERT_THUMBPRINT_GUID "C59119ED-1C0D-4806-A8E9-59AA318176C4"
-#define TLMI_POP_PWD (1 << 0) /* Supervisor */
-#define TLMI_PAP_PWD (1 << 1) /* Power-on */
-#define TLMI_HDD_PWD (1 << 2) /* HDD/NVME */
-#define TLMI_SMP_PWD (1 << 6) /* System Management */
-#define TLMI_CERT (1 << 7) /* Certificate Based */
+#define TLMI_POP_PWD BIT(0) /* Supervisor */
+#define TLMI_PAP_PWD BIT(1) /* Power-on */
+#define TLMI_HDD_PWD BIT(2) /* HDD/NVME */
+#define TLMI_SMP_PWD BIT(6) /* System Management */
+#define TLMI_CERT BIT(7) /* Certificate Based */
#define to_tlmi_pwd_setting(kobj) container_of(kobj, struct tlmi_pwd_setting, kobj)
#define to_tlmi_attr_setting(kobj) container_of(kobj, struct tlmi_attr_setting, kobj)
--
2.40.1
The NVME/HDD index used by WMI starts at 1 so corrected the default
appropriately.
Note, zero index is still permitted in case it is required on future
platforms.
Documentation updated correspondingly
Signed-off-by: Mark Pearson <[email protected]>
---
Changes in v4:
- New patch. Split out changes into separate commit as requested.
- Update documentation.
- Details on zero index added to commit message.
Documentation/ABI/testing/sysfs-class-firmware-attributes | 4 ++--
drivers/platform/x86/think-lmi.c | 4 ++++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-firmware-attributes b/Documentation/ABI/testing/sysfs-class-firmware-attributes
index 4cdba3477176..1b3ecae80b3d 100644
--- a/Documentation/ABI/testing/sysfs-class-firmware-attributes
+++ b/Documentation/ABI/testing/sysfs-class-firmware-attributes
@@ -243,8 +243,8 @@ Description:
index:
Used with HDD and NVME authentication to set the drive index
- that is being referenced (e.g hdd0, hdd1 etc)
- This attribute defaults to device 0.
+ that is being referenced (e.g hdd1, hdd2 etc)
+ This attribute defaults to device 1.
certificate, signature, save_signature:
These attributes are used for certificate based authentication. This is
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
index 71bbe169c77e..2aaaee879488 100644
--- a/drivers/platform/x86/think-lmi.c
+++ b/drivers/platform/x86/think-lmi.c
@@ -1534,6 +1534,10 @@ static int tlmi_analyze(void)
if (!tlmi_priv.pwd_nvme)
goto fail_clear_attr;
+ /* Set default hdd/nvme index to 1 as there is no device 0 */
+ tlmi_priv.pwd_hdd->index = 1;
+ tlmi_priv.pwd_nvme->index = 1;
+
if (tlmi_priv.pwdcfg.core.password_state & TLMI_HDD_PWD) {
/* Check if PWD is configured and set index to first drive found */
if (tlmi_priv.pwdcfg.ext.hdd_user_password ||
--
2.40.1
If Opcode support is available (which is the standard for all platforms
going forward) then there is no need to have the encoding and kbdlang
attributes visible.
Signed-off-by: Mark Pearson <[email protected]>
---
Changes in v2 & v3:
- None. Version bumped in series.
Changes in v4:
- Fixed code alignment as requested.
- This patch was previously #4 and is now #8 in series.
drivers/platform/x86/think-lmi.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
index 2aaaee879488..52d1ce8dfe44 100644
--- a/drivers/platform/x86/think-lmi.c
+++ b/drivers/platform/x86/think-lmi.c
@@ -885,6 +885,11 @@ static umode_t auth_attr_is_visible(struct kobject *kobj,
return 0;
}
+ /* Don't display un-needed settings if opcode available */
+ if ((attr == &auth_encoding.attr || attr == &auth_kbdlang.attr) &&
+ tlmi_priv.opcode_support)
+ return 0;
+
return attr->mode;
}
--
2.40.1
NVME passwords identifier have been standardised across the Lenovo
systems and now use udrp and adrp (user and admin level) instead of
unvp and mnvp.
This should apparently be backwards compatible.
Fixes: 640a5fa50a42 ("platform/x86: think-lmi: Opcode support")
Signed-off-by: Mark Pearson <[email protected]>
---
Changes in v2 & v3:
- None. Version bumped in series.
Changes in v4:
- This patch was previously #2 and is now #6 in series.
- index default change split into new patch (next in series).
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 e3be99b44ce0..71bbe169c77e 100644
--- a/drivers/platform/x86/think-lmi.c
+++ b/drivers/platform/x86/think-lmi.c
@@ -461,9 +461,9 @@ static ssize_t new_password_store(struct kobject *kobj,
sprintf(pwd_type, "mhdp%d", setting->index);
} else if (setting == tlmi_priv.pwd_nvme) {
if (setting->level == TLMI_LEVEL_USER)
- sprintf(pwd_type, "unvp%d", setting->index);
+ sprintf(pwd_type, "udrp%d", setting->index);
else
- sprintf(pwd_type, "mnvp%d", setting->index);
+ sprintf(pwd_type, "adrp%d", setting->index);
} else {
sprintf(pwd_type, "%s", setting->pwd_type);
}
--
2.40.1
The system password identification was incorrect. This means that if
the password was enabled it wouldn't be detected correctly; and setting
it would not work.
Also updated code to use TLMI_SMP_PWD instead of TLMI_SYS_PWD to be in
sync with Lenovo documentation.
Fixes: 640a5fa50a42 ("platform/x86: think-lmi: Opcode support")
Signed-off-by: Mark Pearson <[email protected]>
---
Changes in v2:
- Updated define name to be SMP_PWD instead of SYS_PWD.
- Clarified in comments what each password type is.
Changes in v3:
- None. Version bump with rest of series.
Changes in v4:
- This patch was previously #2 and is now #3 in series.
- Patch split so comment updates moved into new patch (next in series).
drivers/platform/x86/think-lmi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
index 80a5c989db03..f6d1931540f1 100644
--- a/drivers/platform/x86/think-lmi.c
+++ b/drivers/platform/x86/think-lmi.c
@@ -172,7 +172,7 @@ MODULE_PARM_DESC(debug_support, "Enable debug command support");
#define TLMI_POP_PWD (1 << 0)
#define TLMI_PAP_PWD (1 << 1)
#define TLMI_HDD_PWD (1 << 2)
-#define TLMI_SYS_PWD (1 << 3)
+#define TLMI_SMP_PWD (1 << 6) /* System Management */
#define TLMI_CERT (1 << 7)
#define to_tlmi_pwd_setting(kobj) container_of(kobj, struct tlmi_pwd_setting, kobj)
@@ -1519,11 +1519,11 @@ static int tlmi_analyze(void)
tlmi_priv.pwd_power->valid = true;
if (tlmi_priv.opcode_support) {
- tlmi_priv.pwd_system = tlmi_create_auth("sys", "system");
+ tlmi_priv.pwd_system = tlmi_create_auth("smp", "system");
if (!tlmi_priv.pwd_system)
goto fail_clear_attr;
- if (tlmi_priv.pwdcfg.core.password_state & TLMI_SYS_PWD)
+ if (tlmi_priv.pwdcfg.core.password_state & TLMI_SMP_PWD)
tlmi_priv.pwd_system->valid = true;
tlmi_priv.pwd_hdd = tlmi_create_auth("hdd", "hdd");
--
2.40.1
Add comments to clarify what the different password attributes
are (as requested).
Signed-off-by: Mark Pearson <[email protected]>
---
Changes in v4:
- New patch split out from previous patch #2.
drivers/platform/x86/think-lmi.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
index f6d1931540f1..564e3fc33cfb 100644
--- a/drivers/platform/x86/think-lmi.c
+++ b/drivers/platform/x86/think-lmi.c
@@ -169,11 +169,11 @@ MODULE_PARM_DESC(debug_support, "Enable debug command support");
*/
#define LENOVO_CERT_THUMBPRINT_GUID "C59119ED-1C0D-4806-A8E9-59AA318176C4"
-#define TLMI_POP_PWD (1 << 0)
-#define TLMI_PAP_PWD (1 << 1)
-#define TLMI_HDD_PWD (1 << 2)
+#define TLMI_POP_PWD (1 << 0) /* Supervisor */
+#define TLMI_PAP_PWD (1 << 1) /* Power-on */
+#define TLMI_HDD_PWD (1 << 2) /* HDD/NVME */
#define TLMI_SMP_PWD (1 << 6) /* System Management */
-#define TLMI_CERT (1 << 7)
+#define TLMI_CERT (1 << 7) /* Certificate Based */
#define to_tlmi_pwd_setting(kobj) container_of(kobj, struct tlmi_pwd_setting, kobj)
#define to_tlmi_attr_setting(kobj) container_of(kobj, struct tlmi_attr_setting, kobj)
--
2.40.1
On Thu, 1 Jun 2023, Mark Pearson wrote:
> If Opcode support is available (which is the standard for all platforms
> going forward) then there is no need to have the encoding and kbdlang
> attributes visible.
>
> Signed-off-by: Mark Pearson <[email protected]>
Thanks a lot, the patches look good now. One small thing for future: next
time, try to arrange a series such that the patches with Fixes tags are
the first patches, in here I think it's not a big deal since 2/8 doesn't
seem to conflict with 3/8.
For all patches 1-8:
Reviewed-by: Ilpo J?rvinen <[email protected]>
--
i.
On Fri, Jun 2, 2023, at 7:12 AM, Ilpo Järvinen wrote:
> On Thu, 1 Jun 2023, Mark Pearson wrote:
>
>> If Opcode support is available (which is the standard for all platforms
>> going forward) then there is no need to have the encoding and kbdlang
>> attributes visible.
>>
>> Signed-off-by: Mark Pearson <[email protected]>
>
> Thanks a lot, the patches look good now. One small thing for future: next
> time, try to arrange a series such that the patches with Fixes tags are
> the first patches, in here I think it's not a big deal since 2/8 doesn't
> seem to conflict with 3/8.
>
Ah - OK, thanks for the note, I didn't know that.
> For all patches 1-8:
>
> Reviewed-by: Ilpo Järvinen <[email protected]>
>
Many thanks
Mark
Hi,
On 6/1/23 22:05, Mark Pearson wrote:
> When an attribute is being changed if the Admin account is enabled, or if a password
> is being updated then multiple WMI calls are needed.
> Add mutex protection to ensure no race conditions are introduced.
>
> Fixes: b49f72e7f96d ("platform/x86: think-lmi: Certificate authentication support")
> Signed-off-by: Mark Pearson <[email protected]>
Thank you for your patch-series, I've applied the series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
> ---
> Changes in v2:
> - New commit added after review of other patches in series.
> Changes in v3:
> - Simplified mutex handling as recommended.
> Changes in v4:
> - This was the 5th patch in the series but moved to be first.
> - Added Fixes tag.
> - Improved commit information to be clearer.
>
> drivers/platform/x86/think-lmi.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
> index 1138f770149d..6cf77bc26b05 100644
> --- a/drivers/platform/x86/think-lmi.c
> +++ b/drivers/platform/x86/think-lmi.c
> @@ -14,6 +14,7 @@
> #include <linux/acpi.h>
> #include <linux/errno.h>
> #include <linux/fs.h>
> +#include <linux/mutex.h>
> #include <linux/string.h>
> #include <linux/types.h>
> #include <linux/dmi.h>
> @@ -195,6 +196,7 @@ static const char * const level_options[] = {
> };
> static struct think_lmi tlmi_priv;
> static struct class *fw_attr_class;
> +static DEFINE_MUTEX(tlmi_mutex);
>
> /* ------ Utility functions ------------*/
> /* Strip out CR if one is present */
> @@ -437,6 +439,9 @@ static ssize_t new_password_store(struct kobject *kobj,
> /* Strip out CR if one is present, setting password won't work if it is present */
> strip_cr(new_pwd);
>
> + /* Use lock in case multiple WMI operations needed */
> + mutex_lock(&tlmi_mutex);
> +
> pwdlen = strlen(new_pwd);
> /* pwdlen == 0 is allowed to clear the password */
> if (pwdlen && ((pwdlen < setting->minlen) || (pwdlen > setting->maxlen))) {
> @@ -493,6 +498,7 @@ static ssize_t new_password_store(struct kobject *kobj,
> kfree(auth_str);
> }
> out:
> + mutex_unlock(&tlmi_mutex);
> kfree(new_pwd);
> return ret ?: count;
> }
> @@ -981,6 +987,9 @@ static ssize_t current_value_store(struct kobject *kobj,
> /* Strip out CR if one is present */
> strip_cr(new_setting);
>
> + /* Use lock in case multiple WMI operations needed */
> + mutex_lock(&tlmi_mutex);
> +
> /* Check if certificate authentication is enabled and active */
> if (tlmi_priv.certificate_support && tlmi_priv.pwd_admin->cert_installed) {
> if (!tlmi_priv.pwd_admin->signature || !tlmi_priv.pwd_admin->save_signature) {
> @@ -1039,6 +1048,7 @@ static ssize_t current_value_store(struct kobject *kobj,
> kobject_uevent(&tlmi_priv.class_dev->kobj, KOBJ_CHANGE);
> }
> out:
> + mutex_unlock(&tlmi_mutex);
> kfree(auth_str);
> kfree(set_str);
> kfree(new_setting);