All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/gvt/scheduler: fix potential NULL pointer dereference
@ 2018-03-19 19:30 Gustavo A. R. Silva
  2018-03-19 20:38   ` Chris Wilson
  0 siblings, 1 reply; 6+ messages in thread
From: Gustavo A. R. Silva @ 2018-03-19 19:30 UTC (permalink / raw)
  To: Zhenyu Wang, Zhi Wang, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, David Airlie
  Cc: intel-gvt-dev, intel-gfx, dri-devel, linux-kernel, Gustavo A. R. Silva

_workload_ is being dereferenced before it is null checked, hence
there is a potential null pointer dereference.

Fix this by moving the pointer dereference after _workload_ has
been null checked.

Addresses-Coverity-ID: 1430136 ("Dereference before null check")
Fixes: fa3dd623e559 ("drm/i915/gvt: keep oa config in shadow ctx")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/gpu/drm/i915/gvt/scheduler.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index 0681264..be1a297 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -60,9 +60,9 @@ static void set_context_pdp_root_pointer(
 static void sr_oa_regs(struct intel_vgpu_workload *workload,
 		u32 *reg_state, bool save)
 {
-	struct drm_i915_private *dev_priv = workload->vgpu->gvt->dev_priv;
-	u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
-	u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
+	struct drm_i915_private *dev_priv;
+	u32 ctx_oactxctrl;
+	u32 ctx_flexeu0;
 	int i = 0;
 	u32 flex_mmio[] = {
 		i915_mmio_reg_offset(EU_PERF_CNTL0),
@@ -77,6 +77,10 @@ static void sr_oa_regs(struct intel_vgpu_workload *workload,
 	if (!workload || !reg_state || workload->ring_id != RCS)
 		return;
 
+	dev_priv = workload->vgpu->gvt->dev_priv;
+	ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
+	ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
+
 	if (save) {
 		workload->oactxctrl = reg_state[ctx_oactxctrl + 1];
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/gvt/scheduler: fix potential NULL pointer dereference
  2018-03-19 19:30 [PATCH] drm/i915/gvt/scheduler: fix potential NULL pointer dereference Gustavo A. R. Silva
@ 2018-03-19 20:38   ` Chris Wilson
  0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2018-03-19 20:38 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Zhenyu Wang, Zhi Wang, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, David Airlie
  Cc: intel-gvt-dev, intel-gfx, dri-devel, linux-kernel, Gustavo A. R. Silva

Quoting Gustavo A. R. Silva (2018-03-19 19:30:53)
> _workload_ is being dereferenced before it is null checked, hence
> there is a potential null pointer dereference.
> 
> Fix this by moving the pointer dereference after _workload_ has
> been null checked.

The checks are misleading and not required.
-Chris

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/gvt/scheduler: fix potential NULL pointer dereference
@ 2018-03-19 20:38   ` Chris Wilson
  0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2018-03-19 20:38 UTC (permalink / raw)
  To: Zhenyu Wang, Zhi Wang, Jani Nikula, Joonas Lahtinen,
	Rodrigo Vivi, David Airlie
  Cc: intel-gfx, intel-gvt-dev, linux-kernel, dri-devel, Gustavo A. R. Silva

Quoting Gustavo A. R. Silva (2018-03-19 19:30:53)
> _workload_ is being dereferenced before it is null checked, hence
> there is a potential null pointer dereference.
> 
> Fix this by moving the pointer dereference after _workload_ has
> been null checked.

The checks are misleading and not required.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/gvt/scheduler: fix potential NULL pointer dereference
  2018-03-19 20:38   ` Chris Wilson
  (?)
@ 2018-03-19 20:50   ` Gustavo A. R. Silva
  2018-03-19 21:23     ` Chris Wilson
  -1 siblings, 1 reply; 6+ messages in thread
From: Gustavo A. R. Silva @ 2018-03-19 20:50 UTC (permalink / raw)
  To: Chris Wilson, Zhenyu Wang, Zhi Wang, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, David Airlie
  Cc: intel-gvt-dev, intel-gfx, dri-devel, linux-kernel

