On Tue, Jan 14, 2025 at 7:15 PM Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> wrote: > On Tue, Jan 14, 2025 at 08:53:01AM -0800, Yang Shi wrote: > > On 1/14/25 4:05 AM, Lorenzo Stoakes wrote: > > > On Mon, Jan 13, 2025 at 02:30:33PM -0800, Yang Shi wrote: > > > > + fput(vma->vm_file); > > > > + vma->vm_file = NULL; > > > > + vma->vm_pgoff = vma->vm_start >> PAGE_SHIFT; > > This is just not permitted. We maintain mmap state which contains the file > and pgoff state which gets threaded through the mapping operation, and > simply do not expect you to change these fields. > > In future we will assert on this or preferably, restrict users to only > changing VMA flags, the private field and vm_ops. > > > > Hmm, this might have been mremap()'d _potentially_ though? And then now > > > this will be wrong? But then we'd have no way of tracking it correctly... > > > > I'm not quite familiar with the subtle details and corner cases of > > meremap(). But mmap_zero() should be called by mmap(), so the VMA has not > > been visible to user yet at this point IIUC. How come mremap() could move > > it? > > Ah OK, in that case fine on that front. > > But you are not permitted to touch this field (we need to enforce this...) Sidenote: I think the GPU DRM subsystem relies on changing pgoff in some of their mmap handlers; maybe talk to them about this if you haven't already. See for example drm_gem_prime_mmap() and dma_buf_mmap().