linux-kernel - Re: [PATCH 2/2] dax: fix bdev NULL pointer dereferences

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]

Date:	Sun, 31 Jan 2016 21:55:18 +1100
From:	Matthew Wilcox <willy@...ux.intel.com>
To:	Ross Zwisler <zwisler@...il.com>
Cc:	Dan Williams <dan.j.williams@...el.com>,
	linux-nvdimm <linux-nvdimm@...1.01.org>,
	Dave Chinner <david@...morbit.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Christoph Hellwig <hch@...radead.org>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Jan Kara <jack@...e.com>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 2/2] dax: fix bdev NULL pointer dereferences

On Sat, Jan 30, 2016 at 11:12:12PM -0700, Ross Zwisler wrote:
> > I did probably 70% of the work needed to switch the radix tree over to
> > storing PFNs instead of sectors.  It seems viable, though it's a big
> > change from where we are today:
> 
> At one point I had kaddrs in the radix tree, so I could just pull the addresses out
> and flush them.  That would save us a pfn -> kaddrs conversion before flush.
> 
> Is there a reason to store pnfs instead of kaddrs in the radix tree?

Once ARM, MIPS and SPARC get supported, they're going to need temporary
kernel addresses assigned to PFNs rather than permanent ones.  Also,
it'll be easier for teardown to delete PFNs associated with a particular
device than kaddrs associated with a particular device.  And it lets
us support more persistent memory on a 32-bit machine (also on a 64-bit
machine, but that's mostly theoretical)

+/*
+ * DAX uses the 'exceptional' entries to store PFNs in the radix tree.
+ * Bit 0 is clear (the radix tree uses this for its own purposes).  Bit
+ * 1 is set (to indicate an exceptional entry).  Bits 2 & 3 are PFN_DEV
+ * and PFN_MAP.  The top two bits denote the size of the entry (PTE, PMD,
+ * PUD, one reserved).  That leaves us 26 bits on 32-bit systems and 58
+ * bits on 64-bit systems, able to address 256GB and 1024EB respectively.
+ */

It's also pretty cheap to look up the kaddr from the pfn, at least on
64-bit architectures without cache aliasing problems:

+static void *dax_map_pfn(pfn_t pfn, unsigned long index)
+{
+       preempt_disable();
+       pagefault_disable();
+       return pfn_to_kaddr(pfn_t_to_pfn(pfn));
+}
+
+static void dax_unmap_pfn(void *vaddr)
+{
+       pagefault_enable();
+       preempt_enable();
+}

32-bit x86 is going to want to do something similar to
iomap_atomic_prot_pfn().  ARM/SPARC/MIPS will want something in the
kmap_atomic family.