linux-kernel - Re: [PATCH v2] kprobes: kretprobe events missing on 2-core KVM guest

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: <20221110145223.GA14731@openwall.com>
Date:   Thu, 10 Nov 2022 15:52:23 +0100
From:   Solar Designer <solar@...nwall.com>
To:     wuqiang <wuqiang.matt@...edance.com>
Cc:     mhiramat@...nel.org, davem@...emloft.net,
        anil.s.keshavamurthy@...el.com, naveen.n.rao@...ux.ibm.com,
        linux-kernel@...r.kernel.org, mattwu@....com,
        Adam Zabrocki <pi3@....com.pl>
Subject: Re: [PATCH v2] kprobes: kretprobe events missing on 2-core KVM guest

On Thu, Nov 10, 2022 at 04:15:02PM +0800, wuqiang wrote:
> Default value of maxactive is set as num_possible_cpus() for nonpreemptable
> systems. For a 2-core system, only 2 kretprobe instances would be allocated
> in default, then these 2 instances for execve kretprobe are very likely to
> be used up with a pipelined command.
> 
> Here's the testcase: a shell script was added to crontab, and the content
> of the script is:
> 
>   #!/bin/sh
>   do_something_magic `tr -dc a-z < /dev/urandom | head -c 10`
> 
> cron will trigger a series of program executions (4 times every hour). Then
> events loss would be noticed normally after 3-4 hours of testings.
> 
> The issue is caused by a burst of series of execve requests. The best number
> of kretprobe instances could be different case by case, and should be user's
> duty to determine, but num_possible_cpus() as the default value is inadequate
> especially for systems with small number of cpus.
> 
> This patch enables the logic for preemption as default, thus increases the
> minimum of maxactive to 10 for nonpreemptable systems.
> 
> Signed-off-by: wuqiang <wuqiang.matt@...edance.com>

Reviewed-by: Solar Designer <solar@...nwall.com>

Thank you!

> ---
>  Documentation/trace/kprobes.rst |  3 +--
>  kernel/kprobes.c                | 10 +++-------
>  2 files changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/trace/kprobes.rst b/Documentation/trace/kprobes.rst
> index 48cf778a2468..fc7ce76eab65 100644
> --- a/Documentation/trace/kprobes.rst
> +++ b/Documentation/trace/kprobes.rst
> @@ -131,8 +131,7 @@ For example, if the function is non-recursive and is called with a
>  spinlock held, maxactive = 1 should be enough.  If the function is
>  non-recursive and can never relinquish the CPU (e.g., via a semaphore
>  or preemption), NR_CPUS should be enough.  If maxactive <= 0, it is
> -set to a default value.  If CONFIG_PREEMPT is enabled, the default
> -is max(10, 2*NR_CPUS).  Otherwise, the default is NR_CPUS.
> +set to a default value: max(10, 2*NR_CPUS).
>  
>  It's not a disaster if you set maxactive too low; you'll just miss
>  some probes.  In the kretprobe struct, the nmissed field is set to
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index a8b202f87e2d..1e80bddf2654 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -2212,11 +2212,7 @@ int register_kretprobe(struct kretprobe *rp)
>  	rp->kp.post_handler = NULL;
>  
>  	/* Pre-allocate memory for max kretprobe instances */
> -	if (rp->maxactive <= 0) {
> -#ifdef CONFIG_PREEMPTION
> +	if (rp->maxactive <= 0)
>  		rp->maxactive = max_t(unsigned int, 10, 2*num_possible_cpus());
> -#else
> -		rp->maxactive = num_possible_cpus();
> -#endif
> -	}
> +
>  #ifdef CONFIG_KRETPROBE_ON_RETHOOK
> --
> 2.34.1

Alexander