linux-kernel - Re: [PATCH] Partially revert "rust: drm: gem: Implement AlwaysRefCounted for all gem objects automatically"

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] [day] [month] [year] [list]

Message-Id: <DBNUJUSYG465.7YE1YER8B9K@kernel.org>
Date: Mon, 28 Jul 2025 18:59:21 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Lyude Paul" <lyude@...hat.com>
Cc: "Daniel Almeida" <daniel.almeida@...labora.com>,
 <nouveau@...ts.freedesktop.org>, <dri-devel@...ts.freedesktop.org>,
 <rust-for-linux@...r.kernel.org>, "David Airlie" <airlied@...il.com>,
 "Simona Vetter" <simona@...ll.ch>, "Maarten Lankhorst"
 <maarten.lankhorst@...ux.intel.com>, "Maxime Ripard" <mripard@...nel.org>,
 "Thomas Zimmermann" <tzimmermann@...e.de>, "Miguel Ojeda"
 <ojeda@...nel.org>, "Alex Gaynor" <alex.gaynor@...il.com>, "Boqun Feng"
 <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Benno Lossin"
 <lossin@...nel.org>, "Andreas Hindborg" <a.hindborg@...nel.org>, "Alice
 Ryhl" <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>, "Asahi
 Lina" <lina+kernel@...hilina.net>, "Alyssa Rosenzweig"
 <alyssa@...enzweig.io>, "open list" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Partially revert "rust: drm: gem: Implement
 AlwaysRefCounted for all gem objects automatically"

On Fri Jul 25, 2025 at 9:41 PM CEST, Lyude Paul wrote:
> a-ha, ok. I made a mistake here with misremembering where the compilation
> issue I saw here really was.
>
> It's not that multiple gem object implementations are triggering it, it's that
> it immediately breaks compilation if any other type tries to do a blanket
> implementation with AlwaysRefCounted like this.
>
> Here's a properly compiling example with rvkms:
>
> https://gitlab.freedesktop.org/lyudess/linux/-/commits/rvkms-slim
>
> This builds fine because IntoGEMObject is the only one with a blanket
> implementation of AlwaysRefCounted, and we implement AlwaysRefCounted using a
> macro for refcounted Kms objects.
>
> But if we apply this patch which adds the second blanket impl:
>
> https://gitlab.freedesktop.org/lyudess/linux/-/commit/ec094d4fc209a7122b00168e7293f365fe7fc16c
>
> Then compilation fails:
>
>    ➜  nouveau-gsp git:(rvkms-slim) ✗ nice make -j20
>      DESCEND objtool
>      DESCEND bpf/resolve_btfids
>      CALL    scripts/checksyscalls.sh
>      INSTALL libsubcmd_headers
>      INSTALL libsubcmd_headers
>      RUSTC L rust/kernel.o
>    warning: unused import: `pin_init`
>      --> rust/kernel/drm/driver.rs:18:5
>       |
>    18 | use pin_init;
>       |     ^^^^^^^^
>       |
>       = note: `#[warn(unused_imports)]` on by default
>    
>    warning: unused import: `prelude::*`
>     --> rust/kernel/drm/kms/modes.rs:4:13
>      |
>    4 | use crate::{prelude::*, types::Opaque};
>      |             ^^^^^^^^^^
>    
>    error[E0119]: conflicting implementations of trait `types::AlwaysRefCounted`
>       --> rust/kernel/drm/kms.rs:504:1
>        |
>    504 | unsafe impl<T: RcModeObject> AlwaysRefCounted for T {
>        | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation
>        |
>       ::: rust/kernel/drm/gem/mod.rs:97:1
>        |
>    97  | unsafe impl<T: IntoGEMObject> AlwaysRefCounted for T {
>        | ---------------------------------------------------- first implementation here
>    
>    warning: unused import: `Sealed`
>     --> rust/kernel/drm/kms/vblank.rs:7:44
>      |
>    7 | use super::{crtc::*, ModeObject, modes::*, Sealed};
>      |                                            ^^^^^^
>    
>    error: aborting due to 1 previous error; 3 warnings emitted
>    
>    For more information about this error, try `rustc --explain E0119`.
>    make[2]: *** [rust/Makefile:538: rust/kernel.o] Error 1
>    make[1]: *** [/home/lyudess/Projects/linux/worktrees/nouveau-gsp/Makefile:1280: prepare] Error 2
>    make: *** [Makefile:248: __sub-make] Error 2
>
> This is definitely part of the reason I didn't notice this problem until later
> too. My understanding is that this is a result of rust's orphan rule, which
> basically just disallows trait impls where it would be ambiguous which impl
> applies to a specific type. Here, the issue is that there's nothing stopping a
> type from implementing both RcModeObject and IntoGEMObject.

Yeah, this is pretty annoying. I don't think it's related to the orphan rule
though; see also the example in [1].

I think in this case we should just keep the generic
impl<T: IntoGEMObject> AlwaysRefCounted for T and not introduce the blanket one
for T: RcModeObject.

In theory it doesn't matter which one to drop, but I'd rather avoid the revert
and I think there's no reason for both to have the less nice macro solution.

[1] https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=23593da0e5e0ca0d9d2aa654e0c9bde6