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] [day] [month] [year] [list]

Message-ID: <tencent_B96B540A71C98045E8392BED324B65E89505@qq.com>
Date:   Sun, 26 Nov 2023 11:04:52 +0800
From:   Rong Tao <rtoax@...mail.com>
To:     Oleg Nesterov <oleg@...hat.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/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

>