linux-kernel - Re: [PATCH] kprobes: Allow both @symbol

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: <20220313173646.e3d797f9ee3105239179fb5d@kernel.org>
Date:   Sun, 13 Mar 2022 17:36:46 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Bang Li <libang.linuxer@...il.com>
Cc:     naveen.n.rao@...ux.ibm.com, anil.s.keshavamurthy@...el.com,
        davem@...emloft.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kprobes: Allow both @symbol_name and @addr to exist

On Sun, 13 Mar 2022 11:21:25 +0800
Bang Li <libang.linuxer@...il.com> wrote:

> If the address found by the @symbol_name is equal to the @addr, this case
> should be considered a valid parameter combinations.

Could you tell me the reason why this is needed and use case?
I explictly made this case as an error, because I couldn't find
any good reason to support this case.

Thank you,

> 
> Example:
> 
> static struct kprobe kp = {
>     .symbol_name    = "kernel_clone",
>     .addr           = (kprobe_opcode_t*)kernel_clone,
> };
> 
> Signed-off-by: Bang Li <libang.linuxer@...il.com>
> ---
>  kernel/kprobes.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 94cab8c9ce56..70d4ed9e7dc7 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1497,13 +1497,19 @@ bool within_kprobe_blacklist(unsigned long addr)
>  static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr,
>  			const char *symbol_name, unsigned int offset)
>  {
> -	if ((symbol_name && addr) || (!symbol_name && !addr))
> +	kprobe_opcode_t *symbol_addr;
> +
> +	if (!symbol_name && !addr)
>  		goto invalid;
>  
>  	if (symbol_name) {
> -		addr = kprobe_lookup_name(symbol_name, offset);
> -		if (!addr)
> +		symbol_addr = kprobe_lookup_name(symbol_name, offset);
> +		if (!symbol_addr)
>  			return ERR_PTR(-ENOENT);
> +
> +		if (addr && symbol_addr != addr)
> +			goto invalid;
> +		addr = symbol_addr;
>  	}
>  
>  	addr = (kprobe_opcode_t *)(((char *)addr) + offset);
> @@ -2062,8 +2068,7 @@ bool __weak arch_kprobe_on_func_entry(unsigned long offset)
>   * function entry address or not.
>   * This returns 0 if it is the function entry, or -EINVAL if it is not.
>   * And also it returns -ENOENT if it fails the symbol or address lookup.
> - * Caller must pass @addr or @sym (either one must be NULL), or this
> - * returns -EINVAL.
> + * Caller must pass @addr or @sym, or this returns -EINVAL.
>   */
>  int kprobe_on_func_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset)
>  {
> -- 
> 2.25.1
> 


-- 
Masami Hiramatsu <mhiramat@...nel.org>