linux-kernel - RE: [PATCH v10 12/15] platform/x86: dell-smbios: Add filtering support

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, 5 Jan 2018 14:48:39 +0000
From:   <Mario.Limonciello@...l.com>
To:     <pali.rohar@...il.com>
CC:     <dvhart@...radead.org>, <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>, <gnomes@...rguk.ukuu.org.uk>
Subject: RE: [PATCH v10 12/15] platform/x86: dell-smbios: Add filtering
 support



> -----Original Message-----
> From: platform-driver-x86-owner@...r.kernel.org [mailto:platform-driver-x86-
> owner@...r.kernel.org] On Behalf Of Pali Rohár
> Sent: Friday, January 5, 2018 8:44 AM
> To: Limonciello, Mario <Mario_Limonciello@...l.com>
> Cc: dvhart@...radead.org; 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; gnomes@...rguk.ukuu.org.uk
> Subject: Re: [PATCH v10 12/15] platform/x86: dell-smbios: Add filtering support
> 
> On Friday 05 January 2018 14:32:54 Mario.Limonciello@...l.com wrote:
> >
> >
> > > -----Original Message-----
> > > From: platform-driver-x86-owner@...r.kernel.org [mailto:platform-driver-x86-
> > > owner@...r.kernel.org] On Behalf Of Pali Rohár
> > > Sent: Friday, January 5, 2018 5:13 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>; Alan Cox
> > > <gnomes@...rguk.ukuu.org.uk>
> > > Subject: Re: [PATCH v10 12/15] platform/x86: dell-smbios: Add filtering support
> > >
> > > I know that this patch is already applied and merged, but I spotted this
> > > problem:
> > >
> > > On Thursday 19 October 2017 12:50:15 Mario Limonciello wrote:
> > > > +/* calls that are explicitly blacklisted */
> > > > +static struct smbios_call call_blacklist[] = {
> > > > +	{0x0000, 01, 07}, /* manufacturing use */
> > > > +	{0x0000, 06, 05}, /* manufacturing use */
> > > > +	{0x0000, 11, 03}, /* write once */
> > > > +	{0x0000, 11, 07}, /* write once */
> > >
> > > Numbers prefixed by zero means that they are in octal notation, right?
> > Is that how the kernel interprets an integer prefix by zero?
> 
> No, this is how C language define it. See e.g. C11 standard, section
> 6.4.4.1 Integer constants:
> 
> decimal-constant:
> 	nonzero-digit
> 	decimal-constant digit
> 
> octal-constant:
> 	0
> 	octal-constant octal-digit
> 
> So in C decimal number cannot start with digit zero.
> 
> I think the place where octal numbers are used are in permissions (0777)
> 
> > I prefixed by zero for readability, they're supposed to be decimal.
> >
> > > This can lead to misunderstanding, confusion or problems in future...
> > >
> > > Can we have all numbers either in hexadecimal or decimal notation?
> >
> > Could you elaborate more why this is problematic the way it is?
> 
> Currently it is not problem as 7 is same number in octal (07) and
> decimal (7). representation. But e.g. octal 077 is 63 in decimal.
> 
> > Are you meaning you would rather see this?
> > 	{0x0000, 1, 7}, /* manufacturing use */
> > 	{0x0000, 6, 5}, /* manufacturing use */
> > 	{0x0000, 11, 3}, /* write once */
> > 	{0x0000, 11, 7}, /* write once */
> 
> Yes, this is better. If you need to achieve alignment then use spaces.
> Really, not leading zeros.
> 
> > That seems less readable to me but should interpret the same way.
> 
> Example:
> 
> {0x000, 077, 7},
> {0x000, 007, 7},
> 
> is **not** same as
> 
> {0x000,  77, 7},
> {0x000,   7, 7},
> 
> As first number in first section is (decimal) 63, not (decimal) 77.
> 
> > Perhaps it would be better if you submit a patch with what is clearer to
> > you.
> >
> > >
> > > > +	{0x0000, 11, 11}, /* write once */
> > > > +	{0x0000, 19, -1}, /* diagnostics */
> > > > +	/* handled by kernel: dell-laptop */
> > > > +	{0x0000, CLASS_INFO, SELECT_RFKILL},
> > > > +	{0x0000, CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT},
> > > > +};
> > >
> > > --
> > > Pali Rohár
> > > pali.rohar@...il.com
> 
> --
> Pali Rohár
> pali.rohar@...il.com

Thanks very much for sharing.  I wasn't aware of this.  I'll send a patch.