[Intel-gfx] Potential NULL pointer dereference in intel_crt_get_edid

Shaobo He shaobo at cs.utah.edu
Tue Mar 19 00:24:52 UTC 2019


I see. In light of this commit, is it a better solution than adding NULL-checks 
is to replace the if branch conditioned by `WARN_ON` with simply `WARN` like the 
following,

struct i2c_adapter *intel_gmbus_get_adapter(struct drm_i915_private *dev_priv,
					    unsigned int pin)
{
	WARN(!intel_gmbus_is_valid_pin(dev_priv, pin), "Invalid pin: %d\n", pin);

	return &dev_priv->gmbus[pin].adapter;
}

Shaobo
On 3/18/19 5:53 PM, Rodrigo Vivi wrote:
> On Mon, Mar 18, 2019 at 05:39:48PM -0600, Shaobo He wrote:
>> Hi Rodrigo,
>>
>> Sorry I'm a bit lost here. May I ask where the `WARN` is?
> 
> along with the return NULL
> 
> struct i2c_adapter *intel_gmbus_get_adapte()
>    if (WARN_ON(!intel_gmbus_is_valid_pin(dev_priv, pin)))
>                  return NULL;
> 
>>
>> Thanks,
>> Shaobo
>> On 3/18/19 5:26 PM, Rodrigo Vivi wrote:
>>> Hi Shaobo,
>>>
>>> n Mon, Mar 18, 2019 at 05:01:10PM -0600, Shaobo He wrote:
>>>> Hello everyone,
>>>>
>>>> My name is Shaobo He and I am a graduate student at University of Utah. I am
>>>> using a static analysis tool to search for null pointer dereferences and
>>>> came across a potentially invalid memory access in the file
>>>> drivers/gpu/drm/i915/intel_crt.c: in function `intel_crt_detect_ddc`,
>>>> function `intel_gmbus_get_adapter` can return a NULL pointer which is
>>>
>>> if this happens we've done a terrible job on defining the platform...
>>>
>>>> dereferenced by the call to `drm_get_edid` or `intel_gmbus_is_forced_bit`.
>>>
>>> but it seems you are right... this will reach i2c_transfer in the end
>>> and it will break everything after we gave the Warning...
>>>
>>>> It seems that the return value of `intel_gmbus_get_adapter` is never
>>>> NULL-checked. If so, it would be better to replace the branch to return a
>>>> NULL pointer with something like `BUG_ON`.
>>>
>>> what about just adding if (!i2c) return false
>>> instead of BUG.
>>>
>>> We already have the WARN to debug if this case ever happens.
>>>
>>> Thanks,
>>> Rodrigo.
>>>
>>>>
>>>> Please let me know if it makes sense. I am looking forward to your reply.
>>>>
>>>> Best,
>>>> Shaobo


More information about the Intel-gfx mailing list