linux-kernel - Re: [PATCH v2 1/4] mm/migration: reduce the rcu lock duration

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: <d260e8b178defbe4facda3cd8f0b534ec812268a.camel@intel.com>
Date:   Fri, 06 May 2022 11:23:14 +0800
From:   "ying.huang@...el.com" <ying.huang@...el.com>
To:     Miaohe Lin <linmiaohe@...wei.com>, akpm@...ux-foundation.org,
        mike.kravetz@...cle.com, naoya.horiguchi@....com
Cc:     hch@....de, dhowells@...hat.com, cl@...ux.com, david@...hat.com,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/4] mm/migration: reduce the rcu lock duration

On Mon, 2022-04-25 at 21:27 +0800, Miaohe Lin wrote:
> rcu_read_lock is required by grabbing the task refcount but it's not
> needed for ptrace_may_access. So we could release the rcu lock after
> task refcount is successfully grabbed to reduce the rcu holding time.
> 
> Reviewed-by: Muchun Song <songmuchun@...edance.com>
> Reviewed-by: Christoph Hellwig <hch@....de>
> Signed-off-by: Miaohe Lin <linmiaohe@...wei.com>
> Cc: Huang Ying <ying.huang@...el.com>
> Cc: David Howells <dhowells@...hat.com>
> Cc: Christoph Lameter <cl@...ux.com>
> ---
>  mm/migrate.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index b2678279eb43..b779646665fe 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1902,17 +1902,16 @@ static struct mm_struct *find_mm_struct(pid_t pid, nodemask_t *mem_nodes)
>  		return ERR_PTR(-ESRCH);
>  	}
>  	get_task_struct(task);
> +	rcu_read_unlock();
>  
> 
>  	/*
>  	 * Check if this process has the right to modify the specified
>  	 * process. Use the regular "ptrace_may_access()" checks.
>  	 */
>  	if (!ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS)) {
> -		rcu_read_unlock();
>  		mm = ERR_PTR(-EPERM);
>  		goto out;
>  	}
> -	rcu_read_unlock();
>  
> 
>  	mm = ERR_PTR(security_task_movememory(task));
>  	if (IS_ERR(mm))

Hi, Miaohe,

Please check the previous discussion and verify whether the original
reported race condition is stll valid by yourself before resending this
patch again.  If you find that the original race condition isn't
possible now, please add the analysis in your change log.

Best Regards,
Huang, Ying