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: 7245821 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id AF8189F39B for ; Tue, 22 Sep 2015 21:13:52 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B50B0208CA for ; Tue, 22 Sep 2015 21:13:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3964A20898 for ; Tue, 22 Sep 2015 21:13:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934700AbbIVVNf (ORCPT ); Tue, 22 Sep 2015 17:13:35 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:36513 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934652AbbIVVNe (ORCPT ); Tue, 22 Sep 2015 17:13:34 -0400 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 Cc: linux-kernel@vger.kernel.org, Alexander Viro , Matthew Wilcox , linux-fsdevel@vger.kernel.org, "Kirill A. Shutemov" , linux-nvdimm@ml01.01.org, Dan Williams , Dave Chinner 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 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, 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? --- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html --- 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;