linux-kernel - RE: regression introduced by "block: Add support for DAX reads/writes to block devices"

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: <100D68C7BA14664A8938383216E40DE0409144D9@FMSMSX114.amr.corp.intel.com>
Date:	Fri, 7 Aug 2015 18:11:14 +0000
From:	"Wilcox, Matthew R" <matthew.r.wilcox@...el.com>
To:	Jeff Moyer <jmoyer@...hat.com>
CC:	"linda.knippers@...com" <linda.knippers@...com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>
Subject: RE: regression introduced by "block: Add support for DAX
 reads/writes to block devices"

So ... what you're doing here is if somebody requests the last 512 bytes, you're asking for the last page.  That's going to work as long as the partition is a multiple of PAGE_SIZE, but not if the partition happens to be misaligned.  It's an improvement, although I'd be tempted to do this as:

		if (pos == max) {
 			unsigned blkbits = inode->i_blkbits;
- 			sector_t block = pos >> blkbits;
+			long page = pos >> PAGE_SHIFT;
+			sector_t block = page << (PAGE_SHIFT - blkbits);
			unsigned first = pos - (block << blkbits);
			long size;

We need to cope with the case where the end of a partition isn't on a page boundary though.

-----Original Message-----
From: Jeff Moyer [mailto:jmoyer@...hat.com] 
Sent: Thursday, August 06, 2015 2:31 PM
To: Wilcox, Matthew R
Cc: linda.knippers@...com; linux-kernel@...r.kernel.org; linux-fsdevel@...r.kernel.org
Subject: Re: regression introduced by "block: Add support for DAX reads/writes to block devices"

"Wilcox, Matthew R" <matthew.r.wilcox@...el.com> writes:

> I think I see the problem.  I'm kind of wrapped up in other things
> right now; can you try replacing the line in dax_io():
>
> -				bh->b_size = PAGE_ALIGN(end - pos);
> +				bh->b_size = ALIGN(end - pos, 1 << blkbits);

This works for me.  If it looks okay to others, I'll submit a properly
signed-off patch.

Cheers,
Jeff

diff --git a/fs/dax.c b/fs/dax.c
index a7f77e1..b6c4f93 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -98,6 +98,10 @@ static bool buffer_size_valid(struct buffer_head *bh)
 	return bh->b_state != 0;
 }
 
+/*
+ * This function assumes file system block size (represented by
+ * inode->i_blkbits) is less than or equal to the system page size.
+ */
 static ssize_t dax_io(struct inode *inode, struct iov_iter *iter,
 		      loff_t start, loff_t end, get_block_t get_block,
 		      struct buffer_head *bh)
@@ -117,9 +121,15 @@ static ssize_t dax_io(struct inode *inode, struct iov_iter *iter,
 		if (pos == max) {
 			unsigned blkbits = inode->i_blkbits;
 			sector_t block = pos >> blkbits;
-			unsigned first = pos - (block << blkbits);
+			long page = pos >> PAGE_SHIFT;
+			unsigned first; /* byte offset into block */
 			long size;
 
+			/* we can only map entire pages */
+			if (pos & (PAGE_SIZE-1))
+				block = page << (PAGE_SHIFT - blkbits);
+			first = pos - (block << blkbits);
+
 			if (pos == bh_max) {
 				bh->b_size = PAGE_ALIGN(end - pos);
 				bh->b_state = 0;

--
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/