From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qt1-f182.google.com (mail-qt1-f182.google.com [209.85.160.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DB9472989B5 for ; Wed, 12 Aug 2026 04:13:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.182 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786507993; cv=none; b=BbC6gEa0xefn1wGAdbBeJb6M01GfbhE35EmKtThvHE7tmpVIf0RgjF8m2VBQS6upjqOCkEOdCam9T06xjtHzaJ+2Pg16y6jFeee6XmHGw2S4P0nwa3dWxLhAfwxD9Gh4n1Uv20uzGcknujfAzcImlfvUps6SUhUFBnhZmPUil2I= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786507993; c=relaxed/simple; bh=Dzfwonr9rKeQnkqeStmW/2Bvbf4Y0mUvyLlk4d31nfA=; h=Content-Type:Mime-Version:Subject:From:In-Reply-To:Date:Cc: Message-Id:References:To; b=RUOQcF9lH1A5NX9qoDaYJOWX4C8PdzB/KJmXUYqbN4CCYdP42YZWWiQNBsLB/wqfa+EoM8/sFdMR9LagGoh071GbS40ROG2uAtWYsYym1ESFvKPMnXEqei6FIF7Jn9UTfteWP3LDC2o6v+zeXv28GHYpW5fK1VJ5eUc9lDZXpFE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ebNFBBId; arc=none smtp.client-ip=209.85.160.182 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ebNFBBId" Received: by mail-qt1-f182.google.com with SMTP id d75a77b69052e-51bfad59921so1553071cf.0 for ; Tue, 11 Aug 2026 21:13:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1786507991; x=1787112791; darn=vger.kernel.org; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:content-type:from:to:cc :subject:date:message-id:reply-to:content-type; bh=WoA62XvqBE/bJkGlo+pvls7iLFgdIjTnZ2CNMnT1eiI=; b=ebNFBBId2fv/SUOpopDfigYvG137FWd9dsh6zNOBMCIxpQ9Y1BkzD1wbDdXZB4qoK+ hffx1Jex9QWamcN5Vc64ONrR5hbZiQ2R+FAcvyyBH5hnFG4r+Xtldxj9d912uMA7SvWM SNPuntLjHP0ftdkjAzWIi1cVejTP6Btf+29QTTeEAJZvn+ZIT3glhUv0ABU2m0Iy45jg rTGas/2JHMQ4KL9WWISURBLd0jrylJMlTZa9I2oNooSFRgcrZT1VyjDETHoY0dht10it q/Hx6xzmlOIT9HTHm6Jos+skzKuvjF/PMu/H6zUo9/Ubmxk91xT/ODJNvdjNkQ0xWmBT PD1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786507991; x=1787112791; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:content-type:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=WoA62XvqBE/bJkGlo+pvls7iLFgdIjTnZ2CNMnT1eiI=; b=cEVC6jxEYb7riPNiRspQr8yR0d6IrSYl1jUfh5eEQmI2TvuwCpWbhaygKr5Jt06UAH Ry7TNh3F1ac4vC+uMcRtddv5M+vpguIwUTKJ6rceOFHrz/1LK0du04H4uxX96ccYR5Bt vJe6UMZrdohWxdjh5Oo/zBXR/KiICZIU6++RB2S8vujwNdx8abtxKEgMJnSlCV84K1S1 g9zN271sIG8H+v+wH05hgSEuQk3UMVClUZA73WrLpVq2G9Xcqf/MC3yPPZ1kFJYgKG3N sEUP882tDxzat7I192GNrnkupu7vjGu30Ws/w1w/CH4FXlRXdCO5ZB6/jEWiRa2Ba8R2 L43w== X-Forwarded-Encrypted: i=1; AHgh+RoEbAUwLxQw5L9Ir10Uq4PtxVrynwat3dwb5OOS06ZskV/DxkcVHq+LJUoVdPAOLcn2JvJB9xM3olHEeOo=@vger.kernel.org X-Gm-Message-State: AOJu0YwpQxmwovlG6HTHpetSFlUQU90LFCogsp9RA2O/u+opsk0+Dqt/ f4EVyEivVg/iRqDKjD+CHOV5Bi86Vr5qpUDuPj/hp4XTdlKLpIL6VAIF X-Gm-Gg: AR+sD10EdbsUDxtwhqpY8LGCd7WTDl3FEe0+70W87JG7Q/I1r8Pw+Z6sShj3AUhZSNg +y4GhZdUOP7XUVHt2HHPp9rmESRPOnTXEPbrO3ZAmbtq6dpn4g73vc47MbGnBI+Yj1UdiNY+4FZ D8sdrAJVbvtHSAdz64hhZ0HtDg2G+p8SS6Jwr9+cI/FNVpn/SILwybr6VorjO60Ady+YcQ3Trgl etETbqTUys1yCZAxuB6Xqul3GwlH+9oYyyPzTrAL34D1/0qRZdAWN1ySADr7yDc0qolgbAsxoqp T/fQUSO9VxmZScgh+6W9eSZev2lPdtko+1eft9q0r1p7jYjYYfFHrsZqvj+e8G5e+slVT10OEES 0ouf+pIWbalu52/czJPd72AtJpim+neUbGTrr0TDz41lZsjxlqx4OVJZkkTOABjzvGFtoDLN6LG iYmiEOqn6QgR/LIZ3wEMfgPVv91IMfjB3dC6ySH02cuEUDztkJyX5kt4wFHfUvLMNOu8bPcigcm GF6bPktieqt16zVn8cCpoI7X8nh9+mO X-Received: by 2002:a05:622a:a0a:b0:51c:1c2c:a8bc with SMTP id d75a77b69052e-52d6483fbfbmr18441601cf.37.1786507990610; Tue, 11 Aug 2026 21:13:10 -0700 (PDT) Received: from smtpclient.apple ([2600:1016:b10f:b114:6c29:917c:9735:8846]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-52d61e017bdsm10172371cf.27.2026.08.11.21.13.09 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 11 Aug 2026 21:13:10 -0700 (PDT) Content-Type: text/plain; charset=us-ascii Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.600.51.1.1\)) Subject: Re: [PATCH] drm/i915/gem: Fix request use-after-free in active_engine() From: Shuangpeng In-Reply-To: Date: Wed, 12 Aug 2026 00:12:38 -0400 Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, jani.nikula@linux.intel.com, joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com, tursulin@ursulin.net, chris@chris-wilson.co.uk, stable@vger.kernel.org Content-Transfer-Encoding: quoted-printable Message-Id: <0602E2B1-4FF4-4496-916D-D4DB1C1B0830@gmail.com> References: <20260809173646.3292361-1-shuangpeng.kernel@gmail.com> To: Krzysztof Karas X-Mailer: Apple Mail (2.3864.600.51.1.1) Hi Krzysztof, > On Aug 11, 2026, at 05:37, Krzysztof Karas = wrote: >=20 > Hi Shuangpeng, >=20 > On 2026-08-09 at 13:36:46 -0400, Shuangpeng Bai wrote: >> active_engine() walks timeline->requests in reverse under RCU and = takes a >> temporary reference before inspecting each request. However, it drops = that >> reference in the loop body before list_for_each_entry_reverse() = advances >> the cursor. >>=20 >> Concurrent retirement can unlink the same request and drop its base >> reference while active_engine() holds the temporary reference. The = put in >> active_engine() may then be final, freeing or recycling the request = before >> the loop step reads rq->link.prev. SLAB_TYPESAFE_BY_RCU does not = defer that >> reuse. >>=20 >> Open-code the reverse walk and cache the previous request while the = current >> request is still referenced. The next request remains protected by >> i915_request_get_rcu() and validated against the timeline before use. >>=20 >> An i915 mock selftest forced retirement between the active check and = cursor >> advance. The vulnerable tree reached the final request release and >> kmem_cache_free(), while the fixed tree completed the same ordering = without >> accessing rq after the put. > What mock selftest are you referring to? >=20 Thanks for looking. The "i915 mock selftest" I referred to was a local temporary mock selftest. Because generic QEMU does not expose an Intel i915 PCI device, I used the mock engine/request infrastructure to validate only the ordering in active_engine(). In short, the test forces active_engine() to hold the temporary request reference while a worker on the other CPU retires the same request, so the subsequent i915_request_put(rq) in active_engine() becomes the final put. That was only meant to show that this interleaving is feasible. I can clarify that wording in v2. >>=20 >> Fixes: 3cfea8c97c93 ("drm/i915/gem: Hold request reference for = canceling an active context") >> Cc: stable@vger.kernel.org # v5.10+ >> Signed-off-by: Shuangpeng Bai >> --- >> drivers/gpu/drm/i915/gem/i915_gem_context.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >>=20 >> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c = b/drivers/gpu/drm/i915/gem/i915_gem_context.c >> index c58ffa5a8fa6..ff5c892a0176 100644 >> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c >> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c >> @@ -1361,7 +1361,7 @@ static bool __cancel_engine(struct = intel_engine_cs *engine) >> static struct intel_engine_cs *active_engine(struct intel_context = *ce) >> { >> struct intel_engine_cs *engine =3D NULL; >> - struct i915_request *rq; >> + struct i915_request *rq, *prev; >>=20 >> if (intel_context_has_inflight(ce)) >> return intel_context_inflight(ce); >> @@ -1375,7 +1375,8 @@ static struct intel_engine_cs = *active_engine(struct intel_context *ce) >> * (and onto a new timeline->requests list). >> */ >> rcu_read_lock(); >> - list_for_each_entry_reverse(rq, &ce->timeline->requests, link) { >> + rq =3D list_last_entry(&ce->timeline->requests, typeof(*rq), link); >> + while (!list_entry_is_head(rq, &ce->timeline->requests, link)) { >> bool found; >>=20 >> /* timeline is already completed upto this point? */ >> @@ -1387,9 +1388,14 @@ static struct intel_engine_cs = *active_engine(struct intel_context *ce) >> if (likely(rcu_access_pointer(rq->timeline) =3D=3D ce->timeline)) >> found =3D i915_request_active_engine(rq, &engine); >>=20 >> + /* Cache the cursor before the put, which may release rq. */ >> + if (!found) > You could skip this check here and unconditionally set "prev". > Its value is going to be used only once if found =3D=3D false anyway. >=20 Thanks for your comment. I'll simplify that by assigning prev unconditionally before i915_request_put(). Please let me know if this addresses your question. If it does, I'll send v2 with the wording clarified and prev assigned unconditionally before i915_request_put(). Thanks, Shuangpeng >> + prev =3D list_prev_entry(rq, link); >> i915_request_put(rq); >> if (found) >> break; >> + >> + rq =3D prev; >> } >> rcu_read_unlock(); >>=20 >>=20 >> base-commit: a59f57e2aa127c5354168d2ec4bac920df1be4f4 >> --=20 >> 2.43.0 >>=20 >=20 > --=20 > Best Regards, > Krzysztof