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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231125155651.GA23245@redhat.com>
Date:   Sat, 25 Nov 2023 16:56:51 +0100
From:   Oleg Nesterov <oleg@...hat.com>
To:     Rong Tao <rtoax@...mail.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Florent Revest <revest@...omium.org>,
        Kees Cook <keescook@...omium.org>,
        David Hildenbrand <david@...hat.com>,
        Stefan Roesch <shr@...kernel.io>,
        Andy Chiu <andy.chiu@...ive.com>,
        Josh Triplett <josh@...htriplett.org>,
        Joey Gouly <joey.gouly@....com>,
        Miguel Ojeda <ojeda@...nel.org>,
        Ondrej Mosnacek <omosnace@...hat.com>,
        "open list:MEMORY MANAGEMENT" <linux-mm@...ck.org>,
        open list <linux-kernel@...r.kernel.org>, rongtao@...tc.cn
Subject: Re: [PATCH] prctl: Get private anonymous memory region name

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.