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

Groups > linux.kernel > #1307282

Re: [PATCH v2 2/2] dell-wmi: Process only one event on devices with interface version 0

Show all headers | View raw

> BIOS/ACPI on devices with WMI interface version 0 does not clear buffer
> before filling it. So next time when BIOS/ACPI send WMI event which is
> smaller as previous then it contains garbage in buffer from previous event.
> 
> BIOS/ACPI on devices with WMI interface version 1 clears buffer and
> sometimes send more events in buffer at one call.
> 
> Since commit 83fc44c32ad8 ("dell-wmi: Update code for processing WMI
> events") dell-wmi process all events in buffer (and not just first).
> 
> So to prevent reading garbage from buffer we will process only first one
> event on devices with WMI interface version 0.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  drivers/platform/x86/dell-wmi.c |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> index 1ad7a7b..5db9efb 100644
> --- a/drivers/platform/x86/dell-wmi.c
> +++ b/drivers/platform/x86/dell-wmi.c
> @@ -237,6 +237,22 @@ static void dell_wmi_notify(u32 value, void *context)
>  
>  	buffer_end = buffer_entry + buffer_size;
>  
> +	/*
> +	 * BIOS/ACPI on devices with WMI interface version 0 does not clear
> +	 * buffer before filling it. So next time when BIOS/ACPI send WMI event
> +	 * which is smaller as previous then it contains garbage in buffer from
> +	 * previous event.
> +	 *
> +	 * BIOS/ACPI on devices with WMI interface version 1 clears buffer and
> +	 * sometimes send more events in buffer at one call.
> +	 *
> +	 * So to prevent reading garbage from buffer we will process only first
> +	 * one event on devices with WMI interface version 0.
> +	 */
> +	if (dell_wmi_interface_version == 0 && buffer_entry < buffer_end)
> +		if (buffer_end > buffer_entry + buffer_entry[0] + 1)
> +			buffer_end = buffer_entry + buffer_entry[0] + 1;

Wouldn't it be a bit more clear if we clamped buffer_size before setting
buffer_end?  E.g. like this:

	if (buffer_size == 0)
		return;

	if (dell_wmi_interface_version == 0 &&
	    buffer_size > buffer_entry[0] + 1)
		buffer_size = buffer_entry[0] + 1;

	buffer_end = buffer_entry + buffer_size;

If I understand correctly, the second check on the first line added by
your patch prevents a bad dereference when accesing buffer_entry[0].
The only case when that may happen is when buffer_size is 0, which means
we got notified with rubbish anyway, so we can just return (perhaps with
a log message, which I omitted above).

One more minor nit: you should probably decide between "first" and "one"
as the phrase "only first one event" (found both in the commit message
and in the code comment) sounds incorrect to me.

-- 
Best regards,
Michał Kępień

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

Thread

[PATCH v2 0/2] Fixes for dell-wmi Pali Rohár <pali.rohar@gmail.com> - 2016-01-04 22:30 +0100
  [PATCH v2 1/2] dell-wmi: Check if Dell WMI descriptor structure is valid Pali Rohár <pali.rohar@gmail.com> - 2016-01-04 22:30 +0100
  [PATCH v2 2/2] dell-wmi: Process only one event on devices with interface version 0 Pali Rohár <pali.rohar@gmail.com> - 2016-01-04 22:30 +0100
    Re: [PATCH v2 2/2] dell-wmi: Process only one event on devices with  interface version 0 Michał Kępień <kernel@kempniu.pl> - 2016-01-12 12:20 +0100
      Re: [PATCH v2 2/2] dell-wmi: Process only one event on devices with interface version 0 Pali Rohár <pali.rohar@gmail.com> - 2016-01-12 18:50 +0100
        Re: [PATCH v2 2/2] dell-wmi: Process only one event on devices with  interface version 0 Michał Kępień <kernel@kempniu.pl> - 2016-01-12 21:20 +0100
          Re: [PATCH v2 2/2] dell-wmi: Process only one event on devices with  interface version 0 Darren Hart <dvhart@infradead.org> - 2016-01-15 00:10 +0100
  Re: [PATCH v2 0/2] Fixes for dell-wmi Darren Hart <dvhart@infradead.org> - 2016-01-11 20:30 +0100
    Re: [PATCH v2 0/2] Fixes for dell-wmi Gabriele Mazzotta <gabriele.mzt@gmail.com> - 2016-01-12 01:40 +0100

csiph-web