Re: [PATCH 4/8] migrate_pages: split unmap_and_move() to _unmap() and _move()

[Kees Cook]

On Thu, Jan 05, 2023 at 11:26:55AM -0700, Nathan Chancellor wrote:
> Hi Ying,
> 
> On Tue, Dec 27, 2022 at 08:28:55AM +0800, Huang Ying wrote:
> > This is a preparation patch to batch the folio unmapping and moving.
> > 
> > In this patch, unmap_and_move() is split to migrate_folio_unmap() and
> > migrate_folio_move().  So, we can batch _unmap() and _move() in
> > different loops later.  To pass some information between unmap and
> > move, the original unused dst->mapping and dst->private are used.
> > 
> > Signed-off-by: "Huang, Ying" <ying.huang@xxxxxxxxx>
> > Cc: Zi Yan <ziy@xxxxxxxxxx>
> > Cc: Yang Shi <shy828301@xxxxxxxxx>
> > Cc: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
> > Cc: Oscar Salvador <osalvador@xxxxxxx>
> > Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> > Cc: Bharata B Rao <bharata@xxxxxxx>
> > Cc: Alistair Popple <apopple@xxxxxxxxxx>
> > Cc: haoxin <xhao@xxxxxxxxxxxxxxxxx>
> > ---
> >  include/linux/migrate.h |   1 +
> >  mm/migrate.c            | 162 +++++++++++++++++++++++++++++-----------
> >  2 files changed, 121 insertions(+), 42 deletions(-)
> > 
> > diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> > index 3ef77f52a4f0..7376074f2e1e 100644
> > --- a/include/linux/migrate.h
> > +++ b/include/linux/migrate.h
> > @@ -18,6 +18,7 @@ struct migration_target_control;
> >   * - zero on page migration success;
> >   */
> >  #define MIGRATEPAGE_SUCCESS		0
> > +#define MIGRATEPAGE_UNMAP		1
> >  
> >  /**
> >   * struct movable_operations - Driver page migration
> > diff --git a/mm/migrate.c b/mm/migrate.c
> > index 97ea0737ab2b..e2383b430932 100644
> > --- a/mm/migrate.c
> > +++ b/mm/migrate.c
> > @@ -1009,11 +1009,29 @@ static int move_to_new_folio(struct folio *dst, struct folio *src,
> >  	return rc;
> >  }
> >  
> > -static int __unmap_and_move(struct folio *src, struct folio *dst,
> > +static void __migrate_folio_record(struct folio *dst,
> > +				   unsigned long page_was_mapped,
> > +				   struct anon_vma *anon_vma)
> > +{
> > +	dst->mapping = (struct address_space *)anon_vma;
> > +	dst->private = (void *)page_was_mapped;
> > +}
> > +
> > +static void __migrate_folio_extract(struct folio *dst,
> > +				   int *page_was_mappedp,
> > +				   struct anon_vma **anon_vmap)
> > +{
> > +	*anon_vmap = (struct anon_vma *)dst->mapping;
> > +	*page_was_mappedp = (unsigned long)dst->private;
> > +	dst->mapping = NULL;
> > +	dst->private = NULL;
> > +}
> 
> This patch as commit 42871c600cad ("migrate_pages: split
> unmap_and_move() to _unmap() and _move()") in next-20230105 causes the
> following error with clang when CONFIG_RANDSTRUCT is enabled, which is
> the case with allmodconfig:
> 
>   ../mm/migrate.c:1041:15: error: casting from randomized structure pointer type 'struct address_space *' to 'struct anon_vma *'
>           *anon_vmap = (struct anon_vma *)dst->mapping;
>                        ^
>   1 error generated.
> 
> With GCC, there is only a note:
> 
>   ../mm/migrate.c: In function '__migrate_folio_extract':
>   ../mm/migrate.c:1041:20: note: randstruct: casting between randomized structure pointer types (ssa): 'struct anon_vma' and 'struct address_space'
> 
>    1041 |         *anon_vmap = (struct anon_vma *)dst->mapping;
>         |         ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Kees has done fixes for warnings and errors like this in the past (I
> just ran
> 
>   $ git log -p --grep='randomized structure pointer type'
> 
> to find them) but I did not see any that would seem appropriate here
> hence just the report :)

If this struct is literally just a scratch space and the original struct
layout doesn't matter, it may be possible to silence this cast by using
"(void *)" instead of the explicit struct type pointer.

-- 
Kees Cook