* [PATCH v1] drm/nouveau/clk: avoid potential null-dereference @ 2015-01-07 22:29 Andy Shevchenko 2015-01-08 2:45 ` Ilia Mirkin 0 siblings, 1 reply; 7+ messages in thread From: Andy Shevchenko @ 2015-01-07 22:29 UTC (permalink / raw) To: dri-devel, Alexandre Courbot, David Airlie; +Cc: Andy Shevchenko We have to check pointer before usage. Reported-by: Andrey Karpov <karpov@viva64.com> Signed-off-by: Andy Shevchenko <andy.shevchenko@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; if (!pstate) return -ENOMEM; @@ -330,6 +329,9 @@ nouveau_pstate_new(struct nouveau_clock *clk, int idx) pstate->pstate = perfE.pstate; pstate->fanspeed = perfE.fanspeed; + + cstate = &pstate->base; + cstate->voltage = perfE.voltage; cstate->domain[nv_clk_src_core] = perfE.core; cstate->domain[nv_clk_src_shader] = perfE.shader; -- 1.8.3.101.g727a46b _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v1] drm/nouveau/clk: avoid potential null-dereference 2015-01-07 22:29 [PATCH v1] drm/nouveau/clk: avoid potential null-dereference Andy Shevchenko @ 2015-01-08 2:45 ` Ilia Mirkin 2015-01-08 4:32 ` Vince Hsu 0 siblings, 1 reply; 7+ messages in thread From: Ilia Mirkin @ 2015-01-08 2:45 UTC (permalink / raw) To: Andy Shevchenko; +Cc: dri-devel@lists.freedesktop.org On Wed, Jan 7, 2015 at 5:29 PM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > We have to check pointer before usage. > > Reported-by: Andrey Karpov <karpov@viva64.com> > Signed-off-by: Andy Shevchenko <andy.shevchenko@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) > return -ENOMEM; > > @@ -330,6 +329,9 @@ nouveau_pstate_new(struct nouveau_clock *clk, int idx) > > pstate->pstate = perfE.pstate; > pstate->fanspeed = perfE.fanspeed; > + > + cstate = &pstate->base; > + > cstate->voltage = perfE.voltage; > cstate->domain[nv_clk_src_core] = perfE.core; > cstate->domain[nv_clk_src_shader] = perfE.shader; > -- > 1.8.3.101.g727a46b > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1] drm/nouveau/clk: avoid potential null-dereference 2015-01-08 2:45 ` Ilia Mirkin @ 2015-01-08 4:32 ` Vince Hsu 2015-01-08 4:57 ` Ilia Mirkin 0 siblings, 1 reply; 7+ messages in thread From: Vince Hsu @ 2015-01-08 4:32 UTC (permalink / raw) To: Ilia Mirkin, Andy Shevchenko; +Cc: dri-devel@lists.freedesktop.org On 01/08/2015 10:45 AM, Ilia Mirkin wrote: > On Wed, Jan 7, 2015 at 5:29 PM, Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: >> We have to check pointer before usage. >> >> Reported-by: Andrey Karpov <karpov@viva64.com> >> Signed-off-by: Andy Shevchenko <andy.shevchenko@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? Thanks, Vince > >> if (!pstate) >> return -ENOMEM; >> >> @@ -330,6 +329,9 @@ nouveau_pstate_new(struct nouveau_clock *clk, int idx) >> >> pstate->pstate = perfE.pstate; >> pstate->fanspeed = perfE.fanspeed; >> + >> + cstate = &pstate->base; >> + >> cstate->voltage = perfE.voltage; >> cstate->domain[nv_clk_src_core] = perfE.core; >> cstate->domain[nv_clk_src_shader] = perfE.shader; >> -- >> 1.8.3.101.g727a46b >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1] drm/nouveau/clk: avoid potential null-dereference 2015-01-08 4:32 ` Vince Hsu @ 2015-01-08 4:57 ` Ilia Mirkin 2015-01-08 5:40 ` Vince Hsu 0 siblings, 1 reply; 7+ messages in thread From: Ilia Mirkin @ 2015-01-08 4:57 UTC (permalink / raw) To: Vince Hsu; +Cc: Andy Shevchenko, dri-devel@lists.freedesktop.org On Wed, Jan 7, 2015 at 11:32 PM, Vince Hsu <vinceh@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@gmail.com> wrote: >>> >>> We have to check pointer before usage. >>> >>> Reported-by: Andrey Karpov <karpov@viva64.com> >>> Signed-off-by: Andy Shevchenko <andy.shevchenko@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. -ilia _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1] drm/nouveau/clk: avoid potential null-dereference 2015-01-08 4:57 ` Ilia Mirkin @ 2015-01-08 5:40 ` Vince Hsu 2015-01-08 5:52 ` Ilia Mirkin 0 siblings, 1 reply; 7+ messages in thread From: Vince Hsu @ 2015-01-08 5:40 UTC (permalink / raw) To: Ilia Mirkin; +Cc: Andy Shevchenko, dri-devel@lists.freedesktop.org On 01/08/2015 12:57 PM, Ilia Mirkin wrote: > On Wed, Jan 7, 2015 at 11:32 PM, Vince Hsu <vinceh@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@gmail.com> wrote: >>>> We have to check pointer before usage. >>>> >>>> Reported-by: Andrey Karpov <karpov@viva64.com> >>>> Signed-off-by: Andy Shevchenko <andy.shevchenko@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; }; Thanks, Vince ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1] drm/nouveau/clk: avoid potential null-dereference 2015-01-08 5:40 ` Vince Hsu @ 2015-01-08 5:52 ` Ilia Mirkin 2015-01-08 6:02 ` Vince Hsu 0 siblings, 1 reply; 7+ messages in thread From: Ilia Mirkin @ 2015-01-08 5:52 UTC (permalink / raw) To: Vince Hsu; +Cc: Andy Shevchenko, dri-devel@lists.freedesktop.org On Thu, Jan 8, 2015 at 12:40 AM, Vince Hsu <vinceh@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@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@gmail.com> wrote: >>>>> >>>>> We have to check pointer before usage. >>>>> >>>>> Reported-by: Andrey Karpov <karpov@viva64.com> >>>>> Signed-off-by: Andy Shevchenko <andy.shevchenko@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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1] drm/nouveau/clk: avoid potential null-dereference 2015-01-08 5:52 ` Ilia Mirkin @ 2015-01-08 6:02 ` Vince Hsu 0 siblings, 0 replies; 7+ messages in thread From: Vince Hsu @ 2015-01-08 6:02 UTC (permalink / raw) To: Ilia Mirkin; +Cc: Andy Shevchenko, dri-devel@lists.freedesktop.org On 01/08/2015 01:52 PM, Ilia Mirkin wrote: > On Thu, Jan 8, 2015 at 12:40 AM, Vince Hsu <vinceh@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@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@gmail.com> wrote: >>>>>> We have to check pointer before usage. >>>>>> >>>>>> Reported-by: Andrey Karpov <karpov@viva64.com> >>>>>> Signed-off-by: Andy Shevchenko <andy.shevchenko@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)) > Oh I got your point. You're absolutely right. Thanks for the patience. :) Vince ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-01-08 6:02 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-07 22:29 [PATCH v1] drm/nouveau/clk: avoid potential null-dereference Andy Shevchenko 2015-01-08 2:45 ` Ilia Mirkin 2015-01-08 4:32 ` Vince Hsu 2015-01-08 4:57 ` Ilia Mirkin 2015-01-08 5:40 ` Vince Hsu 2015-01-08 5:52 ` Ilia Mirkin 2015-01-08 6:02 ` Vince Hsu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.