| Message ID | 20230601200552.4396-1-mpearson-lenovo@squebb.ca (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [v4,1/8] platform/x86: think-lmi: mutex protection around multiple WMI calls | expand |
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 <mpearson-lenovo@squebb.ca> 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);
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);
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 <mpearson-lenovo@squebb.ca> --- 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(+)