linux-kernel - Re: [PATCH 1/2] mm: Fix slab->page flags corruption.

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: <20120517004434.GX19697@redhat.com>
Date:	Thu, 17 May 2012 02:44:34 +0200
From:	Andrea Arcangeli <aarcange@...hat.com>
To:	Pravin B Shelar <pshelar@...ira.com>
Cc:	cl@...ux.com, penberg@...nel.org, mpm@...enic.com,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org, jesse@...ira.com,
	abhide@...ira.com
Subject: Re: [PATCH 1/2] mm: Fix slab->page flags corruption.

Hi Pravin,

On Mon, May 14, 2012 at 11:41:17AM -0700, Pravin B Shelar wrote:
> Transparent huge pages can change page->flags (PG_compound_lock)
> without taking Slab lock. Since THP can not break slab pages we can
> safely access compound page without taking compound lock.
> 
> Specificly this patch fixes race between compound_unlock and slab
> functions which does page-flags update. This can occur when
> get_page/put_page is called on page from slab object.

DMA on slab running put_page concurrently with kmem_cache_free/kfree
was unexpected. Is this the scenario where the race happens, right?

> diff --git a/mm/swap.c b/mm/swap.c
> index 8ff73d8..d4eb9f6 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -82,6 +82,16 @@ static void put_compound_page(struct page *page)
>  		if (likely(page != page_head &&
>  			   get_page_unless_zero(page_head))) {
>  			unsigned long flags;
> +
> +			if (PageSlab(page_head)) {
> +				/* THP can not break up slab pages, avoid
> +				 * taking compound_lock(). */
> +				if (put_page_testzero(page_head))
> +					VM_BUG_ON(1);
> +
> +				atomic_dec(&page->_mapcount);
> +				goto skip_lock;
> +			}

If a THP is splitted before get_page_unless_zero runs, the head page
may be then freed and reallocated as slab. The "page" then should not
be freed as a tail page anymore, because it's not a tail page. The
head just accidentally become a slab (maybe not even a compound slab).

To avoid such scenario this should be enough:

     if (PageSlab(page_head) && PageTail(page)) {
     ...
     }

Thanks,
Andrea
--
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/