Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1742685

Re: [PATCH v3 3/8] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check

From Pali Rohár <pali.rohar@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH v3 3/8] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check
Date 2017-09-30 22:10 +0200
Message-ID <uvwff-qT-3@gated-at.bofh.it> (permalink)
References <uuyj7-40H-7@gated-at.bofh.it> <uveLn-5E1-1@gated-at.bofh.it> <uvvVT-8vi-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


[Multipart message — attachments visible in raw view] - view raw

On Saturday 30 September 2017 21:48:39 Mario.Limonciello@dell.com wrote:
> > > +/*
> > 
> > > + * Descriptor buffer is 128 byte long and contains:
> > ...
> > 
> > > +	if (obj->buffer.length != 128) {
> > > +		dev_err(&wdev->dev,
> > > +			"Dell descriptor buffer has invalid length (%d)\n",
> > > +			obj->buffer.length);
> > 
> > This seems odd. We call it an error (not a warning) if != 128, but
> > we only abort and return an error if it's < 16.
> > 
> > If it's an error, we should return an error code, if anything above
> > 16 is acceptable but 128 is preferred, the above should be a
> > warning at best. (this scenario seems unlikely).
> 
> Hopefully the original author can speak up to the intentions here.  I
> would feel that it should have errored out if it wasn't expected
> length too.

Code below access first 16 bytes of buffer. Therefore to prevent buffer 
overflow check for 16 bytes is needed.

But IIRC we decided to do not throw error and continue driver loading 
even when buffer length is not 128 (as expected by some Dell 
documentation) as it could be possible regression because driver itself 
does not depend on buffer length.

> > > +		if (obj->buffer.length < 16) {
> > > +			ret = -EINVAL;
> > > +			goto out;
> > > +		}
> > > +	}
> > > +	desc_buffer = (u32 *)obj->buffer.pointer;
> > > +
> > > +	if (desc_buffer[0] != 0x4C4C4544 && desc_buffer[1] !=
> > > 0x494D5720)

-- 
Pali Rohár
pali.rohar@gmail.com

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v3 3/8] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check Mario Limonciello <mario.limonciello@dell.com> - 2017-09-28 06:10 +0200
  Re: [PATCH v3 3/8] platform/x86: dell-wmi-smbios: Use Dell WMI  descriptor check Darren Hart <dvhart@infradead.org> - 2017-09-30 03:30 +0200
    RE: [PATCH v3 3/8] platform/x86: dell-wmi-smbios: Use Dell WMI  descriptor check <Mario.Limonciello@dell.com> - 2017-09-30 21:50 +0200
      Re: [PATCH v3 3/8] platform/x86: dell-wmi-smbios: Use Dell WMI descriptor check Pali Rohár <pali.rohar@gmail.com> - 2017-09-30 22:10 +0200
    Re: [PATCH v3 3/8] platform/x86: dell-wmi-smbios: Use Dell WMI  descriptor check Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-10-01 10:50 +0200

csiph-web