diff mbox

drm/i915/gvt/scheduler: fix potential NULL pointer dereference

Message ID 20180319193053.GA7016@embeddedgus (mailing list archive)
State New, archived
Headers show

Commit Message

Gustavo A. R. Silva March 19, 2018, 7:30 p.m. UTC
_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(-)

Comments

Chris Wilson March 19, 2018, 8:38 p.m. UTC | #1
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
Gustavo A. R. Silva March 19, 2018, 8:50 p.m. UTC | #2
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
Chris Wilson March 19, 2018, 9:23 p.m. UTC | #3
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
diff mbox

Patch

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];