From patchwork Tue Mar 7 20:58:51 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 9609817 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 9E232602B4 for ; Tue, 7 Mar 2017 20:59:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6B2362853B for ; Tue, 7 Mar 2017 20:59:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 600762853D; Tue, 7 Mar 2017 20:59:04 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id D018C2853B for ; Tue, 7 Mar 2017 20:59:03 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5398D6E7C2; Tue, 7 Mar 2017 20:59:03 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wr0-x243.google.com (mail-wr0-x243.google.com [IPv6:2a00:1450:400c:c0c::243]) by gabe.freedesktop.org (Postfix) with ESMTPS id DDE6D6E7C0 for ; Tue, 7 Mar 2017 20:58:59 +0000 (UTC) Received: by mail-wr0-x243.google.com with SMTP id g10so1764876wrg.0 for ; Tue, 07 Mar 2017 12:58:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=43R20l8MUn389A8Pj06ij57hg+H2c6Hr3lMf4hWTPHA=; b=oTR1vz5dG91EjkeYIt8BEaF3Id8RF9vF7aYzLmE8jrRLj9Grew8Sez4E5sN4uSeHAG v65GQ6QinZ3o8h+BdyL7BDtjILPvqGREjX1fHRLhHjZCq8qGaGGWHK7ofUEBkzF0fRrw gj67lFxcdoDiNaLA/0BjQs6+GfC0Dx6Ox8A8X6OyvAO1eKplIN9kUp+f4JvHmYJhviIt 07PL5i03rWZBJnaPWaF5ievMOjpM2E5gOQeaCQQQIBjR/tgq4lK6iiwpp7qEX15AEsPc m/XSH+05bFprVUS9QlUY+7bGQMCxLWvvp2ZAHcUEkAt/sEv7cPL+QrT1U6ou9ZtyA8nu tmrQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references:mime-version:content-transfer-encoding; bh=43R20l8MUn389A8Pj06ij57hg+H2c6Hr3lMf4hWTPHA=; b=bcwIg7HGu8zbmQLXWpVvqk3EasqgzC65VdFki5ejEawr8iu51/doumX4Yr4Z+4a7VB 3KvrW4dGfNW2CQjltq0mhbygegu/n3sh04xwG8Bj8ljYep36TMUKuyj+zFRETPxjvMuL yv3nCyuKbzWqqZB0NtBiKAsO5VgImcLOSoTqrcprwurD+6chViIzmW8LckrE0T3bUaWL 9cTZu01I4XOfRyBV84jpBjF2YUYt9OpHB48tVhNV+TRKr8ezN31vHdN4VknNM/dS6msS 7MDrL/2ZM/+hS/fC5Om8/EaWhe6yj1sJro/HC3x6EZC2dsnBNtg5f0Sj2xWoh6pwIXro zyyw== X-Gm-Message-State: AMke39neOmKbeWWphhGLFqew/HUpf3N5U03GS+VK4SCKtF8G+ujffy+GESHqjjrIUMP8SA== X-Received: by 10.223.156.2 with SMTP id f2mr2313678wrc.4.1488920338232; Tue, 07 Mar 2017 12:58:58 -0800 (PST) Received: from haswell.alporthouse.com ([78.156.65.138]) by smtp.gmail.com with ESMTPSA id f67sm20412355wmd.0.2017.03.07.12.58.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 07 Mar 2017 12:58:57 -0800 (PST) From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Tue, 7 Mar 2017 20:58:51 +0000 Message-Id: <20170307205851.32578-3-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170307205851.32578-1-chris@chris-wilson.co.uk> References: <20170307205851.32578-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v2 3/3] drm/i915/userptr: Disallow wrapping GTT into a userptr X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP If we allow the user to convert a GTT mmap address into a userptr, we may end up in recursion hell, where currently we hit a mutex deadlock but other possibilities include use-after-free during the unbind/cancel_userptr. [ 143.203989] gem_userptr_bli D 0 902 898 0x00000000 [ 143.204054] Call Trace: [ 143.204137] __schedule+0x511/0x1180 [ 143.204195] ? pci_mmcfg_check_reserved+0xc0/0xc0 [ 143.204274] schedule+0x57/0xe0 [ 143.204327] schedule_timeout+0x383/0x670 [ 143.204374] ? trace_hardirqs_on_caller+0x187/0x280 [ 143.204457] ? trace_hardirqs_on_thunk+0x1a/0x1c [ 143.204507] ? usleep_range+0x110/0x110 [ 143.204657] ? irq_exit+0x89/0x100 [ 143.204710] ? retint_kernel+0x2d/0x2d [ 143.204794] ? trace_hardirqs_on_caller+0x187/0x280 [ 143.204857] ? _raw_spin_unlock_irq+0x33/0x60 [ 143.204944] wait_for_common+0x1f0/0x2f0 [ 143.205006] ? out_of_line_wait_on_atomic_t+0x170/0x170 [ 143.205103] ? wake_up_q+0xa0/0xa0 [ 143.205159] ? flush_workqueue_prep_pwqs+0x15a/0x2c0 [ 143.205237] wait_for_completion+0x1d/0x20 [ 143.205292] flush_workqueue+0x2e9/0xbb0 [ 143.205339] ? flush_workqueue+0x163/0xbb0 [ 143.205418] ? __schedule+0x533/0x1180 [ 143.205498] ? check_flush_dependency+0x1a0/0x1a0 [ 143.205681] i915_gem_userptr_mn_invalidate_range_start+0x1c7/0x270 [i915] [ 143.205865] ? i915_gem_userptr_dmabuf_export+0x40/0x40 [i915] [ 143.205955] __mmu_notifier_invalidate_range_start+0xc6/0x120 [ 143.206044] ? __mmu_notifier_invalidate_range_start+0x51/0x120 [ 143.206123] zap_page_range_single+0x1c7/0x1f0 [ 143.206171] ? unmap_single_vma+0x160/0x160 [ 143.206260] ? unmap_mapping_range+0xa9/0x1b0 [ 143.206308] ? vma_interval_tree_subtree_search+0x75/0xd0 [ 143.206397] unmap_mapping_range+0x18f/0x1b0 [ 143.206444] ? zap_vma_ptes+0x70/0x70 [ 143.206524] ? __pm_runtime_resume+0x67/0xa0 [ 143.206723] i915_gem_release_mmap+0x1ba/0x1c0 [i915] [ 143.206846] i915_vma_unbind+0x5c2/0x690 [i915] [ 143.206925] ? __lock_is_held+0x52/0x100 [ 143.207076] i915_gem_object_set_tiling+0x1db/0x650 [i915] [ 143.207236] i915_gem_set_tiling_ioctl+0x1d3/0x3b0 [i915] [ 143.207377] ? i915_gem_set_tiling_ioctl+0x5/0x3b0 [i915] [ 143.207457] drm_ioctl+0x36c/0x670 [ 143.207535] ? debug_lockdep_rcu_enabled.part.0+0x1a/0x30 [ 143.207730] ? i915_gem_object_set_tiling+0x650/0x650 [i915] [ 143.207793] ? drm_getunique+0x120/0x120 [ 143.207875] ? __handle_mm_fault+0x996/0x14a0 [ 143.207939] ? vm_insert_page+0x340/0x340 [ 143.208028] ? up_write+0x28/0x50 [ 143.208086] ? vm_mmap_pgoff+0x160/0x190 [ 143.208163] do_vfs_ioctl+0x12c/0xa60 [ 143.208218] ? debug_lockdep_rcu_enabled+0x35/0x40 [ 143.208267] ? ioctl_preallocate+0x150/0x150 [ 143.208353] ? __do_page_fault+0x36a/0x6e0 [ 143.208400] ? mark_held_locks+0x23/0xc0 [ 143.208479] ? up_read+0x1f/0x40 [ 143.208526] ? entry_SYSCALL_64_fastpath+0x5/0xc6 [ 143.208669] ? __fget_light+0xa7/0xc0 [ 143.208747] SyS_ioctl+0x41/0x70 To prevent the possibility of a deadlock, we defer scheduling the worker until after we have proven that given the current mm, the userptr range does not overlap a GGTT mmaping. If another thread tries to remap the GGTT over the userptr before the worker is scheduled, it will be stopped by its invalidate-range flushing the current work, before the deadlock can occur. v2: Improve discussion of how we end up in the deadlock. Reported-by: Michał Winiarski Testcase: igt/gem_userptr_blits/map-fixed-invalidate-gup Signed-off-by: Chris Wilson Cc: Michał Winiarski Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem_userptr.c | 76 ++++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c index dc9bf5282071..7addbf08bcb9 100644 --- a/drivers/gpu/drm/i915/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -488,6 +488,36 @@ __i915_gem_userptr_set_active(struct drm_i915_gem_object *obj, return ret; } +static bool overlaps_ggtt(struct drm_i915_gem_object *obj, struct mm_struct *mm) +{ + const struct vm_operations_struct *gem_vm_ops = + obj->base.dev->driver->gem_vm_ops; + unsigned long addr = obj->userptr.ptr; + const unsigned long end = addr + obj->base.size; + struct vm_area_struct *vma; + + /* Check for a contiguous set of vma covering the userptr, if any + * are absent, they will EFAULT. More importantly if any point back + * to a drm_i915_gem_object GTT mmaping, we may trigger a deadlock + * between the deferred gup of this userptr and the object being + * unbound calling invalidate_range -> cancel_userptr. + */ + for (vma = find_vma(mm, addr); vma; vma = vma->vm_next) { + if (vma->vm_start > addr) /* gap */ + break; + + if (vma->vm_end >= end) + return false; + + if (vma->vm_ops == gem_vm_ops) /* GTT mmapping */ + break; + + addr = vma->vm_end; + } + + return true; +} + static void __i915_gem_userptr_get_pages_worker(struct work_struct *_work) { @@ -556,8 +586,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work) } static struct sg_table * -__i915_gem_userptr_get_pages_schedule(struct drm_i915_gem_object *obj, - bool *active) +__i915_gem_userptr_get_pages_schedule(struct drm_i915_gem_object *obj) { struct get_pages_work *work; @@ -594,7 +623,7 @@ __i915_gem_userptr_get_pages_schedule(struct drm_i915_gem_object *obj, INIT_WORK(&work->work, __i915_gem_userptr_get_pages_worker); schedule_work(&work->work); - *active = true; + __i915_gem_userptr_set_active(obj, true); return ERR_PTR(-EAGAIN); } @@ -602,10 +631,10 @@ static struct sg_table * i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj) { const int num_pages = obj->base.size >> PAGE_SHIFT; + struct mm_struct *mm = obj->userptr.mm->mm; struct page **pvec; struct sg_table *pages; - int pinned, ret; - bool active; + int pinned; /* If userspace should engineer that these pages are replaced in * the vma between us binding this page into the GTT and completion @@ -632,37 +661,36 @@ i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj) return ERR_PTR(-EAGAIN); } - /* Let the mmu-notifier know that we have begun and need cancellation */ - ret = __i915_gem_userptr_set_active(obj, true); - if (ret) - return ERR_PTR(ret); - pvec = NULL; pinned = 0; - if (obj->userptr.mm->mm == current->mm) { - pvec = drm_malloc_gfp(num_pages, sizeof(struct page *), - GFP_TEMPORARY); - if (pvec == NULL) { - __i915_gem_userptr_set_active(obj, false); - return ERR_PTR(-ENOMEM); - } - pinned = __get_user_pages_fast(obj->userptr.ptr, num_pages, - !obj->userptr.read_only, pvec); + down_read(&mm->mmap_sem); + if (unlikely(overlaps_ggtt(obj, mm))) { + pinned = -EFAULT; + } else if (mm == current->mm) { + pvec = drm_malloc_gfp(num_pages, sizeof(struct page *), + GFP_TEMPORARY | + __GFP_NORETRY | + __GFP_NOWARN); + if (pvec) /* defer to worker if malloc fails */ + pinned = __get_user_pages_fast(obj->userptr.ptr, + num_pages, + !obj->userptr.read_only, + pvec); } - active = false; if (pinned < 0) pages = ERR_PTR(pinned), pinned = 0; else if (pinned < num_pages) - pages = __i915_gem_userptr_get_pages_schedule(obj, &active); + pages = __i915_gem_userptr_get_pages_schedule(obj); else pages = __i915_gem_userptr_set_pages(obj, pvec, num_pages); - if (IS_ERR(pages)) { - __i915_gem_userptr_set_active(obj, active); + up_read(&mm->mmap_sem); + + if (IS_ERR(pages)) release_pages(pvec, pinned, 0); - } drm_free_large(pvec); + return pages; }