drm: Branch 'master' - 2 commits

 freedreno/msm/msm_ringbuffer.c |   31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

New commits:
commit 879d7c0298d1d4bc52d71d599cc07cafb4645808
Author: Rob Clark <[email protected]>
Date:   Wed Aug 8 10:39:52 2018 -0400

    freedreno: fix use-after-free with stateobj rb's
    
    We could be dropping last reference in ->flush(), so clear the entry in
    the parent rb's table to avoid deref'ing after free'd.
    
    Also, ring_bo_del()'s use of ring_cache expects that it is dropping the
    last reference.  So drop our ref to the stateobj's ring_bo first.
    
    Signed-off-by: Rob Clark <[email protected]>

diff --git a/freedreno/msm/msm_ringbuffer.c b/freedreno/msm/msm_ringbuffer.c
index 25207221..35a7a7d4 100644
--- a/freedreno/msm/msm_ringbuffer.c
+++ b/freedreno/msm/msm_ringbuffer.c
@@ -109,6 +109,8 @@ static void ring_bo_del(struct fd_device *dev, struct fd_bo *bo)
 {
 	int ret;
 
+	assert(atomic_read(&bo->refcnt) == 1);
+
 	pthread_mutex_lock(&table_lock);
 	ret = fd_bo_cache_free(&to_msm_device(dev)->ring_cache, bo);
 	pthread_mutex_unlock(&table_lock);
@@ -310,6 +312,8 @@ static void flush_reset(struct fd_ringbuffer *ring)
 
 	for (i = 0; i < msm_ring->nr_bos; i++) {
 		struct msm_bo *msm_bo = to_msm_bo(msm_ring->bos[i]);
+		if (!msm_bo)
+			continue;
 		msm_bo->current_ring_seqno = 0;
 		fd_bo_del(&msm_bo->base);
 	}
@@ -317,7 +321,7 @@ static void flush_reset(struct fd_ringbuffer *ring)
 	/* for each of the cmd buffers, clear their reloc's: */
 	for (i = 0; i < msm_ring->submit.nr_cmds; i++) {
 		struct msm_cmd *target_cmd = msm_ring->cmds[i];
-		if (target_cmd->ring->flags & FD_RINGBUFFER_OBJECT)
+		if (!target_cmd)
 			continue;
 		target_cmd->nr_relocs = 0;
 	}
@@ -484,6 +488,16 @@ static int msm_ringbuffer_flush(struct fd_ringbuffer *ring, uint32_t *last_start
 		struct drm_msm_gem_submit_cmd *cmd = &msm_ring->submit.cmds[i];
 		struct msm_cmd *msm_cmd = msm_ring->cmds[i];
 		if (msm_cmd->ring->flags & FD_RINGBUFFER_OBJECT) {
+			/* we could have dropped last reference: */
+			msm_ring->cmds[i] = NULL;
+
+			/* need to drop ring_bo ref prior to unref'ing the ring,
+			 * because ring_bo_del assumes it is dropping the *last*
+			 * reference:
+			 */
+			fd_bo_del(msm_ring->bos[cmd->submit_idx]);
+			msm_ring->bos[cmd->submit_idx] = NULL;
+
 			msm_ringbuffer_unref(msm_cmd->ring);
 			free(U642VOID(cmd->relocs));
 		}
commit a43940eb91a88ace5519c353caaab0c85a8c0d51
Author: Rob Clark <[email protected]>
Date:   Tue Aug 7 11:13:09 2018 -0400

    freedreno: don't leak stateobj rb refs
    
    One stateobj can be emitted multiple times in a single cmdstream, but
    only the first time is a cmd entry added to the parent.  Since it will
    be only unref'd once after flush, we should only ref it the first time
    it is emitted (ie. the time it is added to cmd table).
    
    Signed-off-by: Rob Clark <[email protected]>

diff --git a/freedreno/msm/msm_ringbuffer.c b/freedreno/msm/msm_ringbuffer.c
index ec4b30fe..25207221 100644
--- a/freedreno/msm/msm_ringbuffer.c
+++ b/freedreno/msm/msm_ringbuffer.c
@@ -237,8 +237,11 @@ static int check_cmd_bo(struct fd_ringbuffer *ring,
 
 /* Ensure that submit has corresponding entry in cmds table for the
  * target cmdstream buffer:
+ *
+ * Returns TRUE if new cmd added (else FALSE if it was already in
+ * the cmds table)
  */
-static void get_cmd(struct fd_ringbuffer *ring, struct msm_cmd *target_cmd,
+static int get_cmd(struct fd_ringbuffer *ring, struct msm_cmd *target_cmd,
 		uint32_t submit_offset, uint32_t size, uint32_t type)
 {
 	struct msm_ringbuffer *msm_ring = to_msm_ringbuffer(ring);
@@ -252,7 +255,7 @@ static void get_cmd(struct fd_ringbuffer *ring, struct msm_cmd *target_cmd,
 				(cmd->size == size) &&
 				(cmd->type == type) &&
 				check_cmd_bo(ring, cmd, target_cmd->ring_bo))
-			return;
+			return FALSE;
 	}
 
 	/* create cmd buf if not: */
@@ -267,6 +270,8 @@ static void get_cmd(struct fd_ringbuffer *ring, struct msm_cmd *target_cmd,
 	cmd->pad = 0;
 
 	target_cmd->size = size;
+
+	return TRUE;
 }
 
 static void * msm_ringbuffer_hostptr(struct fd_ringbuffer *ring)
@@ -559,6 +564,7 @@ static uint32_t msm_ringbuffer_emit_reloc_ring(struct fd_ringbuffer *ring,
 {
 	struct msm_cmd *cmd = NULL;
 	uint32_t idx = 0;
+	int added_cmd = FALSE;
 
 	LIST_FOR_EACH_ENTRY(cmd, &to_msm_ringbuffer(target)->cmd_list, list) {
 		if (idx == cmd_idx)
@@ -577,7 +583,8 @@ static uint32_t msm_ringbuffer_emit_reloc_ring(struct fd_ringbuffer *ring,
 		size = cmd->size;
 	} else {
 		struct fd_ringbuffer *parent = ring->parent ? ring->parent : ring;
-		get_cmd(parent, cmd, submit_offset, size, MSM_SUBMIT_CMD_IB_TARGET_BUF);
+		added_cmd = get_cmd(parent, cmd, submit_offset, size,
+				MSM_SUBMIT_CMD_IB_TARGET_BUF);
 	}
 
 	msm_ringbuffer_emit_reloc(ring, &(struct fd_reloc){
@@ -590,7 +597,7 @@ static uint32_t msm_ringbuffer_emit_reloc_ring(struct fd_ringbuffer *ring,
 	 * being flushed), mesa can't really guarantee that a stateobj isn't
 	 * destroyed after emitted but before flush, so we must hold a ref:
 	 */
-	if (target->flags & FD_RINGBUFFER_OBJECT) {
+	if (added_cmd && (target->flags & FD_RINGBUFFER_OBJECT)) {
 		msm_ringbuffer_ref(target);
 	}
 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
--