linux-kernel - Re: [PATCH v6 09/14] platform/x86: dell-smbios: Introduce dispatcher for SMM calls

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] [day] [month] [year] [list]

Message-ID: <20171012233012.GA1470@fury>
Date:   Thu, 12 Oct 2017 16:30:12 -0700
From:   Darren Hart <dvhart@...radead.org>
To:     Mario.Limonciello@...l.com
Cc:     pali.rohar@...il.com, andy.shevchenko@...il.com,
        linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        luto@...nel.org, quasisec@...gle.com, rjw@...ysocki.net,
        mjg59@...gle.com, hch@....de, greg@...ah.com
Subject: Re: [PATCH v6 09/14] platform/x86: dell-smbios: Introduce dispatcher
 for SMM calls

On Tue, Oct 10, 2017 at 07:14:53PM +0000, Mario.Limonciello@...l.com wrote:
> > -----Original Message-----
> > From: Pali Rohár [mailto:pali.rohar@...il.com]
> > Sent: Tuesday, October 10, 2017 11:01 AM
> > To: Limonciello, Mario <Mario_Limonciello@...l.com>
> > Cc: dvhart@...radead.org; Andy Shevchenko <andy.shevchenko@...il.com>;
> > LKML <linux-kernel@...r.kernel.org>; platform-driver-x86@...r.kernel.org;
> > Andy Lutomirski <luto@...nel.org>; quasisec@...gle.com; rjw@...ysocki.net;
> > mjg59@...gle.com; hch@....de; Greg KH <greg@...ah.com>
> > Subject: Re: [PATCH v6 09/14] platform/x86: dell-smbios: Introduce dispatcher for
> > SMM calls
> > 
> > On Monday 09 October 2017 17:51:47 Mario Limonciello wrote:
> > >  static void dell_rfkill_query(struct rfkill *rfkill, void *data)
> > >  {
> > > -	struct calling_interface_buffer *buffer;
> > >  	int radio = ((unsigned long)data & 0xF);
> > >  	int hwswitch;
> > >  	int status;
> > >  	int ret;
> > >
> > > -	buffer = dell_smbios_get_buffer();
> > > -
> > > -	dell_smbios_send_request(17, 11);
> > > -	ret = buffer->output[0];
> > > +	ret = dell_send_request(17, 11, 0, 0, 0, 0);
> > 
> > Basically I do not like function which takes ten numeric parameters.
> > Before in this code there was just function with 2 parameters, now there
> > are lot of zero parameters, without information what which parameter
> > means...
> > 
> 
> In an earlier patch Darren wanted to keep dell_send_request around and
> remove code that was duplicated multiple times to clear buffer, set arguments etc,
> can you please comment on what exactly you would prefer?

There was a lot of boilerplate, and I think I suggested keeping the same
interface of dell_send_request(class, select) (with 2 arguments) rather than
replacing it with 3 or 4 lines of allocating and populating the buffer with the
same values.

I see you've updated this in v7 with two calls. I'd call that a reasonable
compromise.

> 
> It's very obvious if you look at the dell_send_request function what is happening.

There have been numerous analysis indicating that the more arguments a function
has, the more likely bugs are to be found in those functions. 6 is a lot.

-- 
Darren Hart
VMware Open Source Technology Center