On Wed, Jan 15, 2025 at 4:15 AM Suren Baghdasaryan <surenb@xxxxxxxxxx> wrote:
>
> On Tue, Jan 14, 2025 at 6:27 PM Wei Yang <richard.weiyang@xxxxxxxxx> wrote:
> >
> > On Fri, Jan 10, 2025 at 08:26:03PM -0800, Suren Baghdasaryan wrote:
> >
> > >diff --git a/kernel/fork.c b/kernel/fork.c
> > >index 9d9275783cf8..151b40627c14 100644
> > >--- a/kernel/fork.c
> > >+++ b/kernel/fork.c
> > >@@ -449,6 +449,42 @@ struct vm_area_struct *vm_area_alloc(struct mm_struct *mm)
> > > return vma;
> > > }
> > >
> > >+static void vm_area_init_from(const struct vm_area_struct *src,
> > >+ struct vm_area_struct *dest)
> > >+{
[snip]
> > Would this be difficult to maintain? We should make sure not miss or overwrite
> > anything.
>
> Yeah, it is less maintainable than a simple memcpy() but I did not
> find a better alternative. I added a warning above the struct
> vm_area_struct definition to update this function every time we change
> that structure. Not sure if there is anything else I can do to help
> with this.
>
Bare minimum this could have a BUILD_BUG_ON in below the func for the
known-covered size. But it would have to be conditional on arch and
some macros, somewhat nasty.
KASAN or KMSAN (I don't remember which) can be used to find missing
copies. To that end the target struct could be marked as fully
uninitialized before copy and have a full read performed from it
afterwards -- guaranteed to trip over any field which any field not
explicitly covered (including padding though). I don't know what magic
macros can be used to do in Linux, I am saying the support to get this
result is there. I understand most people don't use this, but this
still should be enough to trip over buggy patches in -next.
Finally, the struct could have macros delimiting copy/non-copy
sections (with macros expanding to field names), for illustrative
purposes:
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 332cee285662..25063a3970c8 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -677,6 +677,7 @@ struct vma_numab_state {
* getting a stable reference.
*/
struct vm_area_struct {
+#define vma_start_copy0 vm_rcu
/* The first cache line has the info for VMA tree walking. */
union {
@@ -731,6 +732,7 @@ struct vm_area_struct {
/* Unstable RCU readers are allowed to read this. */
struct vma_lock *vm_lock;
#endif
+#define vma_end_copy1 vm_lock
/*
* For areas with an address space and backing store,
then you would do everything with a series of calls
however, the __randomize_layout annotation whacks that idea (maybe it
can be retired?)
--
Mateusz Guzik <mjguzik gmail.com>
