Re: [PATCH 00/10] perf/uprobe: Optimize uprobes

[Peter Zijlstra <peterz@infradead.org>]

On Sat, Jul 27, 2024 at 04:45:53AM +0100, Matthew Wilcox wrote:

> Hum.  What if we added SLAB_TYPESAFE_BY_RCU to files_cachep?  That way
> we could do:
> 
> 	inode = NULL;
> 	rcu_read_lock();
> 	vma = find_vma(mm, address);
> 	if (!vma)
> 		goto unlock;
> 	file = READ_ONCE(vma->vm_file);
> 	if (!file)
> 		goto unlock;
> 	inode = file->f_inode;
> 	if (file != READ_ONCE(vma->vm_file))
> 		inode = NULL;

remove_vma() does not clear vm_file, nor do I think we ever re-assign
this field after it is set on creation.

That is, I'm struggling to see what this would do. AFAICT this can still
happen:

	rcu_read_lock();
	vma = find_vma();
					remove_vma()
					  fput(vma->vm_file);
								dup_fd)
								  newf = kmem_cache_alloc(...)
								  newf->f_inode = blah

	file = READ_ONCE(vma->vm_file);
	inode = file->f_inode; // blah
	if (file != READ_ONCE(vma->vm_file)) // still match


> unlock:
> 	rcu_read_unlock();
> 
> 	if (inode)
> 		return inode;
> 	mmap_read_lock();
> 	vma = find_vma(mm, address);
> 	...
> 
> I think this would be safe because 'vma' will not be reused while we
> hold the read lock, and while 'file' might be reused, whatever f_inode
> points to won't be used if vm_file is no longer what it once was.


Also, we need vaddr_to_offset() which needs additional serialization
against vma_lock.