Re: [PATCH v2 2/2] rust: debugfs: avoid transmuting FileOps

[Tamir Duberstein]

On Tue, May 26, 2026 at 4:18 PM Matthew Maurer <mmaurer@xxxxxxxxxx> wrote:
>
> On Tue, May 26, 2026 at 11:08 AM Tamir Duberstein <tamird@xxxxxxxxxx> wrote:
> > `FileOps` has the default Rust representation, which does not guarantee
> > the same layout for different generic instantiations [1]. Thus the
> > reference created by `adapt()` is not justified.
>
> This is correct, though I'll note for the sake of anyone concerned
> with an existing kernel that this can only break with
> `-Zrandomize-fields` in the current compiler - the layout engine only
> examines layout data + an optional random seed when that flag is set,
> so swapping out a ZST, no-alignment-requirement field for another ZST,
> no-alignment-requirement field will never result in a mismatch unless
> you are actively randomizing fields.
>
> > The callback operations do need to access private data through one of
> > the transparent adapter types. Express that relationship directly: make
> > `Adapter<D>` the unsafe proof that operations implemented for an adapter
> > may access stored `D`, and instantiate the corresponding operations
> > tables as `FileOps<D>`. The adapter implementations are justified by
> > their transparent representations [2].
>
> I think your approach here may need to be adjusted because:
> 1. `Adapter` requires additional knowledge at the trait usage site
> (that the invariants of the target are upheld - specifically
> type-inhabitedness).
> 2. You now access `FILE_OPS` directly off the type, rather than off a
> value, so you no longer have any proof that types are inhabited that
> you require.
>
> You might be able to rework this so that you can track inhabitedness
> properly, but maybe a simpler resolution might work - what if we just
> add `#[repr(C)]` onto `FileOps`? The addition of an unaligned ZST
> field to a `#[repr(C)]` struct should never change its representation.

Thanks for explaining this. I had missed that receiving a callback
reference at registration does not help justify reconstructing a value
of its type later, once the operations table can be obtained without
that reference.

I tried to follow that observation through the existing design.
`#[repr(C)]` appears to address the transmute of `FileOps`, but I think
the callback adapters would still discard the callback references and
later reconstruct `F` and `W` through `materialize_zst()`. So I have
been working on a more intrusive v3 that removes both mechanisms.

The approach I have currently is to use debugfs auxiliary data to retain
the actual callback references. `FileOps` becomes parameterized by both
the backing data type and the auxiliary data type, and callback
operations recover their callback value through `debugfs_get_aux()`.

This is not API-neutral. Debugfs exposes one auxiliary pointer, so a
read-write callback file needs one object containing both retained
callback references. In the version I have now,
`read_write_callback_file()` takes a `&'static ReadWriteCallbacks<F, W>`
instead of two separate callback references. There are no in-tree users
of that API, but it is still a visible change and may be the wrong
tradeoff.

Does this seem like a reasonable direction?

Thanks,
Tamir