Hi Chris,

On 03/19/2018 03:38 PM, Chris Wilson wrote:
> Quoting Gustavo A. R. Silva (2018-03-19 19:30:53)
>> _workload_ is being dereferenced before it is null checked, hence
>> there is a potential null pointer dereference.
>>
>> Fix this by moving the pointer dereference after _workload_ has
>> been null checked.
> 
> The checks are misleading and not required.

All of them?

if (!workload || !reg_state || workload->ring_id != RCS)
	return;

or just the null check on workload ?

Thanks for the feedback.
--
Gustavo

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/gvt/scheduler: fix potential NULL pointer dereference
  2018-03-19 20:50   ` Gustavo A. R. Silva
@ 2018-03-19 21:23     ` Chris Wilson
  2018-03-19 21:45       ` Gustavo A. R. Silva
  0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2018-03-19 21:23 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Zhenyu Wang, Zhi Wang, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, David Airlie
  Cc: intel-gvt-dev, intel-gfx, dri-devel, linux-kernel

Quoting Gustavo A. R. Silva (2018-03-19 20:50:12)
> Hi Chris,
> 
> On 03/19/2018 03:38 PM, Chris Wilson wrote:
> > Quoting Gustavo A. R. Silva (2018-03-19 19:30:53)
> >> _workload_ is being dereferenced before it is null checked, hence
> >> there is a potential null pointer dereference.
> >>
> >> Fix this by moving the pointer dereference after _workload_ has
> >> been null checked.
> > 
> > The checks are misleading and not required.
> 
> All of them?
> 
> if (!workload || !reg_state || workload->ring_id != RCS)
>         return;

workload can not be NULL (dereference in caller), reg_state can not be
NULL (by construct from kmap()).

It may be not an RCS ring through.
-Chris

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/gvt/scheduler: fix potential NULL pointer dereference
  2018-03-19 21:23     ` Chris Wilson
@ 2018-03-19 21:45       ` Gustavo A. R. Silva
  0 siblings, 0 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2018-03-19 21:45 UTC (permalink / raw)
  To: Chris Wilson, Zhenyu Wang, Zhi Wang, Jani Nikula,
	Joonas Lahtinen, Rodrigo Vivi, David Airlie
  Cc: intel-gvt-dev, intel-gfx, dri-devel, linux-kernel



On 03/19/2018 04:23 PM, Chris Wilson wrote:
> Quoting Gustavo A. R. Silva (2018-03-19 20:50:12)
>> Hi Chris,
>>
>> On 03/19/2018 03:38 PM, Chris Wilson wrote:
>>> Quoting Gustavo A. R. Silva (2018-03-19 19:30:53)
>>>> _workload_ is being dereferenced before it is null checked, hence
>>>> there is a potential null pointer dereference.
>>>>
>>>> Fix this by moving the pointer dereference after _workload_ has
>>>> been null checked.
>>>
>>> The checks are misleading and not required.
>>
>> All of them?
>>
>> if (!workload || !reg_state || workload->ring_id != RCS)
>>          return;
> 
> workload can not be NULL (dereference in caller), reg_state can not be
> NULL (by construct from kmap()).
> 
> It may be not an RCS ring through

I got it.

I'll send the following patch then:

--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -74,7 +74,7 @@ static void sr_oa_regs(struct intel_vgpu_workload 
*workload,
                 i915_mmio_reg_offset(EU_PERF_CNTL6),
         };

-       if (!workload || !reg_state || workload->ring_id != RCS)
+       if(workload->ring_id != RCS)
                 return;

         if (save) {


Thanks
--
Gustavo

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-19 21:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-19 19:30 [PATCH] drm/i915/gvt/scheduler: fix potential NULL pointer dereference Gustavo A. R. Silva
2018-03-19 20:38 ` Chris Wilson
2018-03-19 20:38   ` Chris Wilson
2018-03-19 20:50   ` Gustavo A. R. Silva
2018-03-19 21:23     ` Chris Wilson
2018-03-19 21:45       ` Gustavo A. R. Silva

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.