Re: [PATCH] kprobes: Allow both @symbol_name and @addr to exist

[Masami Hiramatsu <mhiramat@kernel.org>]

On Sun, 13 Mar 2022 11:21:25 +0800
Bang Li <libang.linuxer@gmail.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@gmail.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@kernel.org>