linux-kernel - Re: [PATCH 1/1] platform/x86: think-lmi: Convert container

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 12 Apr 2024 16:42:01 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Mark Pearson <mpearson-lenovo@...ebb.ca>
cc: Mark Pearson <markpearson@...ovo.com>, Hans de Goede <hdegoede@...hat.com>, 
    "platform-driver-x86@...r.kernel.org" <platform-driver-x86@...r.kernel.org>, 
    LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] platform/x86: think-lmi: Convert container_of()
 macros to static inline

On Fri, 12 Apr 2024, Mark Pearson wrote:

> Thanks Ilpo,
> 
> On Fri, Apr 12, 2024, at 9:09 AM, Ilpo Järvinen wrote:
> > The macros to_tlmi_pwd_setting() and to_tlmi_attr_setting() are fragile
> > because they expect the variable name to be 'kobj', otherwise the build
> > will fail because container_of()'s 3rd parameter (member) is taken from
> > the parameter given to the macro.
> >
> > While at it, move them into a more logical place.
> >
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> > ---
> >  drivers/platform/x86/think-lmi.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/platform/x86/think-lmi.c 
> > b/drivers/platform/x86/think-lmi.c
> > index 9345316b45db..0f2264bb7577 100644
> > --- a/drivers/platform/x86/think-lmi.c
> > +++ b/drivers/platform/x86/think-lmi.c
> > @@ -175,9 +175,6 @@ MODULE_PARM_DESC(debug_support, "Enable debug 
> > command support");
> >  #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)
> > -
> >  static const struct tlmi_err_codes tlmi_errs[] = {
> >  	{"Success", 0},
> >  	{"Not Supported", -EOPNOTSUPP},
> > @@ -198,6 +195,16 @@ static struct think_lmi tlmi_priv;
> >  static const struct class *fw_attr_class;
> >  static DEFINE_MUTEX(tlmi_mutex);
> > 
> > +static inline struct tlmi_pwd_setting *to_tlmi_pwd_setting(struct 
> > kobject *kobj)
> > +{
> > +	return container_of(kobj, struct tlmi_pwd_setting, kobj);
> > +}
> > +
> > +static inline struct tlmi_attr_setting *to_tlmi_attr_setting(struct 
> > kobject *kobj)
> > +{
> > +	return container_of(kobj, struct tlmi_attr_setting, kobj);
> > +}
> > +
> >  /* Convert BIOS WMI error string to suitable error code */
> >  static int tlmi_errstr_to_err(const char *errstr)
> >  {
> > -- 
> > 2.39.2
> 
> Looks good to me. Let me know if you want this tested on Lenovo HW and 
> I'll do a build with this in - but it looks very uncontroversial :) 
> 
> Reviewed-by Mark Pearson <mpearson-lenovo@...ebbb.ca>

Yes, pretty uncontroversial so probably not worth the effort to test on 
HW. The compile done by lkp is good enough to capture stupid mistakes 
(which it already passed here internally before I even sent these out).

-- 
 i.