Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] dma-fence: @release deprecation advice causes UAF/SLAB corruption for embedded fences - [Low] dma-fence: Missing @wait callback alternative documentation --
commit 491d04795e6f38b069b06a9246cd098fdc0a8b55 Author: Philipp Stanner <[email protected]> dma-buf/dma-fence: Mark two callbacks as deprecated This commit marks the ops->wait() and ops->release() callbacks in dma_fence_ops as deprecated. It updates the documentation to advise against implementing them, aiming to move towards a design that supports driver unloading in all circumstances. > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h > index dd07d128adc3b..388c07d11e708 100644 > --- a/include/linux/dma-fence.h > +++ b/include/linux/dma-fence.h > @@ -220,6 +220,8 @@ struct dma_fence_ops { > /** > * @wait: > * > + * DEPRECATED! > + * > * Custom wait implementation, defaults to dma_fence_default_wait() if > * not set. [Severity: Low] The commit message indicates it will "document what users should do instead" for both ops->release() and ops->wait(). However, while instructions are provided for @release, the alternative implementation details for @wait seem to be missing here. Could documentation be added to explain what developers should use instead of the @wait callback? [ ... ] > @@ -243,6 +245,8 @@ struct dma_fence_ops { > /** > * @release: > * > + * DEPRECATED! > + * > * Called on destruction of fence to release additional resources. [ ... ] > * If the callback is implemented the memory backing the dma_fence > * object must be freed RCU safe. > + * > + * Deprecated because it prevents the producer of a fence from > + * unloading. No new users must be implemented. Parties with a > + * hypothetical need for this callback can instead simply and directly > + * perform their custom release operations one RCU grace period after > + * they have signaled the fence. > */ > void (*release)(struct dma_fence *fence); [Severity: High] Does this advice correctly account for embedded dma_fence objects? If a driver embeds struct dma_fence inside a driver-specific container, removes its .release callback to comply with this documentation, and frees the container one RCU grace period after signaling, could this cause a use-after-free for consumers still holding a long-term reference like dma_resv? Alternatively, if a driver simply omits .release to follow the "no new users" guidance without changing its allocation strategy, the core will execute dma_fence_free() on the final dma_fence_put(). This fallback executes kfree_rcu() directly on the fence pointer. If the fence is embedded and not placed at offset 0, passing an interior pointer to kfree_rcu() can corrupt the SLAB allocator. Would it make sense to update the documentation to explicitly warn that omitting .release requires the dma_fence to either be a standalone allocation or placed at offset 0 within its container? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
