> 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@xxxxxxxxx>
> ---
> 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ń
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
