From owner-svn-src-stable@freebsd.org Fri Nov 6 16:48:34 2015 Return-Path: Delivered-To: svn-src-stable@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id A5825A28D1C; Fri, 6 Nov 2015 16:48:34 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 61AAC11EC; Fri, 6 Nov 2015 16:48:34 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id tA6GmXCp076054; Fri, 6 Nov 2015 16:48:33 GMT (envelope-from jhb@FreeBSD.org) Received: (from jhb@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id tA6GmXEN076053; Fri, 6 Nov 2015 16:48:33 GMT (envelope-from jhb@FreeBSD.org) Message-Id: <201511061648.tA6GmXEN076053@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: jhb set sender to jhb@FreeBSD.org using -f From: John Baldwin Date: Fri, 6 Nov 2015 16:48:33 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r290454 - in stable: 10/sys/dev/drm2/i915 9/sys/dev/drm2/i915 X-SVN-Group: stable-10 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Nov 2015 16:48:34 -0000 Author: jhb Date: Fri Nov 6 16:48:33 2015 New Revision: 290454 URL: https://svnweb.freebsd.org/changeset/base/290454 Log: MFC 288452,289719: 288452: Most error cases in i915_gem_do_execbuffer() jump to one of two labels to release resources (such as unholding pages) when errors occur. Some recently added error checks return immediately instead of jumping to a label resulting in leaks. Fix these to jump to a label to do cleanup instead. Note that stable/9 does not have the "recently added" error checks, but it does have some older error checks (that were are no longer present in stable/10 and head) that have the same bug and this fixes those instead. 289719: i915_gem_do_execbuffer() holds the pages backing each relocation region for various reasons while executing user commands. After these commands are completed, the pages backing the relocation regions are unheld. Since relocation regions do not have to be page aligned, the code in validate_exec_list() allocates 2 extra page pointers in the array of held pages populated by vm_fault_quick_hold_pages(). However, the cleanup code that unheld the pages always assumed that only the buffer size / PAGE_SIZE pages were used. This meant that non-page aligned buffers would not unheld the last 1 or 2 pages in the list. Fix this by saving the number of held pages returned by vm_fault_quick_hold_pages() for each relocation region and using this count during cleanup. Modified: stable/10/sys/dev/drm2/i915/i915_gem_execbuffer.c Directory Properties: stable/10/ (props changed) Changes in other areas also in this revision: Modified: stable/9/sys/dev/drm2/i915/i915_gem_execbuffer.c Directory Properties: stable/9/sys/ (props changed) stable/9/sys/dev/ (props changed) Modified: stable/10/sys/dev/drm2/i915/i915_gem_execbuffer.c ============================================================================== --- stable/10/sys/dev/drm2/i915/i915_gem_execbuffer.c Fri Nov 6 16:43:22 2015 (r290453) +++ stable/10/sys/dev/drm2/i915/i915_gem_execbuffer.c Fri Nov 6 16:48:33 2015 (r290454) @@ -939,13 +939,15 @@ i915_gem_check_execbuffer(struct drm_i91 static int validate_exec_list(struct drm_i915_gem_exec_object2 *exec, int count, - vm_page_t ***map) + vm_page_t ***map, int **maplen) { vm_page_t *ma; int i, length, page_count; /* XXXKIB various limits checking is missing there */ *map = malloc(count * sizeof(*ma), DRM_I915_GEM, M_WAITOK | M_ZERO); + *maplen = malloc(count * sizeof(*maplen), DRM_I915_GEM, M_WAITOK | + M_ZERO); for (i = 0; i < count; i++) { /* First check for malicious input causing overflow */ if (exec[i].relocation_count > @@ -967,9 +969,10 @@ validate_exec_list(struct drm_i915_gem_e page_count = howmany(length, PAGE_SIZE) + 2; ma = (*map)[i] = malloc(page_count * sizeof(vm_page_t), DRM_I915_GEM, M_WAITOK | M_ZERO); - if (vm_fault_quick_hold_pages(&curproc->p_vmspace->vm_map, - exec[i].relocs_ptr, length, VM_PROT_READ | VM_PROT_WRITE, - ma, page_count) == -1) { + (*maplen)[i] = vm_fault_quick_hold_pages( + &curproc->p_vmspace->vm_map, exec[i].relocs_ptr, length, + VM_PROT_READ | VM_PROT_WRITE, ma, page_count); + if ((*maplen)[i] == -1) { free(ma, DRM_I915_GEM); (*map)[i] = NULL; return (-EFAULT); @@ -1121,6 +1124,7 @@ i915_gem_do_execbuffer(struct drm_device struct drm_clip_rect *cliprects = NULL; struct intel_ring_buffer *ring; vm_page_t **relocs_ma; + int *relocs_len; u32 ctx_id = i915_execbuffer2_get_context_id(*args); u32 exec_start, exec_len; u32 seqno; @@ -1135,7 +1139,8 @@ i915_gem_do_execbuffer(struct drm_device if (args->batch_len == 0) return (0); - ret = validate_exec_list(exec, args->buffer_count, &relocs_ma); + ret = validate_exec_list(exec, args->buffer_count, &relocs_ma, + &relocs_len); if (ret != 0) goto pre_struct_lock_err; @@ -1149,7 +1154,8 @@ i915_gem_do_execbuffer(struct drm_device if (ctx_id != 0) { DRM_DEBUG("Ring %s doesn't support contexts\n", ring->name); - return -EPERM; + ret = -EPERM; + goto pre_struct_lock_err; } break; case I915_EXEC_BLT: @@ -1157,7 +1163,8 @@ i915_gem_do_execbuffer(struct drm_device if (ctx_id != 0) { DRM_DEBUG("Ring %s doesn't support contexts\n", ring->name); - return -EPERM; + ret = -EPERM; + goto pre_struct_lock_err; } break; default: @@ -1169,7 +1176,8 @@ i915_gem_do_execbuffer(struct drm_device if (!intel_ring_initialized(ring)) { DRM_DEBUG("execbuf with invalid ring: %d\n", (int)(args->flags & I915_EXEC_RING_MASK)); - return -EINVAL; + ret = -EINVAL; + goto pre_struct_lock_err; } mode = args->flags & I915_EXEC_CONSTANTS_MASK; @@ -1401,13 +1409,11 @@ err: pre_struct_lock_err: for (i = 0; i < args->buffer_count; i++) { if (relocs_ma[i] != NULL) { - vm_page_unhold_pages(relocs_ma[i], howmany( - exec[i].relocation_count * - sizeof(struct drm_i915_gem_relocation_entry), - PAGE_SIZE)); + vm_page_unhold_pages(relocs_ma[i], relocs_len[i]); free(relocs_ma[i], DRM_I915_GEM); } } + free(relocs_len, DRM_I915_GEM); free(relocs_ma, DRM_I915_GEM); free(cliprects, DRM_I915_GEM); return ret;