Re: [PATCH] I2C/ACPI: Fix possible ZERO_SIZE_PTR pointer dereferencing error.

[Lan Tianyu <tianyu.lan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

On 08/19/2014 11:48 PM, Mika Westerberg wrote:
> On Tue, Aug 19, 2014 at 10:38:08AM -0500, Wolfram Sang wrote:
>> On Tue, Aug 19, 2014 at 06:16:49PM +0300, Mika Westerberg wrote:
>>> On Tue, Aug 19, 2014 at 10:03:55AM -0500, Wolfram Sang wrote:
>>>> On Tue, Aug 12, 2014 at 10:33:38AM +0800, Xiubo Li wrote:
>>>>> Since we cannot make sure the 'data_len' will always be none zero
>>>>> here, and then if 'data_len' equals to zero, the kzalloc() will
>>>>> return ZERO_SIZE_PTR, which equals to ((void *)16).
>>>>
>>>> I assume the read request with length == 0 comes from a broken BIOS?
>>>
>>> I'm also interested. Does this trigger in a real system?
>>
>> Even if not now, we should consider potentially broken BIOSes, or? Which
>> extends the question to: Do we need even more sanity checks when taking
>> broken BIOSes into account?
>
> Typically ACPICA has done this work for us (e.g it fixes things upfront so
> that we get sane data). I'm not sure if it does that for I2C Operation
> Regions, though (that's why I'm asking if it happens in a real system or is
> this more like a theoretical possibility).
>
> Tianyu, any comments?
>

Sorry for later response due to leave home today. acpi_gsb_i2c_read_bytes()
dedicates for GenericSerialBus Read/Write N Bytes protocol(ACPI Spec
5.5.2.4.5.3.8). Bios wants to read N Bytes when uses this protocol and the
length specified by Bios should be greater than 1. If the Bios specified 0
bytes, the associated function(E,G read battery info) would be totally unusable.
I think such Bios can't pass through Windows certification:). From this point, I
think the check is not necessary.

If you still thought this maybe happen, I think it makes more sense to add the
check length in the ACPICA. Because ACPICA will allocate a data buffer for I2C
ACPI operation region access before call the callback. The buffer length will be
result of protocol head length plus data length. If data length is 0 and this
means the access will be invalid and ACPICA should ignore it or produce a warning.