linux-kernel - Re: [PATCH v7 11/22] Replace ext2_clear_xip_target with dax_clear

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]

Message-ID: <20140410183104.GA8060@quack.suse.cz>
Date:	Thu, 10 Apr 2014 20:31:04 +0200
From:	Jan Kara <jack@...e.cz>
To:	Matthew Wilcox <willy@...ux.intel.com>
Cc:	Jan Kara <jack@...e.cz>,
	Matthew Wilcox <matthew.r.wilcox@...el.com>,
	linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 11/22] Replace ext2_clear_xip_target with
 dax_clear_blocks

On Thu 10-04-14 10:16:30, Matthew Wilcox wrote:
> On Wed, Apr 09, 2014 at 11:46:44AM +0200, Jan Kara wrote:
> >   Another day, some more review ;) Comments below.
> 
> I'm really grateful for all this review!  It's killing me, though ;-)
  Yeah, I know that feeling. :)

> > > +int dax_clear_blocks(struct inode *inode, sector_t block, long size)
> > > +{
> > > +	struct block_device *bdev = inode->i_sb->s_bdev;
> > > +	const struct block_device_operations *ops = bdev->bd_disk->fops;
> > > +	sector_t sector = block << (inode->i_blkbits - 9);
> > > +	unsigned long pfn;
> > > +
> > > +	might_sleep();
> > > +	do {
> > > +		void *addr;
> > > +		long count = ops->direct_access(bdev, sector, &addr, &pfn,
> > > +									size);
> >   So do you assume blocksize == PAGE_SIZE here? If not, addr could be in
> > the middle of the page AFAICT.
> 
> You're right.  Depending on how clear_page() is implemented, that
> might go badly wrong.  Of course, both ext2 & ext4 require block_size
> == PAGE_SIZE right now, so anything else is by definition untested.
> I've been trying to keep DAX free from that assumption, but obviously
> haven't caught all the places.
> 
> How does this look?
  That looks fine.

								Honza
-- 
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/