All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Theodore Ts'o <tytso@mit.edu>,
	James Bottomley <James.Bottomley@HansenPartnership.com>,
	Dave Chinner <david@fromorbit.com>, Jens Axboe <axboe@fb.com>,
	linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org
Subject: [regression] fix 32-bit breakage in block device read(2) (was Re: 32-bit bug in iovec iterator changes)
Date: Mon, 23 Jun 2014 08:44:40 +0100	[thread overview]
Message-ID: <20140623074440.GV18016@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20140622115007.GA5333@thunk.org>

On Sun, Jun 22, 2014 at 07:50:07AM -0400, Theodore Ts'o wrote:
> On Sun, Jun 22, 2014 at 02:00:32AM +0100, Al Viro wrote:
> > 
> > PS: I agree that it's worth careful commenting, obviously, but
> > before sending it to Linus (*with* comments) I want to get a
> > confirmation that this one-liner actually fixes what Ted is seeing.
> > I have reproduced it here, and that change makes the breakage go
> > away in my testing, but I'd like to make sure that we are seeing the
> > same thing.  Ted?
> 
> Hep, that fixes things.  Thanks for explaining what was going on!

OK, here it is, hopefully with sufficient comments:
    
blkdev_read_iter() wants to cap the iov_iter by the amount of
data remaining to the end of device.  That's what iov_iter_truncate()
is for (trim iter->count if it's above the given limit).  So far,
so good, but the argument of iov_iter_truncate() is size_t, so on
32bit boxen (in case of a large device) we end up with that upper
limit truncated down to 32 bits *before* comparing it with iter->count.

Easily fixed by making iov_iter_truncate() take 64bit argument -
it does the right thing after such change (we only reach the
assignment in there when the current value of iter->count is greater
than the limit, i.e. for anything that would get truncated we don't
reach the assignment at all) and that argument is not the new
value of iter->count - it's an upper limit for such.

The overhead of passing u64 is not an issue - the thing is inlined,
so callers passing size_t won't pay any penalty.

Reported-by: Theodore Tso <tytso@mit.edu>
Tested-by: Theodore Tso <tytso@mit.edu>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---

diff --git a/include/linux/uio.h b/include/linux/uio.h
index ddfdb53..17ae7e3 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -94,8 +94,20 @@ static inline size_t iov_iter_count(struct iov_iter *i)
 	return i->count;
 }
 
-static inline void iov_iter_truncate(struct iov_iter *i, size_t count)
+/*
+ * Cap the iov_iter by given limit; note that the second argument is
+ * *not* the new size - it's upper limit for such.  Passing it a value
+ * greater than the amount of data in iov_iter is fine - it'll just do
+ * nothing in that case.
+ */
+static inline void iov_iter_truncate(struct iov_iter *i, u64 count)
 {
+	/*
+	 * count doesn't have to fit in size_t - comparison extends both
+	 * operands to u64 here and any value that would be truncated by
+	 * conversion in assignement is by definition greater than all
+	 * values of size_t, including old i->count.
+	 */
 	if (i->count > count)
 		i->count = count;
 }

  reply	other threads:[~2014-06-23  7:44 UTC|newest]

Thread overview: 22+ 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 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
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                             ` Al Viro [this message]
2014-06-23 15:43                               ` [regression] fix 32-bit breakage in block device read(2) (was Re: 32-bit bug in iovec iterator changes) 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=20140623074440.GV18016@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=axboe@fb.com \
    --cc=david@fromorbit.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.