From patchwork Tue Sep 22 21:13:33 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Morton X-Patchwork-Id: 7245801 Return-Path: X-Original-To: patchwork-linux-nvdimm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 24B39BEEC1 for ; Tue, 22 Sep 2015 21:13:37 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 486832089D for ; Tue, 22 Sep 2015 21:13:36 +0000 (UTC) Received: from ml01.01.org (ml01.01.org [198.145.21.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 396AB20898 for ; Tue, 22 Sep 2015 21:13:35 +0000 (UTC) Received: from ml01.vlan14.01.org (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 1F085613C3; Tue, 22 Sep 2015 14:13:35 -0700 (PDT) X-Original-To: linux-nvdimm@ml01.01.org Delivered-To: linux-nvdimm@ml01.01.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 60CBB613A7 for ; Tue, 22 Sep 2015 14:13:34 -0700 (PDT) Received: from akpm3.mtv.corp.google.com (unknown [216.239.45.65]) by mail.linuxfoundation.org (Postfix) with ESMTPSA id CBED01608; Tue, 22 Sep 2015 21:13:33 +0000 (UTC) Date: Tue, 22 Sep 2015 14:13:33 -0700 From: Andrew Morton To: Ross Zwisler Subject: Re: [PATCH v2] dax: fix NULL pointer in __dax_pmd_fault() Message-Id: <20150922141333.c28e3c5d800267937ca7b29a@linux-foundation.org> In-Reply-To: <1442950582-10140-1-git-send-email-ross.zwisler@linux.intel.com> References: <1442950582-10140-1-git-send-email-ross.zwisler@linux.intel.com> X-Mailer: Sylpheed 3.4.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) Mime-Version: 1.0 Cc: linux-nvdimm@ml01.01.org, Dave Chinner , linux-kernel@vger.kernel.org, Alexander Viro , linux-fsdevel@vger.kernel.org, "Kirill A. Shutemov" X-BeenThere: linux-nvdimm@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "Linux-nvdimm developer list." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, 22 Sep 2015 13:36:22 -0600 Ross Zwisler wrote: > The following commit: > > commit 46c043ede471 ("mm: take i_mmap_lock in unmap_mapping_range() for > DAX") > > moved some code in __dax_pmd_fault() that was responsible for zeroing > newly allocated PMD pages. The new location didn't properly set up > 'kaddr', though, so when run this code resulted in a NULL pointer BUG. > > Fix this by getting the correct 'kaddr' via bdev_direct_access(). Why the heck didn't gcc warn? I had a fiddle: gcc warns about the first printk, but not about the second. So that "if (...) return ..." seems to have defeated gcc uninitialized-var detection. wtf? > --- a/fs/dax.c > +++ b/fs/dax.c > @@ -569,8 +569,20 @@ int __dax_pmd_fault(struct vm_area_struct *vma, unsigned long address, > if (!buffer_size_valid(&bh) || bh.b_size < PMD_SIZE) > goto fallback; > > + sector = bh.b_blocknr << (blkbits - 9); > + > if (buffer_unwritten(&bh) || buffer_new(&bh)) { > int i; > + > + length = bdev_direct_access(bh.b_bdev, sector, &kaddr, &pfn, > + bh.b_size); > + if (length < 0) { > + result = VM_FAULT_SIGBUS; > + goto out; > + } > + if ((length < PMD_SIZE) || (pfn & PG_PMD_COLOUR)) > + goto fallback; > + > for (i = 0; i < PTRS_PER_PMD; i++) > clear_pmem(kaddr + i * PAGE_SIZE, PAGE_SIZE); > wmb_pmem(); hm, that's a lot of copy-n-paste. Do we really need to run bdev_direct_access() twice? Will `kaddr' and `pfn' change? --- a/fs/dax.c~a +++ a/fs/dax.c @@ -529,15 +529,18 @@ int __dax_pmd_fault(struct vm_area_struc unsigned long pmd_addr = address & PMD_MASK; bool write = flags & FAULT_FLAG_WRITE; long length; - void __pmem *kaddr; + void *kaddr; pgoff_t size, pgoff; sector_t block, sector; unsigned long pfn; int result = 0; +// printk("%p\n", kaddr); + /* Fall back to PTEs if we're going to COW */ if (write && !(vma->vm_flags & VM_SHARED)) return VM_FAULT_FALLBACK; + printk("%p\n", kaddr); /* If the PMD would extend outside the VMA */ if (pmd_addr < vma->vm_start) return VM_FAULT_FALLBACK;