[PATCH v1] drm/nouveau/clk: avoid potential null-dereference

[Ilia Mirkin]

On Thu, Jan 8, 2015 at 12:40 AM, Vince Hsu <vinceh at nvidia.com> wrote:
> On 01/08/2015 12:57 PM, Ilia Mirkin wrote:
>>
>> On Wed, Jan 7, 2015 at 11:32 PM, Vince Hsu <vinceh at nvidia.com> wrote:
>>>
>>> On 01/08/2015 10:45 AM, Ilia Mirkin wrote:
>>>>
>>>> On Wed, Jan 7, 2015 at 5:29 PM, Andy Shevchenko
>>>> <andy.shevchenko at gmail.com> wrote:
>>>>>
>>>>> We have to check pointer before usage.
>>>>>
>>>>> Reported-by: Andrey Karpov <karpov at viva64.com>
>>>>> Signed-off-by: Andy Shevchenko <andy.shevchenko at gmail.com>
>>>>> ---
>>>>>    drivers/gpu/drm/nouveau/core/subdev/clock/base.c | 4 +++-
>>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/nouveau/core/subdev/clock/base.c
>>>>> b/drivers/gpu/drm/nouveau/core/subdev/clock/base.c
>>>>> index e51b72d..2e84436 100644
>>>>> --- a/drivers/gpu/drm/nouveau/core/subdev/clock/base.c
>>>>> +++ b/drivers/gpu/drm/nouveau/core/subdev/clock/base.c
>>>>> @@ -322,7 +322,6 @@ nouveau_pstate_new(struct nouveau_clock *clk, int
>>>>> idx)
>>>>>                   return 0;
>>>>>
>>>>>           pstate = kzalloc(sizeof(*pstate), GFP_KERNEL);
>>>>> -       cstate = &pstate->base;
>>>>
>>>> What's wrong with this line? If pstate == NULL, &pstate->base == NULL
>>>> as well and we return.
>>>
>>> If pstate == NULL (kzalloc returned NULL), pstate->base triggers a null
>>> pointer
>>> deference error?
>>
>> Where do you see "pstate->base"? I only see "&pstate->base" which
>> merely computes an offset into a structure... No reason to dereference
>> pstate.
>
> Sorry that I don't quite understand. We do need deference pstate to get
> the member base which has type nouveau_cstate, and then assign
> the address of base to cstate for later use.
>
> struct nouveau_pstate {
>     struct list_head head;
>     struct list_head list; /* c-states */
>     struct nouveau_cstate base;
>     u8 pstate;
>     u8 fanspeed;
> };

&pstate->base is the same thing as

(void *)pstate + offsetof(struct nouveau_pstate, base)

At no point is pstate dereferenced. In fact, take a look at

http://en.wikipedia.org/wiki/Offsetof

which says that the traditional implementation of offsetof is

#define offsetof(st, m) ((size_t)(&((st *)0)->m))

Cheers,

  -ilia