[PATCH] drm/radeon/kms: only warn on mipmap size checks in r600 cs checker

[PATCH] drm/radeon/kms: only warn on mipmap size checks in r600 cs checker

[Alex Deucher]

The texture base address registers are in units of 256 bytes.
The original CS checker treated these offsets as bytes, so the
original check was wrong.  I fixed the units in a patch during
the 2.6.36 cycle, but this ended up breaking some existing
userspace (probably due to a bug in either userspace texture allocation
or the drm texture mipmap checker).  So for now, until we come
up with a better fix, just warn if the mipmap size it too large.
This will keep existing userspace working and it should be just
as safe as before when we were checking the wrong units.  These
are GPU MC addresses, so if they fall outside of the VRAM or
GART apertures, they end up at the GPU default page, so this should
be safe from a security perspective.

Signed-off-by: Alex Deucher <alexdeucher@gmail.com>
Cc: Jerome Glisse <glisse@freedesktop.org>
---
 drivers/gpu/drm/radeon/r600_cs.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/radeon/r600_cs.c b/drivers/gpu/drm/radeon/r600_cs.c
index d886494..27023a3 100644
--- a/drivers/gpu/drm/radeon/r600_cs.c
+++ b/drivers/gpu/drm/radeon/r600_cs.c
@@ -1172,7 +1172,6 @@ static inline int r600_check_texture_resource(struct radeon_cs_parser *p,  u32 i
 	if ((mipmap_size + word0) > radeon_bo_size(mipmap)) {
 		dev_warn(p->dev, "mipmap bo too small (%d %d %d %d %d %d -> %d have %ld)\n",
 			w0, h0, bpe, blevel, nlevels, word0, mipmap_size, radeon_bo_size(texture));
-		return -EINVAL;
 	}
 	return 0;
 }
-- 
1.7.1.1

Re: [PATCH] drm/radeon/kms: only warn on mipmap size checks in r600 cs checker

[Andy Furniss]

Alex Deucher wrote:
> The texture base address registers are in units of 256 bytes.
> The original CS checker treated these offsets as bytes, so the
> original check was wrong.  I fixed the units in a patch during
> the 2.6.36 cycle, but this ended up breaking some existing
> userspace (probably due to a bug in either userspace texture allocation
> or the drm texture mipmap checker).  So for now, until we come
> up with a better fix, just warn if the mipmap size it too large.
> This will keep existing userspace working and it should be just
> as safe as before when we were checking the wrong units.  These
> are GPU MC addresses, so if they fall outside of the VRAM or
> GART apertures, they end up at the GPU default page, so this should
> be safe from a security perspective.
>
> Signed-off-by: Alex Deucher<alexdeucher@gmail.com>
> Cc: Jerome Glisse<glisse@freedesktop.org>
> ---
>   drivers/gpu/drm/radeon/r600_cs.c |    1 -
>   1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/r600_cs.c b/drivers/gpu/drm/radeon/r600_cs.c
> index d886494..27023a3 100644
> --- a/drivers/gpu/drm/radeon/r600_cs.c
> +++ b/drivers/gpu/drm/radeon/r600_cs.c
> @@ -1172,7 +1172,6 @@ static inline int r600_check_texture_resource(struct radeon_cs_parser *p,  u32 i
>   	if ((mipmap_size + word0)>  radeon_bo_size(mipmap)) {
>   		dev_warn(p->dev, "mipmap bo too small (%d %d %d %d %d %d ->  %d have %ld)\n",
>   			w0, h0, bpe, blevel, nlevels, word0, mipmap_size, radeon_bo_size(texture));
> -		return -EINVAL;
>   	}
>   	return 0;
>   }

It fixes ut2004 demo OK, but it spams the logs with 000s of errors.

Re: [PATCH] drm/radeon/kms: only warn on mipmap size checks in r600 cs checker

[Dave Airlie]

On Tue, Sep 14, 2010 at 8:24 PM, Andy Furniss <andyqos@ukfsn.org> wrote:
> Alex Deucher wrote:
>>
>> The texture base address registers are in units of 256 bytes.
>> The original CS checker treated these offsets as bytes, so the
>> original check was wrong.  I fixed the units in a patch during
>> the 2.6.36 cycle, but this ended up breaking some existing
>> userspace (probably due to a bug in either userspace texture allocation
>> or the drm texture mipmap checker).  So for now, until we come
>> up with a better fix, just warn if the mipmap size it too large.
>> This will keep existing userspace working and it should be just
>> as safe as before when we were checking the wrong units.  These
>> are GPU MC addresses, so if they fall outside of the VRAM or
>> GART apertures, they end up at the GPU default page, so this should
>> be safe from a security perspective.

Probably should just remove the warning or ratelimit it.

really not much the user can do about it.

Dave.

[PATCH] drm/radeon/kms: only warn on mipmap size checks in r600 cs checker (v2)

[Alex Deucher]

The texture base address registers are in units of 256 bytes.
The original CS checker treated these offsets as bytes, so the
original check was wrong.  I fixed the units in a patch during
the 2.6.36 cycle, but this ended up breaking some existing
userspace (probably due to a bug in either userspace texture allocation
or the drm texture mipmap checker).  So for now, until we come
up with a better fix, just warn if the mipmap size it too large.
This will keep existing userspace working and it should be just
as safe as before when we were checking the wrong units.  These
are GPU MC addresses, so if they fall outside of the VRAM or
GART apertures, they end up at the GPU default page, so this should
be safe from a security perspective.

v2: Just disable the warning.  It just spams the log and there's
nothing the user can do about it.

Signed-off-by: Alex Deucher <alexdeucher@gmail.com>
Cc: Jerome Glisse <glisse@freedesktop.org>
---
 drivers/gpu/drm/radeon/r600_cs.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/r600_cs.c b/drivers/gpu/drm/radeon/r600_cs.c
index d886494..250a3a9 100644
--- a/drivers/gpu/drm/radeon/r600_cs.c
+++ b/drivers/gpu/drm/radeon/r600_cs.c
@@ -1170,9 +1170,8 @@ static inline int r600_check_texture_resource(struct radeon_cs_parser *p,  u32 i
 	/* using get ib will give us the offset into the mipmap bo */
 	word0 = radeon_get_ib_value(p, idx + 3) << 8;
 	if ((mipmap_size + word0) > radeon_bo_size(mipmap)) {
-		dev_warn(p->dev, "mipmap bo too small (%d %d %d %d %d %d -> %d have %ld)\n",
-			w0, h0, bpe, blevel, nlevels, word0, mipmap_size, radeon_bo_size(texture));
-		return -EINVAL;
+		/*dev_warn(p->dev, "mipmap bo too small (%d %d %d %d %d %d -> %d have %ld)\n",
+		  w0, h0, bpe, blevel, nlevels, word0, mipmap_size, radeon_bo_size(texture));*/
 	}
 	return 0;
 }
-- 
1.7.1.1