Re: [PATCH] mm: do not assume file == vma->vm_file in compat_vma_mmap_prepare()

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Wed, Sep 03, 2025 at 03:52:21PM +0100, Lorenzo Stoakes wrote:
> On Tue, Sep 02, 2025 at 11:08:10AM -0400, Liam R. Howlett wrote:
> >
> > One nit below.

> > >  static inline void set_vma_from_desc(struct vm_area_struct *vma,
> > > -		struct vm_area_desc *desc)
> > > +		struct file *orig_file, struct vm_area_desc *desc)
> > >  {
> > >  	/*
> > >  	 * Since we're invoking .mmap_prepare() despite having a partially
> > > @@ -258,7 +258,13 @@ static inline void set_vma_from_desc(struct vm_area_struct *vma,
> > >
> > >  	/* Mutable fields. Populated with initial state. */
> > >  	vma->vm_pgoff = desc->pgoff;
> > > -	if (vma->vm_file != desc->file)
> > > +	/*
> > > +	 * The desc->file may not be the same as vma->vm_file, but if the
> > > +	 * f_op->mmap_prepare() handler is setting this parameter to something
> > > +	 * different, it indicates that it wishes the VMA to have its file
> > > +	 * assigned to this.
> > > +	 */
> > > +	if (orig_file != desc->file && vma->vm_file != desc->file)
> > >  		vma_set_file(vma, desc->file);
> >
> > So now we have to be sure both orig_file and vma->vm_file != desc->file
> > to set it?  This seems to make the function name less accurate.
>
> I'll update the comment accordingly.
>
> On this in general - In future an mmap_prepare() caller may wish to change
> the file to desc->file from vma->vm_file which currently won't work for a
> stacked file system.
>
> It's pretty niche and unlikely anybody does it, but if they do, since I am
> the one implementing all this I will adjust the descriptor to send a
> separate file parameter and adjust this code accordingly.
>
> Cheers, Lorenzo

Actually, let me also send vma->vm_file in desc and make that what gets updated,
and make struct file read-only.

That way we solve this problem a lot more neatly. No users are currently setting
desc->file so it's safe to do right now.