Re: [PATCH] prctl: Get private anonymous memory region name

[Rong Tao <rtoax@foxmail.com>]

On 11/25/23 23:56, Oleg Nesterov wrote:
> On 11/25, Rong Tao wrote:
>> +static int prctl_get_vma(unsigned long opt, unsigned long addr,
>> +			 unsigned long buf, unsigned long arg)
>> +{
>> +	struct mm_struct *mm = current->mm;
>> +	const char __user *u_buf;
>> +	int error;
>> +
>> +	switch (opt) {
>> +	case PR_GET_VMA_ANON_NAME:
>> +		const struct anon_vma_name *anon_name = NULL;
>> +
>> +		u_buf = (const char __user *)buf;
>> +		error = 0;
>> +
>> +		mmap_read_lock(mm);
>> +		anon_name = madvise_get_anon_name(mm, addr);
>> +		if (!anon_name) {
>> +			mmap_read_unlock(mm);
>> +			error = -EFAULT;
> may be another error code makes sense to distinguish this case from
> the copy_to_user() failure?
>
>> +			break;
>> +		}
>> +
>> +		if (copy_to_user((char __user *)u_buf, anon_name->name,
>> +				 strlen(anon_name->name) + 1))
>> +			error = -EFAULT;
> and I guess you can simplify this code a bit,
>
> 		anon_name = madvise_get_anon_name(...);
> 		if (!anon_name || copy_to_user(...))
> 			error = -EFAULT;
Thanks a lot, I just modify the code, please review.
> 		
> 		mmap_read_unlock(mm);
> 		anon_vma_name_put(anon_name); // safe if anon_name == NULL;
>
>> +const struct anon_vma_name *madvise_get_anon_name(struct mm_struct *mm,
>> +						  unsigned long start)
>> +{
>> +	struct vm_area_struct *vma;
>> +	struct anon_vma_name *anon_name;
>> +
>> +	vma = find_vma(mm, start);
>> +	if (vma) {
>> +		anon_name = anon_vma_name(vma);
>> +		if (anon_name) {
>> +			anon_vma_name_get(anon_name);
>> +			return anon_name;
>> +		}
>> +	}
>> +
>> +	return NULL;
> Again, afaics this can be simplified,
>
> 	struct anon_vma_name *anon_name = NULL;
>
> 	vma = find_vma(mm, start);
> 	if (vma) {
> 		anon_name = anon_vma_name(vma);
> 		anon_vma_name_get(anon_name);
> 	}
>
> 	return anon_name;
>
> Oleg.
Thanks again.

Rong Tao

>