Unsafe code in VmaNew's methods assumes that the type has the same
layout as the inner `bindings::vm_area_struct`. This is not guaranteed by
the default struct representation in Rust, but requires specifying the
`transparent` representation.

Fixes: dcb81aeab406e ("mm: rust: add VmaNew for f_ops->mmap()")
Signed-off-by: Baptiste Lepers <baptiste.lepers@gmail.com>
---
 rust/kernel/mm/virt.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rust/kernel/mm/virt.rs b/rust/kernel/mm/virt.rs
index 6086ca981b06..a1bfa4e19293 100644
--- a/rust/kernel/mm/virt.rs
+++ b/rust/kernel/mm/virt.rs
@@ -209,6 +209,7 @@ pub fn vm_insert_page(&self, address: usize, page: &Page) -> Result {
 ///
 /// For the duration of 'a, the referenced vma must be undergoing initialization in an
 /// `f_ops->mmap()` hook.
+#[repr(transparent)]
 pub struct VmaNew {
     vma: VmaRef,
 }
-- 
2.43.0

On Tue, 12 Aug 2025 15:26:56 +0200 Baptiste Lepers <baptiste.lepers@gmail.com> wrote:

> Unsafe code in VmaNew's methods assumes that the type has the same
> layout as the inner `bindings::vm_area_struct`. This is not guaranteed by
> the default struct representation in Rust, but requires specifying the
> `transparent` representation.
> 
> ...
>
> +++ b/rust/kernel/mm/virt.rs
> @@ -209,6 +209,7 @@ pub fn vm_insert_page(&self, address: usize, page: &Page) -> Result {
>  ///
>  /// For the duration of 'a, the referenced vma must be undergoing initialization in an
>  /// `f_ops->mmap()` hook.
> +#[repr(transparent)]
>  pub struct VmaNew {
>      vma: VmaRef,
>  }

Alice suggests that I add a cc:stable to this.  But I see nothing in
the changelog which explains why we're proposing a backport.

So please send us a description of the userspace-visible runtime
impact of this flaw and I'll paste it into the changelog, thanks.

On Thu, Aug 21, 2025 at 1:29 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Tue, 12 Aug 2025 15:26:56 +0200 Baptiste Lepers <baptiste.lepers@gmail.com> wrote:
>
> > Unsafe code in VmaNew's methods assumes that the type has the same
> > layout as the inner `bindings::vm_area_struct`. This is not guaranteed by
> > the default struct representation in Rust, but requires specifying the
> > `transparent` representation.
> >
> > ...
> >
> > +++ b/rust/kernel/mm/virt.rs
> > @@ -209,6 +209,7 @@ pub fn vm_insert_page(&self, address: usize, page: &Page) -> Result {
> >  ///
> >  /// For the duration of 'a, the referenced vma must be undergoing initialization in an
> >  /// `f_ops->mmap()` hook.
> > +#[repr(transparent)]
> >  pub struct VmaNew {
> >      vma: VmaRef,
> >  }
>
> Alice suggests that I add a cc:stable to this.  But I see nothing in
> the changelog which explains why we're proposing a backport.
>
> So please send us a description of the userspace-visible runtime
> impact of this flaw and I'll paste it into the changelog, thanks.

I don't think it has any userspace-visible runtime impact. But I've
seen many things get backported when they are incorrect even if it
works in practice, so that is why I suggested to backport it anyway.

The annotation makes it so that VmaNew is guaranteed to have the same
layout and ABI as struct vm_area_struct, which is required for
correctness. Without the annotation, rustc doesn't *guarantee* that
the layout/ABI is identical, but in this case, they are identical in
practice even if the annotation is missing.

Alice

On Thu, Aug 21, 2025 at 11:45 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> seen many things get backported when they are incorrect even if it
> works in practice, so that is why I suggested to backport it anyway.

To clarify: "incorrect" here means in the stable kernel, i.e. not the
backported patch.

Andrew: in the past, I was quite conservative in what I would mark as
Cc: stable for Rust, but after discussing a few past cases with the
current stable kernel team and/or being requested to add the tag, I am
nowadays way more optimistic in tagging. Some backports would not pass
the stated rules in principle, but they still wanted them.

For instance, I tag even Clippy cleanups (so far, since it seems
doable to keep it clean with the amount of code we have).
Nevertheless, I still skip the tag when I really feel there is no
point, and let their scripts pick them up if they really, really want
them.

For this particular case, since we support several compiler versions
nowadays, I would have just tagged it if it had went through my
branch. There is also little risk whether it gets backported or not.

I hope that gives some context.

Cheers,
Miguel

On Tue, Aug 12, 2025 at 03:26:56PM +0200, Baptiste Lepers wrote:
> Unsafe code in VmaNew's methods assumes that the type has the same
> layout as the inner `bindings::vm_area_struct`. This is not guaranteed by
> the default struct representation in Rust, but requires specifying the
> `transparent` representation.
> 
> Fixes: dcb81aeab406e ("mm: rust: add VmaNew for f_ops->mmap()")
> Signed-off-by: Baptiste Lepers <baptiste.lepers@gmail.com>
> ---
>  rust/kernel/mm/virt.rs | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/rust/kernel/mm/virt.rs b/rust/kernel/mm/virt.rs
> index 6086ca981b06..a1bfa4e19293 100644
> --- a/rust/kernel/mm/virt.rs
> +++ b/rust/kernel/mm/virt.rs
> @@ -209,6 +209,7 @@ pub fn vm_insert_page(&self, address: usize, page: &Page) -> Result {
>  ///
>  /// For the duration of 'a, the referenced vma must be undergoing initialization in an
>  /// `f_ops->mmap()` hook.
> +#[repr(transparent)]
>  pub struct VmaNew {
>      vma: VmaRef,
>  }
> -- 
> 2.43.0
> 
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You have marked a patch with a "Fixes:" tag for a commit that is in an
  older released kernel, yet you do not have a cc: stable line in the
  signed-off-by area at all, which means that the patch will not be
  applied to any older kernel releases.  To properly fix this, please
  follow the documented rules in the
  Documentation/process/stable-kernel-rules.rst file for how to resolve
  this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

On Tue, Aug 12, 2025 at 3:29 PM Baptiste Lepers
<baptiste.lepers@gmail.com> wrote:
>
> Unsafe code in VmaNew's methods assumes that the type has the same
> layout as the inner `bindings::vm_area_struct`. This is not guaranteed by
> the default struct representation in Rust, but requires specifying the
> `transparent` representation.

Right. It's the case that the layout matches in practice, but we
should use repr(transparent) so that rustc guarantees it. Thanks!

> Fixes: dcb81aeab406e ("mm: rust: add VmaNew for f_ops->mmap()")
> Signed-off-by: Baptiste Lepers <baptiste.lepers@gmail.com>

Andrew, can you pick this up? Thanks!

Reviewed-by: Alice Ryhl <aliceryhl@google.com>