linux-kernel - Re: mm: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected in split_huge_page_to

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: <87powwvm6b.fsf@linux.vnet.ibm.com>
Date:	Wed, 20 Jan 2016 11:15:32 +0530
From:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
To:	"Kirill A. Shutemov" <kirill@...temov.name>,
	Dmitry Vyukov <dvyukov@...gle.com>
Cc:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Sasha Levin <sasha.levin@...cle.com>, jmarchan@...hat.com,
	Hugh Dickins <hughd@...gle.com>,
	Dave Hansen <dave.hansen@...el.com>,
	David Rientjes <rientjes@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Vlastimil Babka <vbabka@...e.cz>,
	Mel Gorman <mgorman@...hsingularity.net>,
	Ebru Akagunduz <ebru.akagunduz@...il.com>,
	Dan Williams <dan.j.williams@...el.com>,
	Minchan Kim <minchan@...nel.org>,
	"linux-mm\@kvack.org" <linux-mm@...ck.org>,
	LKML <linux-kernel@...r.kernel.org>,
	syzkaller <syzkaller@...glegroups.com>,
	Kostya Serebryany <kcc@...gle.com>,
	Alexander Potapenko <glider@...gle.com>
Subject: Re: mm: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected in split_huge_page_to_list

"Kirill A. Shutemov" <kirill@...temov.name> writes:

......

>
> I think this should fix the issue:
>
> From 10859758dadfa249616870f63c1636ec9857c501 Mon Sep 17 00:00:00 2001
> From: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
> Date: Mon, 18 Jan 2016 16:28:12 +0300
> Subject: [PATCH] thp: fix interrupt unsafe locking in split_huge_page()
>
> split_queue_lock can be taken from interrupt context in some cases, but
> I forgot to convert locking in split_huge_page() to interrupt-safe
> primitives.
>
> Let's fix this.

Can you add the stack trace from the problem reported to the commit
message ?. That will help in identifying the interrupt context call path
easily.

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@...ux.vnet.ibm.com> 
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> Reported-by: Dmitry Vyukov <dvyukov@...gle.com>
> ---
>  mm/huge_memory.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 50342eff7960..21fda6a10e89 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3357,6 +3357,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
>  	struct anon_vma *anon_vma;
>  	int count, mapcount, ret;
>  	bool mlocked;
> +	unsigned long flags;
>
>  	VM_BUG_ON_PAGE(is_huge_zero_page(page), page);
>  	VM_BUG_ON_PAGE(!PageAnon(page), page);
> @@ -3396,7 +3397,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
>  		lru_add_drain();
>
>  	/* Prevent deferred_split_scan() touching ->_count */
> -	spin_lock(&split_queue_lock);
> +	spin_lock_irqsave(&split_queue_lock, flags);
>  	count = page_count(head);
>  	mapcount = total_mapcount(head);
>  	if (!mapcount && count == 1) {
> @@ -3404,11 +3405,11 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
>  			split_queue_len--;
>  			list_del(page_deferred_list(head));
>  		}
> -		spin_unlock(&split_queue_lock);
> +		spin_unlock_irqrestore(&split_queue_lock, flags);
>  		__split_huge_page(page, list);
>  		ret = 0;
>  	} else if (IS_ENABLED(CONFIG_DEBUG_VM) && mapcount) {
> -		spin_unlock(&split_queue_lock);
> +		spin_unlock_irqrestore(&split_queue_lock, flags);
>  		pr_alert("total_mapcount: %u, page_count(): %u\n",
>  				mapcount, count);
>  		if (PageTail(page))
> @@ -3416,7 +3417,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
>  		dump_page(page, "total_mapcount(head) > 0");
>  		BUG();
>  	} else {
> -		spin_unlock(&split_queue_lock);
> +		spin_unlock_irqrestore(&split_queue_lock, flags);
>  		unfreeze_page(anon_vma, head);
>  		ret = -EBUSY;
>  	}
> -- 
>  Kirill A. Shutemov