linux-kernel - Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault

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]

Date:   Thu, 21 Feb 2019 11:32:51 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     linux-kernel@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        stable@...r.kernel.org, Changbin Du <changbin.du@...il.com>
Subject: Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access
 kernel memory that can fault


[ Linus, you may want to read this ]

On Fri, 22 Feb 2019 01:16:43 +0900
Masami Hiramatsu <mhiramat@...nel.org> wrote:


Yep, I'll take this patch. Hmm, my for-next isn't based on my urgent
branch, and this would go on top of urgent. I can do one of three
things to push this to Linus for the merge window:

 1) Create a separate branch to add updates for on my urgent branch.
    This would have me do two pull requests to Linus.

 2) Cherry pick the urgent commit that this updates.
    But that has a stable tag, which could confuse things as it would
    create the same commit twice, both with stable tags (I could take
    the stable tag off of the cherry pick though).

 3) Merge the urgent branch into my for-next branch at that commit,
    with a message to why I'm doing this. The urgent branch is based
    off of a older tag in Linus's tree, so that would only pull in
    tracing changes (my stuff), and wont pull in anyone else's work.

I may go with option 3, because I believe this may be one of the cases
that is allowed to have merges in pull requests.

 a) Only my stuff got merged
 b) Have a dependency on something that already went into Linus's tree
    (prevent me from having to rebase already tested work).
 c) Have it documented in the merge commit to why its being merged

Unless Linus feels otherwise (use one of the other options?), I may go
ahead and do that.

-- Steve

> ---------
> tracing/kprobes: Use probe_kernel_read instead of probe_mem_read
> 
> From: Masami Hiramatsu <mhiramat@...nel.org>
> 
> Use probe_kernel_read() instead of probe_mem_read() because
> probe_mem_read() is a kind of wrapper for switching memory
> read function between uprobes and kprobes.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@...nel.org>
> ---
>  kernel/trace/trace_kprobe.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 9eaf07f99212..99592c27465e 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -865,7 +865,7 @@ fetch_store_strlen(unsigned long addr)
>  	u8 c;
>  
>  	do {
> -		ret = probe_mem_read(&c, (u8 *)addr + len, 1);
> +		ret = probe_kernel_read(&c, (u8 *)addr + len, 1);
>  		len++;
>  	} while (c && ret == 0 && len < MAX_STRING_SIZE);
>  
>