public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Dave Chinner <david@fromorbit.com>, Jens Axboe <axboe@fb.com>,
	linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org
Subject: Re: 32-bit bug in iovec iterator changes
Date: Sat, 21 Jun 2014 19:09:22 -0400	[thread overview]
Message-ID: <20140621230922.GA13188@thunk.org> (raw)
In-Reply-To: <20140621055306.GP18016@ZenIV.linux.org.uk>

On Sat, Jun 21, 2014 at 06:53:07AM +0100, Al Viro wrote:
> 
> ed include/linux/uio.h <<EOF
> /iov_iter_truncate/s/size_t/u64/
> w
> q
> EOF
> 
> Could you check if that fixes the sucker?

The following patch (attached at the end) appears to fix the problem,
but looking at uio.h, I'm completely confused about *why* it fixes the
problem.  In particular, iov_iter_iovec() makes no sense to me at all,
and I don't understand how the calculation of iov_len makes any sense:

		.iov_len = min(iter->count,
			       iter->iov->iov_len - iter->iov_offset),

It also looks like uio.h is mostly about offsets to memory pointers,
and so why this would make a difference when the issue is the block
device offset goes above 2**30?

There must be deep magic going on here, and so I don't know if your
s/size_t/u64/g substitation also extends to the various functions that
have size_t in them:

unsigned long iov_shorten(struct iovec *iov, unsigned long nr_segs, size_t to);
size_t iov_iter_copy_from_user_atomic(struct page *page,
		struct iov_iter *i, unsigned long offset, size_t bytes);
void iov_iter_advance(struct iov_iter *i, size_t bytes);
int iov_iter_fault_in_readable(struct iov_iter *i, size_t bytes);
size_t iov_iter_single_seg_count(const struct iov_iter *i);
size_t copy_page_to_iter(struct page *page, size_t offset, size_t bytes,
			 struct iov_iter *i);
size_t copy_page_from_iter(struct page *page, size_t offset, size_t bytes,
			 struct iov_iter *i);
unsigned long iov_iter_alignment(const struct iov_iter *i);
void iov_iter_init(struct iov_iter *i, int direction, const struct iovec *iov,
			unsigned long nr_segs, size_t count);
ssize_t iov_iter_get_pages(struct iov_iter *i, struct page **pages,
			size_t maxsize, size_t *start);
ssize_t iov_iter_get_pages_alloc(struct iov_iter *i, struct page ***pages,
			size_t maxsize, size_t *start);


Anyway, this patch does appear to make the problem go away, but given
that I don't understand what is going on here, please take it with a
huge grain of salt.  And might I suggest some comments to perhaps give
some context to someone who is trying to understand
include/linux/uio.h?

Thanks!!

						- Ted

diff --git a/include/linux/uio.h b/include/linux/uio.h
index e2231e4..bea7b7d 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -16,7 +16,7 @@ struct page;
 
 struct kvec {
 	void *iov_base; /* and that should *never* hold a userland pointer */
-	size_t iov_len;
+	u64 iov_len;
 };
 
 enum {
@@ -27,8 +27,8 @@ enum {
 
 struct iov_iter {
 	int type;
-	size_t iov_offset;
-	size_t count;
+	u64 iov_offset;
+	u64 count;
 	union {
 		const struct iovec *iov;
 		const struct bio_vec *bvec;
@@ -41,12 +41,12 @@ struct iov_iter {
  *
  * NOTE that it is not safe to use this function until all the iovec's
  * segment lengths have been validated.  Because the individual lengths can
- * overflow a size_t when added together.
+ * overflow a u64 when added together.
  */
-static inline size_t iov_length(const struct iovec *iov, unsigned long nr_segs)
+static inline u64 iov_length(const struct iovec *iov, unsigned long nr_segs)
 {
 	unsigned long seg;
-	size_t ret = 0;
+	u64 ret = 0;
 
 	for (seg = 0; seg < nr_segs; seg++)
 		ret += iov[seg].iov_len;
@@ -89,12 +89,12 @@ ssize_t iov_iter_get_pages_alloc(struct iov_iter *i, struct page ***pages,
 			size_t maxsize, size_t *start);
 int iov_iter_npages(const struct iov_iter *i, int maxpages);
 
-static inline size_t iov_iter_count(struct iov_iter *i)
+static inline u64 iov_iter_count(struct iov_iter *i)
 {
 	return i->count;
 }
 
-static inline void iov_iter_truncate(struct iov_iter *i, size_t count)
+static inline void iov_iter_truncate(struct iov_iter *i, u64 count)
 {
 	if (i->count > count)
 		i->count = count;
@@ -104,7 +104,7 @@ static inline void iov_iter_truncate(struct iov_iter *i, size_t count)
  * reexpand a previously truncated iterator; count must be no more than how much
  * we had shrunk it.
  */
-static inline void iov_iter_reexpand(struct iov_iter *i, size_t count)
+static inline void iov_iter_reexpand(struct iov_iter *i, u64 count)
 {
 	i->count = count;
 }


  reply	other threads:[~2014-06-21 23:09 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-19 15:35 BUG: scheduling while atomic in blk_mq codepath? Theodore Ts'o
2014-06-19 15:59 ` Jens Axboe
2014-06-19 16:08   ` Theodore Ts'o
2014-06-19 16:21     ` Theodore Ts'o
2014-06-19 22:38       ` Dave Chinner
2014-06-21  3:51         ` 32-bit bug in iovec iterator changes Theodore Ts'o
2014-06-21  5:53           ` Al Viro
2014-06-21 23:09             ` Theodore Ts'o [this message]
2014-06-21 23:49               ` Al Viro
2014-06-22  0:03                 ` James Bottomley
2014-06-22  0:26                   ` Al Viro
2014-06-22  0:32                     ` James Bottomley
2014-06-22  0:53                       ` Al Viro
2014-06-22  1:00                         ` Al Viro
2014-06-22 11:50                           ` Theodore Ts'o
2014-06-23  7:44                             ` [regression] fix 32-bit breakage in block device read(2) (was Re: 32-bit bug in iovec iterator changes) Al Viro
2014-06-23 15:43                               ` Theodore Ts'o
2014-06-24 12:33                                 ` One Thousand Gnomes
2014-06-25 16:56                               ` Linus Torvalds
2014-06-26 15:27                               ` Bruno Wolff III
2014-06-22  1:00                         ` 32-bit bug in iovec iterator changes James Bottomley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140621230922.GA13188@thunk.org \
    --to=tytso@mit.edu \
    --cc=axboe@fb.com \
    --cc=david@fromorbit.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox