From patchwork Tue Sep 22 08:12:34 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Carpenter X-Patchwork-Id: 11791807 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id AECCE112E for ; Tue, 22 Sep 2020 08:14:55 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 4DBF423A1E for ; Tue, 22 Sep 2020 08:14:55 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=oracle.com header.i=@oracle.com header.b="kxx8brxE" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4DBF423A1E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=oracle.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 5781B900035; Tue, 22 Sep 2020 04:14:54 -0400 (EDT) Delivered-To: linux-mm-outgoing@kvack.org Received: by kanga.kvack.org (Postfix, from userid 40) id 527DA6B00EA; Tue, 22 Sep 2020 04:14:54 -0400 (EDT) X-Original-To: int-list-linux-mm@kvack.org X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3C98F900035; Tue, 22 Sep 2020 04:14:54 -0400 (EDT) X-Original-To: linux-mm@kvack.org X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0034.hostedemail.com [216.40.44.34]) by kanga.kvack.org (Postfix) with ESMTP id 272126B00E9 for ; Tue, 22 Sep 2020 04:14:54 -0400 (EDT) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id DF9B4180AD801 for ; Tue, 22 Sep 2020 08:14:53 +0000 (UTC) X-FDA: 77289986466.05.hand85_520c9382714c Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin05.hostedemail.com (Postfix) with ESMTP id C3AFC1801B5EE for ; Tue, 22 Sep 2020 08:14:53 +0000 (UTC) X-Spam-Summary: 1,0,0,,d41d8cd98f00b204,dan.carpenter@oracle.com,,RULES_HIT:30012:30054:30064:30090,0,RBL:156.151.31.85:@oracle.com:.lbl8.mailshell.net-64.10.201.10 62.18.0.100;04ygsz918itpfypnu6cd454ksq89eopmaqxo9sasw9zemfhk5ma1zjztp16c15f.5fgdpshug3z6tdxcbu5cwxz6pj3zb39hwnwpghyci6yi5ypybgk8moqtwduxorz.1-lbl8.mailshell.net-223.238.255.100,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:ft,MSBL:0,DNSBL:neutral,Custom_rules:0:0:0,LFtime:23,LUA_SUMMARY:none X-HE-Tag: hand85_520c9382714c X-Filterd-Recvd-Size: 6591 Received: from userp2120.oracle.com (userp2120.oracle.com [156.151.31.85]) by imf46.hostedemail.com (Postfix) with ESMTP for ; Tue, 22 Sep 2020 08:14:53 +0000 (UTC) Received: from pps.filterd (userp2120.oracle.com [127.0.0.1]) by userp2120.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 08M8EhNc095850; Tue, 22 Sep 2020 08:14:43 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=date : from : to : cc : subject : message-id : mime-version : content-type; s=corp-2020-01-29; bh=bauJ2DC/bCsNhrnQrUWzEjWIVChV4bJP3Ef+wIQJ+GE=; b=kxx8brxErB8cHdZkhXKkSIE0UnX6kAhelrA2X3dFEWmHyzZlFS/NDHa7x7SXqCmPtbkg ZsDTQOte7zC0u/iUb55snk8oDvXr+Jc46l+XBEQZj8HYzgbkHbo7IIjvV0fN1JQg3U2Y tPSnx1/9F/Abg45fZj4zFvTTZG7Cq3wBYRDPANPv9Ksu2IVM0CwDIUSdXCJvz4gkCZ2R TuX9ZOJlpd2mxqdsVIE1q0A3rcJavmjk+3JnzbW8SRrYVswg54qqqQTbnp+8QhrX7+Gc xo7JZiMGNJeue0VWpZR3CV7HRnPpIir4Zx7MXQfJZvHtUIjkwiqlbCx2v/mXQbciIxcq WQ== Received: from aserp3020.oracle.com (aserp3020.oracle.com [141.146.126.70]) by userp2120.oracle.com with ESMTP id 33ndnubc0n-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 22 Sep 2020 08:14:43 +0000 Received: from pps.filterd (aserp3020.oracle.com [127.0.0.1]) by aserp3020.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 08M8Bc9x097485; Tue, 22 Sep 2020 08:12:42 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by aserp3020.oracle.com with ESMTP id 33nuw3756h-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 22 Sep 2020 08:12:42 +0000 Received: from abhmp0002.oracle.com (abhmp0002.oracle.com [141.146.116.8]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id 08M8CfNn027097; Tue, 22 Sep 2020 08:12:41 GMT Received: from mwanda (/41.57.98.10) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Tue, 22 Sep 2020 01:12:41 -0700 Date: Tue, 22 Sep 2020 11:12:34 +0300 From: Dan Carpenter To: =?iso-8859-1?q?J=E9r=F4me?= Glisse , Ralph Campbell Cc: Jason Gunthorpe , Wei Yongjun , linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: [PATCH] mm/hmm/test: use after free in dmirror_allocate_chunk() Message-ID: <20200922081234.GA1274646@mwanda> MIME-Version: 1.0 Content-Disposition: inline X-Mailer: git-send-email haha only kidding X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9751 signatures=668679 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 phishscore=0 malwarescore=0 mlxscore=0 suspectscore=2 adultscore=0 mlxlogscore=999 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2006250000 definitions=main-2009220067 X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9751 signatures=668679 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0 lowpriorityscore=0 phishscore=0 adultscore=0 suspectscore=2 bulkscore=0 clxscore=1011 impostorscore=0 mlxlogscore=999 mlxscore=0 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2006250000 definitions=main-2009220068 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: The error handling code does this: err_free: kfree(devmem); ^^^^^^^^^^^^^ err_release: release_mem_region(devmem->pagemap.range.start, range_len(&devmem->pagemap.range)); ^^^^^^^^ The problem is that when we use "devmem->pagemap.range.start" the "devmem" pointer is either NULL or freed. Neither the allocation nor the call to request_free_mem_region() has to be done under the lock so I moved those to the start of the function. Fixes: b2ef9f5a5cb3 ("mm/hmm/test: add selftest driver for HMM") Signed-off-by: Dan Carpenter Reviewed-by: Ralph Campbell --- It's weird that I didn't catch the use after free when this code was merged in May... My bad. Not sure what happened there. How I found this was that I have been reviewing release_mem_region() leaks and the NULL dereference path is a leak. lib/test_hmm.c | 47 ++++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/lib/test_hmm.c b/lib/test_hmm.c index c8133f50160b..0503c78cb322 100644 --- a/lib/test_hmm.c +++ b/lib/test_hmm.c @@ -459,6 +459,22 @@ static bool dmirror_allocate_chunk(struct dmirror_device *mdevice, unsigned long pfn_last; void *ptr; + devmem = kzalloc(sizeof(*devmem), GFP_KERNEL); + if (!devmem) + return -ENOMEM; + + res = request_free_mem_region(&iomem_resource, DEVMEM_CHUNK_SIZE, + "hmm_dmirror"); + if (IS_ERR(res)) + goto err_devmem; + + devmem->pagemap.type = MEMORY_DEVICE_PRIVATE; + devmem->pagemap.range.start = res->start; + devmem->pagemap.range.end = res->end; + devmem->pagemap.nr_range = 1; + devmem->pagemap.ops = &dmirror_devmem_ops; + devmem->pagemap.owner = mdevice; + mutex_lock(&mdevice->devmem_lock); if (mdevice->devmem_count == mdevice->devmem_capacity) { @@ -471,30 +487,16 @@ static bool dmirror_allocate_chunk(struct dmirror_device *mdevice, sizeof(new_chunks[0]) * new_capacity, GFP_KERNEL); if (!new_chunks) - goto err; + goto err_release; mdevice->devmem_capacity = new_capacity; mdevice->devmem_chunks = new_chunks; } - res = request_free_mem_region(&iomem_resource, DEVMEM_CHUNK_SIZE, - "hmm_dmirror"); - if (IS_ERR(res)) - goto err; - - devmem = kzalloc(sizeof(*devmem), GFP_KERNEL); - if (!devmem) - goto err_release; - - devmem->pagemap.type = MEMORY_DEVICE_PRIVATE; - devmem->pagemap.range.start = res->start; - devmem->pagemap.range.end = res->end; - devmem->pagemap.nr_range = 1; - devmem->pagemap.ops = &dmirror_devmem_ops; - devmem->pagemap.owner = mdevice; - ptr = memremap_pages(&devmem->pagemap, numa_node_id()); - if (IS_ERR(ptr)) - goto err_free; + if (IS_ERR(ptr)) { + mutex_unlock(&mdevice->devmem_lock); + goto err_release; + } devmem->mdevice = mdevice; pfn_first = devmem->pagemap.range.start >> PAGE_SHIFT; @@ -525,12 +527,11 @@ static bool dmirror_allocate_chunk(struct dmirror_device *mdevice, return true; -err_free: - kfree(devmem); err_release: release_mem_region(devmem->pagemap.range.start, range_len(&devmem->pagemap.range)); -err: - mutex_unlock(&mdevice->devmem_lock); +err_devmem: + kfree(devmem); + return false; }