Re: fs: out of bounds on stack in iov_iter_advance

From: Jens Axboe
Date: Tue Nov 10 2015 - 21:44:25 EST


On 11/10/2015 07:41 PM, Jens Axboe wrote:
On 11/10/2015 07:40 PM, Jens Axboe wrote:
On 11/10/2015 07:31 PM, Linus Torvalds wrote:
On Tue, Nov 10, 2015 at 6:25 PM, Jens Axboe <axboe@xxxxxxxxx> wrote:
On Tue, Nov 10 2015, Linus Torvalds wrote:
Al, ping?

On Thu, Nov 5, 2015 at 7:38 PM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
On Thu, Nov 5, 2015 at 6:19 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx>
wrote:

How are we going to handle that one? I can put it into mainline
pull
request via vfs.git, with Cc: stable, but if e.g. Jens prefers to
take it
via the block tree, I'll be glad to leave it for him to deal with.

Put it in the vfs tree (I'm hoping for a pull request soon..)

I pulled the block trees from Jens yesterday, so there is presumably
nothing pending there right now.

Apparently my "hoping for a pull request soon" was ridiculously
optimistic.

Al, looking at the most recent linux-next, most of the vfs commits
there seem to be committed in the last day or two. I'm getting the
feeling that that is all 4.5 material by now.

Should I just take the iov patch as-is, since apparently no vfs pull
request is happening this merge cycle? And no, I'm not taking
"developed during the second week of the merge window, and sent in the
last few days of it". I'm done with that.

I've got 8 other patches pending for a post core merge, just waiting
for
the last core pull request to go in. I haven't seen this iov iter fix,
though.

It was in this thread, looked like this (without the whitespace damage):

dax_io(): don't let non-error value escape via retval instead of
EFAULT

Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
---
diff --git a/fs/dax.c b/fs/dax.c
index a86d3cc..7b653e9 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -169,8 +169,10 @@ static ssize_t dax_io(struct inode *inode,
struct iov_iter *iter,
else
len = iov_iter_zero(max - pos, iter);

- if (!len)
+ if (!len) {
+ retval = -EFAULT;
break;
+ }

pos += len;
addr += len;


although I don't think I saw a confirmation that that was what Sasha
actually hit (but Sasha had narrowed it down to DAX, so it looks
possible/likely)

I found it right after sending that email. Patch looks pretty straight
forward, at least from the case of max - pos != 0 and len == 0 on
return. Might be cleaner to add a

if (retval < 0)
break;

check, that should be the case where max == pos anyway. But we'd
potentially return -Exx into -EFAULT for that case with the patch.

Hmm?

So we already do that, in the 'if' above. I think the patch looks fine.

Queued up. Unless Al objects, it'll be part of the 'for-linus' pull later this week.

--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/