[PATCH 2/6] drm/i915/gvt: Prevent invalid ring_id access to array regs[]
Zhenyu Wang
zhenyuw at linux.intel.com
Fri Mar 22 06:21:23 UTC 2019
On 2019.03.20 11:21:26 +0800, Colin Xu wrote:
> gvt only inits ring regs for existing ring in supported GEN graphics,
> the ring_id could be incontinuous so regs[] could have uninitialized
> value. So limit the ring_id to initilized rings.
> Access to them could cause unexpected behaviours. Although these access
> are protected by current gvt logic and won't hit during running, but still
> has potential security risk in future.
>
> intel_context_lookup() could return NULL but is_inhibit_context() will
> access the pointer. Validate intel_context pointer before using.
>
> Signed-off-by: Colin Xu <colin.xu at intel.com>
> ---
> drivers/gpu/drm/i915/gvt/mmio_context.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c b/drivers/gpu/drm/i915/gvt/mmio_context.c
> index 8a2f6e9d2257..01602c2ed85a 100644
> --- a/drivers/gpu/drm/i915/gvt/mmio_context.c
> +++ b/drivers/gpu/drm/i915/gvt/mmio_context.c
> @@ -162,6 +162,9 @@ static void load_render_mocs(struct drm_i915_private *dev_priv)
> for (ring_id = 0; ring_id < ARRAY_SIZE(regs); ring_id++) {
> if (!HAS_ENGINE(dev_priv, ring_id))
> continue;
> + if (ring_id != RCS0 && ring_id != VCS0 && ring_id != VCS1 &&
> + ring_id != BCS0 && ring_id != VECS0)
> + continue;
> offset.reg = regs[ring_id];
If we won't want to depend on any order of engine id definition, maybe just change
current array index way into a simple switch, and bail out for not valid engine.
> for (i = 0; i < GEN9_MOCS_SIZE; i++) {
> gen9_render_mocs.control_table[ring_id][i] =
> @@ -340,8 +343,12 @@ static void handle_tlb_pending_event(struct intel_vgpu *vgpu, int ring_id)
> [VECS0] = 0x4270,
> };
>
> - if (WARN_ON(ring_id >= ARRAY_SIZE(regs)))
> + if (ring_id != RCS0 && ring_id != VCS0 && ring_id != VCS1 &&
> + ring_id != BCS0 && ring_id != VECS0) {
> + WARN_ON(1);
> + gvt_vgpu_err("Invalid ring id (%d)\n", ring_id);
> return;
> + }
>
> if (!test_and_clear_bit(ring_id, (void *)s->tlb_handle_pending))
> return;
> @@ -389,8 +396,13 @@ static void switch_mocs(struct intel_vgpu *pre, struct intel_vgpu *next,
> int i;
>
> dev_priv = pre ? pre->gvt->dev_priv : next->gvt->dev_priv;
> - if (WARN_ON(ring_id >= ARRAY_SIZE(regs)))
> +
> + if (ring_id != RCS0 && ring_id != VCS0 && ring_id != VCS1 &&
> + ring_id != BCS0 && ring_id != VECS0) {
> + WARN_ON(1);
> + gvt_err("Invalid ring id (%d)\n", ring_id);
> return;
> + }
>
> if (ring_id == RCS0 &&
> (IS_KABYLAKE(dev_priv) ||
> @@ -458,6 +470,7 @@ static void switch_mmio(struct intel_vgpu *pre,
> struct drm_i915_private *dev_priv;
> struct intel_vgpu_submission *s;
> struct engine_mmio *mmio;
> + struct intel_context *ce = NULL;
> u32 old_v, new_v;
>
> dev_priv = pre ? pre->gvt->dev_priv : next->gvt->dev_priv;
> @@ -495,9 +508,8 @@ static void switch_mmio(struct intel_vgpu *pre,
> * image if it's not inhibit context, it will restore
> * itself.
> */
> - if (mmio->in_context &&
> - !is_inhibit_context(intel_context_lookup(s->shadow_ctx,
> - dev_priv->engine[ring_id])))
> + ce = intel_context_lookup(s->shadow_ctx, dev_priv->engine[ring_id]);
> + if (mmio->in_context && ce && !is_inhibit_context(ce))
> continue;
>
> if (mmio->mask)
> --
> 2.21.0
>
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20190322/b863a45b/attachment.sig>
More information about the intel-gvt-dev
mailing list