Re: [PATCH v2 00/19] `zerocopy` support

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Tue, Jun 09, 2026 at 12:43:03PM +0000, Alice Ryhl wrote:
> On Tue, Jun 09, 2026 at 12:21:21PM +0000, Alice Ryhl wrote:
> > On Mon, Jun 08, 2026 at 04:14:19PM +0200, Miguel Ojeda wrote:
> > > This patch series introduces support for `zerocopy`:
> > >
> > >     Fast, safe, compile error. Pick two.
> > >
> > >     Zerocopy makes zero-cost memory manipulation effortless. We write
> > >     `unsafe` so you don't have to.
> > 
> > I tried applying this and using it with Binder. I ran into one
> > challenge, which is this uapi struct:
> > 
> > struct binder_transaction_data {
> > 	/* The first two are only used for bcTRANSACTION and brTRANSACTION,
> > 	 * identifying the target and contents of the transaction.
> > 	 */
> > 	union {
> > 		/* target descriptor of command transaction */
> > 		__u32	handle;
> > 		/* target descriptor of return transaction */
> > 		binder_uintptr_t ptr;
> > 	} target;
> > 	binder_uintptr_t	cookie;	/* target object cookie */
> > 	...
> > }
> > 
> > The problem is that when the union contains a handle, there are 4 bytes
> > of padding in the union. Currently Rust Binder handles this by wrapping
> > the uapi struct in MaybeUninit and using MaybeUninit::zeroed() to
> > construct it, ensuring that even if padding is present, it is zeroed.
> > 
> > However, this trick relies on unsafely implementing AsBytes for
> > BinderTransactionData with the safety comment being that the MaybeUninit
> > actually always contains initialized data.
> > 
> > To translate this to zerocopy, I'd have to do this:
> > 
> > unsafe impl zerocopy::IntoBytes for $newname {
> >     fn only_derive_is_allowed_to_implement_this_trait() {}
> > }
> > 
> > One fix could be to update the uapi header by explicitly adding the
> > padding, but that's kind of awkward for a union like this, since I'd
> > have to do it like this with an extra struct:
> > 
> > 	union {
> > 		/* target descriptor of command transaction */
> > 		struct {
> > 			__u32	handle;
> > 			__u32	_pad;
> > 		};
> > 		/* target descriptor of return transaction */
> > 		binder_uintptr_t ptr;
> > 	} target;
> > 
> > It's not clear to me if changing the uapi headers like this is even
> > allowed to begin with. It's a somewhat non-trivial change.
> 
> Hey Greg,
> Do you have any input on this from the C side?
> 
> For context, zerocopy is a tool that helps convert raw bytes into
> structs and vice-versa. When I tried using zerocopy with Rust Binder to
> see if it helps there, I found that zerocopy is flagging that Rust
> Binder copied a uapi struct containing padding into userspace, which is
> of course dangerous since padding on the kernel stack may contain data
> we don't want to leak to userspace.

A UAPI structure should not have padding, and if it does, it must be
zeroed out, as you say.  So in C we normally manually fix this up.

> Now, there's not actually a problem today because I'm using another
> strategy to ensure that the padding is zeroed when copying the output of
> the ioctl to userspace, but ideally I'd like to switch over to using
> zerocopy.
> 
> How is this kind of thing usually handled on the C side? As far as I can
> tell, C Binder handles it by just being extra careful like this:
> 
> 	fp->binder = 0;
> 	fp->handle = rdata.desc;

Yes, we have to almost always manually do this.  Sometimes we use memset
on the structure first before copying, or rely on a foo = {}; to do it
for us (but I never remember if the compiler will zero out the holes...)

> where fp->binder and fp->handle are fields of the same union, with
> fp->binder being 8 bytes and fp->handle being 4 bytes. The first
> assignment ensures that the extra 4 bytes in the union are zeroed.

Yes, that's why we manually do this :(

thanks,

greg k-h