linux-kernel - Re: [RFC PATCH 3/3] drm/sched: Prevent adding dependencies to an armed job

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d4b7c1635aee89c49416e26ef482238f4ee1fafe.camel@redhat.com>
Date: Tue, 28 Oct 2025 10:27:06 +0100
From: Philipp Stanner <pstanner@...hat.com>
To: Matthew Brost <matthew.brost@...el.com>
Cc: intel-xe@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org, jiangshanlai@...il.com, tj@...nel.org, 
	simona.vetter@...ll.ch, christian.koenig@....com, dakr@...nel.org
Subject: Re: [RFC PATCH 3/3] drm/sched: Prevent adding dependencies to an
 armed job

On Mon, 2025-10-27 at 09:56 -0700, Matthew Brost wrote:
> On Mon, Oct 27, 2025 at 12:13:58PM +0100, Philipp Stanner wrote:
> > I've got a kernel.org addr by now by the way
> > 
> > On Tue, 2025-10-21 at 14:39 -0700, Matthew Brost wrote:
> > > According to the DMA scheduler documentation, once a job is armed, it
> > > must be pushed. Drivers should avoid calling the failing code path that
> > > attempts to add dependencies after a job has been armed.
> > > 
> > 
> > Why is that a "failing code path"?
> > 
> 
> I noticed this after I sent - it should something like:
> 
> 'avoid calling a possible failing code path, which allocates memory.'
> 
> I can make this a bit more clear.
> 
> > The issue with adding callbacks is that adding them to an already
> > signaled fence is a bad idea. I'm not sure if it's illegal, though.
> > dma_fence_add_cb() merely returns an error then, but the driver could
> > in priniciple then execute its cb code itself.
> > 
> > And even if we agree that this is a hard rule that must be followed,
> > then drm_sched_job_arm() *might* not be the right place, because just
> > because a job is armed doesn't mean that its fence is about to get
> > signaled. drm_sched_entity_push_job() would be the critical place.
> > 
> 
> I think this break our rule once arm is called, push must be called as
> adding dependencies can possibly fail. This rule is called out in your
> documentation patch too. I've seen 2 driver posted in the past year add
> dependencies after arming, so I figured lets catch this misuse in the
> scheduler.

We can establish that as a rule, I'm OK with that.

P.