Re: [PATCH RFC 1/2] mm, madvise: simplify anon_name handling

[Vlastimil Babka <vbabka@suse.cz>]

On 6/23/25 18:56, Lorenzo Stoakes wrote:
> On Mon, Jun 23, 2025 at 04:59:50PM +0200, Vlastimil Babka wrote:
> I think we can actually avoid this altogether... So we could separate this into two functions:
> 
> tatic int madvise_update_vma_anon_name(struct madvise_behavior *madv_behavior)
> {
> 	struct vm_area_struct *vma = madv_behavior->vma;
> 	VMA_ITERATOR(vmi, madv_behavior->mm, range->start);
> 	struct madvise_behavior_range *range = &madv_behavior->range;
> 	struct anon_vma_name *anon_name = madv_behavior->anon_name;
> 
> 	if (anon_vma_name_eq(anon_vma_name(vma), anon_name))
> 		rturn 0;
> 
> 	vma = vma_modify_flags_name(&vmi, madv_behavior->prev, vma,
> 			range->start, range->end, vma->vm_flags, anon_name);
> 	if (IS_ERR(vma))
> 		return PTR_ERR(vma);
> 
> 	madv_behavior->vma = vma;
> 
> 	/* vm_flags is protected by the mmap_lock held in write mode. */
> 	vma_start_write(vma);
> 	return replace_anon_vma_name(vma, anon_name);
> }
> 
> /*
>  * Update the vm_flags and/or anon_name on region of a vma, splitting it or
>  * merging it as necessary. Must be called with mmap_lock held for writing.
>  */
> static int madvise_update_vma(vm_flags_t new_flags,
> 		struct madvise_behavior *madv_behavior)
> {
> 	struct vm_area_struct *vma = madv_behavior->vma;
> 	struct madvise_behavior_range *range = &madv_behavior->range;
> 	VMA_ITERATOR(vmi, madv_behavior->mm, range->start);
> 
> 	if (new_flags == vma->vm_flags)
> 		return 0;
> 
> 	vma = vma_modify_flags(&vmi, madv_behavior->prev, vma,
> 			range->start, range->end, new_flags);

Using vma_modify_flags() is a great suggestion to avoid passing the existing
vma->anon_name explicitly, thanks! I believe I can do that without
duplicating the whole madvise_update_vma() function and it doesn't look that
bad so I'll try going that way in v2. This also addresses Suren's concerns
as there will be no local variable pointing to the vma->anon_name that can
become a UAF again by future changes so we shouldn't need the warning
comments either.

Thanks!