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

Open Source and information security mailing list archives

Date:	Sat, 30 Jan 2016 23:12:12 -0700
From:	Ross Zwisler <zwisler@...il.com>
To:	Matthew Wilcox <willy@...ux.intel.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 Jan 30, 2016, at 7:32 PM, Matthew Wilcox <willy@...ux.intel.com> wrote:
> 
>> On Fri, Jan 29, 2016 at 10:01:13PM -0800, Dan Williams wrote:
>>> On Fri, Jan 29, 2016 at 9:28 PM, Matthew Wilcox <willy@...ux.intel.com> wrote:
>>> If we store the PFN of the underlying page instead, we don't have this
>>> problem.  Instead, we have a different problem; of the device going
>>> away under us.  I'm trying to find the code which tears down PTEs when
>>> the device goes away, and I'm not seeing it.  What do we do about user
>>> mappings of the device?
>> 
>> I deferred the dax tear down code until next cycle as Al rightly
>> pointed out some needed re-works:
>> 
>> https://lists.01.org/pipermail/linux-nvdimm/2016-January/003995.html
> 
> Thanks; I eventually found it in my email somewhere over the Pacific.
> 
> 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?

> 
> fs/dax.c                   | 415 +++++++++++++++++++++++----------------------
> include/linux/dax.h        |   3 +-
> include/linux/pfn_t.h      |  33 +++-
> include/linux/radix-tree.h |   9 -
> 4 files changed, 236 insertions(+), 224 deletions(-)
> 
> I'll try and get that finished off this week.
> 
> One concrete and easily-separable piece is that dax_clear_blocks() has
> the wrong signature.  It currently takes an inode & block as parameters;
> it has no way of finding out the correct block device.  It's only two
> callers are filesystems (ext2 and xfs).  Those filesystems should be
> passing the block_device instead of the inode.  But without the inode,
> we can't convert a block number to a sector number, so we also need
> to pass the sector number, not the block number.  It still has type
> sector_t, annoyingly.
> 
> @@ -63,12 +238,11 @@ static void dax_unmap_atomic(struct block_device *bdev,
>  * and hence this means the stack from this point must follow GFP_NOFS
>  * semantics for all operations.
>  */
> -int dax_clear_blocks(struct inode *inode, sector_t block, long _size)
> +int dax_clear_blocks(struct block_device *bdev, sector_t sector, long size)
> {
> -       struct block_device *bdev = inode->i_sb->s_bdev;
>        struct blk_dax_ctl dax = {
> -               .sector = block << (inode->i_blkbits - 9),
> -               .size = _size,
> +               .sector = sector,
> +               .size = size,
>        };
> 
>        might_sleep();
> 
> but I haven't looked at doing the conversion of xfs or ext2 to use that
> new interface.
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@...ts.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm

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