Re: fs: out of bounds on stack in iov_iter_advance

[Jens Axboe]

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/