linux-kernel - Re: THP bug and crash on sparc64 3.8

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: <alpine.DEB.2.02.1302191134570.790@chino.kir.corp.google.com>
Date:	Tue, 19 Feb 2013 11:37:30 -0800 (PST)
From:	David Rientjes <rientjes@...gle.com>
To:	David Miller <davem@...emloft.net>
cc:	mroos@...ee, linux-mm@...r.kernel.org,
	linux-kernel@...r.kernel.org, sparclinux@...r.kernel.org
Subject: Re: THP bug and crash on sparc64 3.8

On Tue, 19 Feb 2013, David Miller wrote:

> diff --git a/arch/sparc/mm/tsb.c b/arch/sparc/mm/tsb.c
> index 7f64743..338b406 100644
> --- a/arch/sparc/mm/tsb.c
> +++ b/arch/sparc/mm/tsb.c
> @@ -288,14 +288,13 @@ static unsigned long tsb_size_to_rss_limit(unsigned long new_size)
>   * the number of entries that the current TSB can hold at once.  Currently,
>   * we trigger when the RSS hits 3/4 of the TSB capacity.
>   */
> -void tsb_grow(struct mm_struct *mm, unsigned long tsb_index, unsigned long rss)
> +void tsb_grow(struct mm_struct *mm, unsigned long tsb_index, unsigned long rss, gfp_t gfp)
>  {
>  	unsigned long max_tsb_size = 1 * 1024 * 1024;
>  	unsigned long new_size, old_size, flags;
>  	struct tsb *old_tsb, *new_tsb;
>  	unsigned long new_cache_index, old_cache_index;
>  	unsigned long new_rss_limit;
> -	gfp_t gfp_flags;
>  
>  	if (max_tsb_size > (PAGE_SIZE << MAX_ORDER))
>  		max_tsb_size = (PAGE_SIZE << MAX_ORDER);
> @@ -312,12 +311,11 @@ void tsb_grow(struct mm_struct *mm, unsigned long tsb_index, unsigned long rss)
>  		new_rss_limit = ~0UL;
>  
>  retry_tsb_alloc:
> -	gfp_flags = GFP_KERNEL;
>  	if (new_size > (PAGE_SIZE * 2))
> -		gfp_flags = __GFP_NOWARN | __GFP_NORETRY;
> +		gfp = __GFP_NOWARN | __GFP_NORETRY;

The existing code here already looks buggy for a different reason, it 
should be gfp |= __GFP_NOWARN | __GFP_NORETRY;

I was wondering if you could get away with just a simple one-liner fix 
above by doing

	gfp_flags = rss ? GFP_KERNEL : GFP_NOWAIT;

but I agree this is more robust.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/