linux-kernel - Re: [PATCH 3/3] uprobes: Restrict valid_vma(false) to skip VM

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: <20120925090509.GC18334@linux.vnet.ibm.com>
Date:	Tue, 25 Sep 2012 14:35:09 +0530
From:	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Ingo Molnar <mingo@...e.hu>, Peter Zijlstra <peterz@...radead.org>,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	Anton Arapov <anton@...hat.com>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] uprobes: Restrict valid_vma(false) to skip
 VM_SHARED

* Oleg Nesterov <oleg@...hat.com> [2012-09-16 19:52:48]:

> valid_vma(false) ignores ->vm_flags, this is not actually right.
> We should never try to write into MAP_SHARED mapping, this can
> confuse an apllication which actually writes to ->vm_file.

Agree,

> 
> With this patch valid_vma(false) ignores VM_WRITE only but checks
> other (immutable) bits checked by valid_vma(true).

Yes, checking for other immutable flags other than VM_WRITE is good.

> This can also
> speedup uprobe_munmap() and uprobe_unregister().
> 

I didnt get how it speeds up uprobe_munmap() and uprobe_unregister()?

> Note: even after this patch _unregister can confuse the probed
> application if it does mprotect(PROT_WRITE) after _register and
> installs "int3", but this is hardly possible to avoid and this
> doesn't differ from gdb case.
> 

Again I didnt quite understand how unregister can confuse the probed
application.

> Signed-off-by: Oleg Nesterov <oleg@...hat.com>


The changes look good.

Acked-by: Srikar Dronamraju <srikar@...ux.vnet.ibm.com>

> ---
>  kernel/events/uprobes.c |   13 ++++---------
>  1 files changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index b9b50dd..78364a2 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -100,17 +100,12 @@ struct uprobe {
>   */
>  static bool valid_vma(struct vm_area_struct *vma, bool is_register)
>  {
> -	if (!vma->vm_file)
> -		return false;
> -
> -	if (!is_register)
> -		return true;
> +	vm_flags_t flags = VM_HUGETLB | VM_MAYEXEC | VM_SHARED;
> 
> -	if ((vma->vm_flags & (VM_HUGETLB | VM_WRITE | VM_MAYEXEC | VM_SHARED))
> -				== VM_MAYEXEC)
> -		return true;
> +	if (is_register)
> +		flags |= VM_WRITE;
> 
> -	return false;
> +	return vma->vm_file && (vma->vm_flags & flags) == VM_MAYEXEC;
>  }
> 
>  static unsigned long offset_to_vaddr(struct vm_area_struct *vma, loff_t offset)
> -- 
> 1.5.5.1
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/