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  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, 20 Aug 2020 20:25:55 -0400
From:   Qian Cai <cai@....pw>
To:     Davidlohr Bueso <dave@...olabs.net>
Cc:     akpm@...ux-foundation.org, catalin.marinas@....com,
        oleg@...hat.com, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Davidlohr Bueso <dbueso@...e.de>
Subject: Re: [PATCH] mm/kmemleak: rely on rcu for task stack scanning

On Thu, Aug 20, 2020 at 01:39:02PM -0700, Davidlohr Bueso wrote:
> kmemleak_scan() currently relies on the big tasklist_lock
> hammer to stabilize iterating through the tasklist. Instead,
> this patch proposes simply using rcu along with the rcu-safe
> for_each_process_thread flavor (without changing scan semantics),
> which doesn't make use of next_thread/p->thread_group and thus
> cannot race with exit. Furthermore, any races with fork()
> and not seeing the new child should be benign as it's not
> running yet and can also be detected by the next scan.

It is not entirely clear to me what problem the patch is trying to solve. If
this is about performance, we will probably need some number.

> 
> Signed-off-by: Davidlohr Bueso <dbueso@...e.de>
> ---
>  mm/kmemleak.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 5e252d91eb14..c0014d3b91c1 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -1471,15 +1471,15 @@ static void kmemleak_scan(void)
>  	if (kmemleak_stack_scan) {
>  		struct task_struct *p, *g;
>  
> -		read_lock(&tasklist_lock);
> -		do_each_thread(g, p) {
> +		rcu_read_lock();
> +		for_each_process_thread(g, p) {
>  			void *stack = try_get_task_stack(p);
>  			if (stack) {
>  				scan_block(stack, stack + THREAD_SIZE, NULL);
>  				put_task_stack(p);
>  			}
> -		} while_each_thread(g, p);
> -		read_unlock(&tasklist_lock);
> +		}
> +		rcu_read_unlock();
>  	}
>  
>  	/*
> -- 
> 2.26.2

Powered by blists - more mailing lists