linux-kernel - [PATCH] hugetlbfs: change put_page/unlock_page order in hugetlbfs

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: <20170826191124.51642-1-namit@vmware.com>
Date:   Sat, 26 Aug 2017 12:11:24 -0700
From:   Nadav Amit <namit@...are.com>
To:     Nadia Yvette Chambers <nyc@...omorphy.com>
CC:     <linux-kernel@...r.kernel.org>, Nadav Amit <nadav.amit@...il.com>,
        Nadav Amit <namit@...are.com>,
        Eric Biggers <ebiggers3@...il.com>,
        Mike Kravetz <mike.kravetz@...cle.com>
Subject: [PATCH] hugetlbfs: change put_page/unlock_page order in hugetlbfs_fallocate()

hugetlfs_fallocate() currently performs put_page() before unlock_page().
This scenario opens a small time window, from the time the page is added
to the page cache, until it is unlocked, in which the page might be
removed from the page-cache by another core. If the page is removed
during this time windows, it might cause a memory corruption, as the
wrong page will be unlocked.

It is arguable whether this scenario can happen in a real system, and
there are several mitigating factors. The issue was found by code
inspection (actually grep), and not by actually triggering the flow.
Yet, since putting the page before unlocking is incorrect it should be
fixed, if only to prevent future breakage or someone copy-pasting this
code.

Fixes: 70c3547e36f5c ("hugetlbfs: add hugetlbfs_fallocate()")

cc: Eric Biggers <ebiggers3@...il.com>
cc: Mike Kravetz <mike.kravetz@...cle.com>

Signed-off-by: Nadav Amit <namit@...are.com>
---
 fs/hugetlbfs/inode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 28d2753be094..9475fee79cee 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -655,11 +655,11 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
 		mutex_unlock(&hugetlb_fault_mutex_table[hash]);

 		/*
-		 * page_put due to reference from alloc_huge_page()
 		 * unlock_page because locked by add_to_page_cache()
+		 * page_put due to reference from alloc_huge_page()
 		 */
-		put_page(page);
 		unlock_page(page);
+		put_page(page);
 	}

 	if (!(mode & FALLOC_FL_KEEP_SIZE) && offset + len > inode->i_size)
-- 
2.11.0