[PATCH] aio: Fix type of iterator variable in do_io_submit()

[PATCH] aio: Fix type of iterator variable in do_io_submit()

[Eric Biggers]

do_io_submit() iterated over the userspace iocb structure pointers using
a variable i of type 'int'.  This was wrong since 'nr', the number of
iocb structure pointers, could potentially be up to LONG_MAX /
sizeof(struct iocb *).  Fix it (and also remove the unnecessary
initialization to 0).

Signed-off-by: Eric Biggers <ebiggers3@gmail.com>
---
 fs/aio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/aio.c b/fs/aio.c
index 12a3de0e..4c96af7 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1441,7 +1441,7 @@ long do_io_submit(aio_context_t ctx_id, long nr,
 {
 	struct kioctx *ctx;
 	long ret = 0;
-	int i = 0;
+	long i;
 	struct blk_plug plug;
 
 	if (unlikely(nr < 0))
-- 
1.9.2

Re: [PATCH] aio: Fix type of iterator variable in do_io_submit()

[Matthew Wilcox]

On Tue, Apr 22, 2014 at 05:57:03PM -0500, Eric Biggers wrote:
> do_io_submit() iterated over the userspace iocb structure pointers using
> a variable i of type 'int'.  This was wrong since 'nr', the number of
> iocb structure pointers, could potentially be up to LONG_MAX /
> sizeof(struct iocb *).  Fix it (and also remove the unnecessary
> initialization to 0).

You're not wrong, but do we *really* want users to be able to submit
144115188075855872 I/Os with a single system call?  How about limiting
them to a single billion?  Given that they have to allocate 64GB of
*control* data structures to submit this many I/Os, I think this will
be sufficient for many years to come.

Re: [PATCH] aio: Fix type of iterator variable in do_io_submit()

[Benjamin LaHaise]

On Wed, Apr 23, 2014 at 10:16:17AM -0400, Matthew Wilcox wrote:
> On Tue, Apr 22, 2014 at 05:57:03PM -0500, Eric Biggers wrote:
> > do_io_submit() iterated over the userspace iocb structure pointers using
> > a variable i of type 'int'.  This was wrong since 'nr', the number of
> > iocb structure pointers, could potentially be up to LONG_MAX /
> > sizeof(struct iocb *).  Fix it (and also remove the unnecessary
> > initialization to 0).
> 
> You're not wrong, but do we *really* want users to be able to submit
> 144115188075855872 I/Os with a single system call?  How about limiting
> them to a single billion?  Given that they have to allocate 64GB of
> *control* data structures to submit this many I/Os, I think this will
> be sufficient for many years to come.

Practically speaking, this change has no effect.  The io_submit() syscall 
will exit far before we even hit INT_MAX because of the limits on the 
number of iocbs.

		-ben
-- 
"Thought is the essence of where you are now."

Re: [PATCH] aio: Fix type of iterator variable in do_io_submit()

[Eric Biggers]

On Wed, Apr 23, 2014 at 10:42:37AM -0400, Benjamin LaHaise wrote:
> Practically speaking, this change has no effect.  The io_submit() syscall 
> will exit far before we even hit INT_MAX because of the limits on the 
> number of iocbs.

Yes it looks like it doesn't actually make a difference due to the default
'aio-max-nr' limit of 1048576 (although you actually can submit just over twice
this many).  In my opinion this change should still be made so that the
correctness of the code doesn't rely on that nonlocal assumption, however.
And/or the explicit limit of (LONG_MAX / sizeof(struct iocb *)) elements could
be changed to something lower, as Matthew suggested.

Eric