Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Date:   Sat, 25 Nov 2023 16:56:51 +0100
From:   Oleg Nesterov <oleg@redhat.com>
To:     Rong Tao <rtoax@foxmail.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Florent Revest <revest@chromium.org>,
        Kees Cook <keescook@chromium.org>,
        David Hildenbrand <david@redhat.com>,
        Stefan Roesch <shr@devkernel.io>,
        Andy Chiu <andy.chiu@sifive.com>,
        Josh Triplett <josh@joshtriplett.org>,
        Joey Gouly <joey.gouly@arm.com>,
        Miguel Ojeda <ojeda@kernel.org>,
        Ondrej Mosnacek <omosnace@redhat.com>,
        "open list:MEMORY MANAGEMENT" <linux-mm@kvack.org>,
        open list <linux-kernel@vger.kernel.org>, rongtao@cestc.cn
Subject: Re: [PATCH] prctl: Get private anonymous memory region name
Message-ID: <20231125155651.GA23245@redhat.com>
References: <tencent_977CBF8E8CA6234A1B740A35655D5D7EAA0A@qq.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <tencent_977CBF8E8CA6234A1B740A35655D5D7EAA0A@qq.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk

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.