Am 03.04.2013 19:10, schrieb Jerome Glisse:
On Wed, Apr 03, 2013 at 05:53:55PM +0200, Christian König wrote:
Am 03.04.2013 16:53, schrieb Jerome Glisse:
On Wed, Apr 03, 2013 at 01:18:31AM +0200, Christian König wrote:
[SNIP]
/* hardcode those limit for now */
#define RADEON_VA_IB_OFFSET (1 << 20)
#define RADEON_VA_RESERVED_SIZE (8 << 20)
@@ -357,8 +360,9 @@ struct radeon_bo_list {
struct ttm_validate_buffer tv;
struct radeon_bo *bo;
uint64_t gpu_offset;
- unsigned rdomain;
- unsigned wdomain;
+ bool written;
+ unsigned domain;
+ unsigned alt_domain;
u32 tiling_flags;
};
I think that the change to the rdomain/wdomain should be in a patch
of its own. I think the change is fine but we had issue with change
that touched that part previously, would make bisecting and
understanding the change implication easier.
Agree, I actually planed to do so, but for the whole IP review stuff
we needed to maintain a more or less stable patch base. Long story,
but I'm going to change it.
@@ -826,7 +830,6 @@ struct radeon_cs_reloc {
struct radeon_bo *robj;
struct radeon_bo_list lobj;
uint32_t handle;
- uint32_t flags;
};
Why removing the flags ? iirc it's not really use right now but i
remember plan to use it.
Ups, just a rebasing artifact. But when it's unused we should remove
it, probably just not in this patch.
[SNIP]
+static int radeon_uvd_cs_reloc(struct radeon_cs_parser *p, int data0, int data1)
+{
+ struct radeon_cs_chunk *relocs_chunk;
+ struct radeon_cs_reloc *reloc;
+ unsigned idx, cmd;
+ uint64_t start, end;
+
+ relocs_chunk = &p->chunks[p->chunk_relocs_idx];
+ idx = radeon_get_ib_value(p, data1);
+ if (idx >= relocs_chunk->length_dw) {
+ DRM_ERROR("Relocs at %d after relocations chunk end %d !\n",
+ idx, relocs_chunk->length_dw);
+ return -EINVAL;
+ }
+
+ reloc = p->relocs_ptr[(idx / 4)];
+ start = reloc->lobj.gpu_offset;
+ end = start + radeon_bo_size(reloc->robj);
+ start += radeon_get_ib_value(p, data0);
I am assuming there is no way for you to know the size that the uvd engine will write to ?
You are not checking anything on uvd possibly overwritting after the bo end.
Yeah that gave me headache for a quite long time, too. The problem
is to figure out how much is actually written you need to keep track
of the whole lot of informations including the UVD session,
create/decode/destroy messages and allot of fiddling with the codec
specific parameters.
And if I understand the UVD internals correctly even if we check
everything there is no guarantee that a special crafted bitstream
could not let UVD to write over the end of the buffer....
Is it ok if we but a big TODO on it for the initial patch?
Cheers,
Christian.
I think i only need one assurance and i think for uvd this will be the case.
If UVD block write past bo end can you be sure that no matter what it will
overwritte to address > start ie it could not overwritte to begining of VRAM.
I have big doubt on that given the 256M window, i fear that it might go back
to writting to begining of memory where the page table is.
Crafting an attack from it would still be a bit tricky because it is
compressed image data that gets written, but never less it is indeed
possible.
Note that i think that now that we have cp dma pagetable entry update we can
probably just move the pagetable to end of vram on 90% GPU with UVD this will
be > 256M which seems like a zone where UVD can never write.
Well not exactly, it is planned that the 256M limit goes away with some
of the next hw generations. And at least at this point we need to make
sure that UVD never writes somewhere it shouldn't. Anyway moving the
page table to not CPU accessible VRAM sounds like a pretty good idea.
If we can have such assurance i guess we can make uvd as an option and make
a very explicit comment stating that UVD engine can be use as an exploit
vector path.
I think I will just sit down and implement size checking, at least for
the destination buffer, cause after all that's just a texture. And for
the reference buffer I maybe just use what userspace send to the
hardware as buffer size, and make a sanity check on that one also.
Ok, need to think about it a bit more.
Jerome
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
