| Message ID | 20220321180624.4761-1-markpearson@lenovo.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | platform/x86: think-lmi: certificate support clean ups | expand |
Hi, On 3/21/22 19:06, Mark Pearson wrote: > Complete some clean-ups as reqested from the last review as follow-ups > - Remove certificate from structure as no need to store it any more > - Clean up return code handling > - Moved freeing of signature to before admin object released (issue > seen in testing when unloading module) > - Minor code flow improvements > > Signed-off-by: Mark Pearson <markpearson@lenovo.com> Thank you for your patch, I've applied this patch 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 > --- > drivers/platform/x86/think-lmi.c | 45 +++++++++++--------------------- > drivers/platform/x86/think-lmi.h | 1 - > 2 files changed, 15 insertions(+), 31 deletions(-) > > diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c > index 1817dd8d5d95..a01a92769c1a 100644 > --- a/drivers/platform/x86/think-lmi.c > +++ b/drivers/platform/x86/think-lmi.c > @@ -740,16 +740,8 @@ static ssize_t certificate_store(struct kobject *kobj, > if (!tlmi_priv.certificate_support) > return -EOPNOTSUPP; > > - new_cert = kstrdup(buf, GFP_KERNEL); > - if (!new_cert) > - return -ENOMEM; > - /* Strip out CR if one is present */ > - strip_cr(new_cert); > - > /* If empty then clear installed certificate */ > - if (new_cert[0] == '\0') { /* Clear installed certificate */ > - kfree(new_cert); > - > + if ((buf[0] == '\0') || (buf[0] == '\n')) { /* Clear installed certificate */ > /* Check that signature is set */ > if (!setting->signature || !setting->signature[0]) > return -EACCES; > @@ -763,14 +755,16 @@ static ssize_t certificate_store(struct kobject *kobj, > > ret = tlmi_simple_call(LENOVO_CLEAR_BIOS_CERT_GUID, auth_str); > kfree(auth_str); > - if (ret) > - return ret; > > - kfree(setting->certificate); > - setting->certificate = NULL; > - return count; > + return ret ?: count; > } > > + new_cert = kstrdup(buf, GFP_KERNEL); > + if (!new_cert) > + return -ENOMEM; > + /* Strip out CR if one is present */ > + strip_cr(new_cert); > + > if (setting->cert_installed) { > /* Certificate is installed so this is an update */ > if (!setting->signature || !setting->signature[0]) { > @@ -792,21 +786,14 @@ static ssize_t certificate_store(struct kobject *kobj, > auth_str = kasprintf(GFP_KERNEL, "%s,%s", > new_cert, setting->password); > } > - if (!auth_str) { > - kfree(new_cert); > + kfree(new_cert); > + if (!auth_str) > return -ENOMEM; > - } > > ret = tlmi_simple_call(guid, auth_str); > kfree(auth_str); > - if (ret) { > - kfree(new_cert); > - return ret; > - } > > - kfree(setting->certificate); > - setting->certificate = new_cert; > - return count; > + return ret ?: count; > } > > static struct kobj_attribute auth_certificate = __ATTR_WO(certificate); > @@ -846,7 +833,6 @@ static ssize_t save_signature_store(struct kobject *kobj, > { > struct tlmi_pwd_setting *setting = to_tlmi_pwd_setting(kobj); > char *new_signature; > - int ret; > > if (!capable(CAP_SYS_ADMIN)) > return -EPERM; > @@ -1195,6 +1181,10 @@ static void tlmi_release_attr(void) > > kset_unregister(tlmi_priv.attribute_kset); > > + /* Free up any saved signatures */ > + kfree(tlmi_priv.pwd_admin->signature); > + kfree(tlmi_priv.pwd_admin->save_signature); > + > /* Authentication structures */ > sysfs_remove_group(&tlmi_priv.pwd_admin->kobj, &auth_attr_group); > kobject_put(&tlmi_priv.pwd_admin->kobj); > @@ -1211,11 +1201,6 @@ static void tlmi_release_attr(void) > } > > kset_unregister(tlmi_priv.authentication_kset); > - > - /* Free up any saved certificates/signatures */ > - kfree(tlmi_priv.pwd_admin->certificate); > - kfree(tlmi_priv.pwd_admin->signature); > - kfree(tlmi_priv.pwd_admin->save_signature); > } > > static int tlmi_sysfs_init(void) > diff --git a/drivers/platform/x86/think-lmi.h b/drivers/platform/x86/think-lmi.h > index 4f69df6eed07..4daba6151cd6 100644 > --- a/drivers/platform/x86/think-lmi.h > +++ b/drivers/platform/x86/think-lmi.h > @@ -63,7 +63,6 @@ struct tlmi_pwd_setting { > int index; /*Used for HDD and NVME auth */ > enum level_option level; > bool cert_installed; > - char *certificate; > char *signature; > char *save_signature; > };
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c index 1817dd8d5d95..a01a92769c1a 100644 --- a/drivers/platform/x86/think-lmi.c +++ b/drivers/platform/x86/think-lmi.c @@ -740,16 +740,8 @@ static ssize_t certificate_store(struct kobject *kobj, if (!tlmi_priv.certificate_support) return -EOPNOTSUPP; - new_cert = kstrdup(buf, GFP_KERNEL); - if (!new_cert) - return -ENOMEM; - /* Strip out CR if one is present */ - strip_cr(new_cert); - /* If empty then clear installed certificate */ - if (new_cert[0] == '\0') { /* Clear installed certificate */ - kfree(new_cert); - + if ((buf[0] == '\0') || (buf[0] == '\n')) { /* Clear installed certificate */ /* Check that signature is set */ if (!setting->signature || !setting->signature[0]) return -EACCES; @@ -763,14 +755,16 @@ static ssize_t certificate_store(struct kobject *kobj, ret = tlmi_simple_call(LENOVO_CLEAR_BIOS_CERT_GUID, auth_str); kfree(auth_str); - if (ret) - return ret; - kfree(setting->certificate); - setting->certificate = NULL; - return count; + return ret ?: count; } + new_cert = kstrdup(buf, GFP_KERNEL); + if (!new_cert) + return -ENOMEM; + /* Strip out CR if one is present */ + strip_cr(new_cert); + if (setting->cert_installed) { /* Certificate is installed so this is an update */ if (!setting->signature || !setting->signature[0]) { @@ -792,21 +786,14 @@ static ssize_t certificate_store(struct kobject *kobj, auth_str = kasprintf(GFP_KERNEL, "%s,%s", new_cert, setting->password); } - if (!auth_str) { - kfree(new_cert); + kfree(new_cert); + if (!auth_str) return -ENOMEM; - } ret = tlmi_simple_call(guid, auth_str); kfree(auth_str); - if (ret) { - kfree(new_cert); - return ret; - } - kfree(setting->certificate); - setting->certificate = new_cert; - return count; + return ret ?: count; } static struct kobj_attribute auth_certificate = __ATTR_WO(certificate); @@ -846,7 +833,6 @@ static ssize_t save_signature_store(struct kobject *kobj, { struct tlmi_pwd_setting *setting = to_tlmi_pwd_setting(kobj); char *new_signature; - int ret; if (!capable(CAP_SYS_ADMIN)) return -EPERM; @@ -1195,6 +1181,10 @@ static void tlmi_release_attr(void) kset_unregister(tlmi_priv.attribute_kset); + /* Free up any saved signatures */ + kfree(tlmi_priv.pwd_admin->signature); + kfree(tlmi_priv.pwd_admin->save_signature); + /* Authentication structures */ sysfs_remove_group(&tlmi_priv.pwd_admin->kobj, &auth_attr_group); kobject_put(&tlmi_priv.pwd_admin->kobj); @@ -1211,11 +1201,6 @@ static void tlmi_release_attr(void) } kset_unregister(tlmi_priv.authentication_kset); - - /* Free up any saved certificates/signatures */ - kfree(tlmi_priv.pwd_admin->certificate); - kfree(tlmi_priv.pwd_admin->signature); - kfree(tlmi_priv.pwd_admin->save_signature); } static int tlmi_sysfs_init(void) diff --git a/drivers/platform/x86/think-lmi.h b/drivers/platform/x86/think-lmi.h index 4f69df6eed07..4daba6151cd6 100644 --- a/drivers/platform/x86/think-lmi.h +++ b/drivers/platform/x86/think-lmi.h @@ -63,7 +63,6 @@ struct tlmi_pwd_setting { int index; /*Used for HDD and NVME auth */ enum level_option level; bool cert_installed; - char *certificate; char *signature; char *save_signature; };
Complete some clean-ups as reqested from the last review as follow-ups - Remove certificate from structure as no need to store it any more - Clean up return code handling - Moved freeing of signature to before admin object released (issue seen in testing when unloading module) - Minor code flow improvements Signed-off-by: Mark Pearson <markpearson@lenovo.com> --- drivers/platform/x86/think-lmi.c | 45 +++++++++++--------------------- drivers/platform/x86/think-lmi.h | 1 - 2 files changed, 15 insertions(+), 31 deletions(-)