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

[Oleg Nesterov]

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;
		
		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.