Re: [PATCH 09/10] drm/etnaviv: validate readback register address

[Lucas Stach <l.stach@pengutronix.de>]

Am Freitag, den 09.12.2016, 12:21 +0100 schrieb Christian Gmeiner:
> Reading some registers end in a system crash ala:
> 
>   Unhandled fault: external abort on non-linefetch (0x1028) at 0xfe641000
>   Internal error: : 1028 [#1] PREEMPT ARM
> 
> Avoid those by register validation.

Avoiding crashes is one thing, but I believe this needs to go further
and avoid reads from any register that isn't a performance counter. This
probably isn't a big hole, but we want to avoid leaking the GPU state to
userspace.

Regards,
Lucas

> 
> Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 08f9b3d..4383c0d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -277,6 +277,27 @@ static int submit_reloc(struct etnaviv_gem_submit *submit, void *stream,
>  	return 0;
>  }
>  
> +static int readback_reg_valid(unsigned reg)
> +{
> +	/*
> +	 * 0x000..0x200:     ok
> +	 * 0x200..0x400:     crash
> +	 * 0x400..0x800:     ok
> +	 * 0x800..0xa00:     crash
> +	 * 0xa00..0xc00:     crash
> +	 * 0xc00..0xe00:     crash
> +	 * 0xe00..0x1000:    crash
> +	 * everything above: crash
> +	 */
> +	if (reg >= 0x200 && reg < 400)
> +		return 0;
> +
> +	if (reg >= 0x800)
> +		return 0;
> +
> +	return 1;
> +}
> +
>  static int submit_readback(struct etnaviv_gem_submit *submit,
>  		struct etnaviv_cmdbuf *cmdbuf,
>  		const struct drm_etnaviv_gem_submit_readback *readbacks,
> @@ -303,6 +324,11 @@ static int submit_readback(struct etnaviv_gem_submit *submit,
>  			return -EINVAL;
>  		}
>  
> +		if (!readback_reg_valid(r->reg)) {
> +			DRM_ERROR("invalid readback reg (would cause crash)");
> +			return -EINVAL;
> +		}
> +
>  		cmdbuf->readbacks[i].bo_vma = etnaviv_gem_vmap(&bo->obj->base);
>  		cmdbuf->readbacks[i].offset = r->readback_offset;
>  		cmdbuf->readbacks[i].reg = r->reg;


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel