linux-kernel - Re: [PATCH] mm: do not assume file == vma->vm_file in compat_vma_mmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <29d99c91-815d-4f43-9c05-c62b76e8c7f8@lucifer.local>
Date: Wed, 3 Sep 2025 16:08:47 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: "Liam R. Howlett" <Liam.Howlett@...cle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
        David Hildenbrand <david@...hat.com>, Vlastimil Babka <vbabka@...e.cz>,
        Mike Rapoport <rppt@...nel.org>,
        Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
        Jann Horn <jannh@...gle.com>, Pedro Falcato <pfalcato@...e.de>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org
Subject: Re: [PATCH] mm: do not assume file == vma->vm_file in
 compat_vma_mmap_prepare()

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.