Re: fs: out of bounds on stack in iov_iter_advance

[Jens Axboe <axboe@kernel.dk>]

On 11/10/2015 07:31 PM, Linus Torvalds wrote:
> On Tue, Nov 10, 2015 at 6:25 PM, Jens Axboe <axboe@kernel.dk> wrote:
>> On Tue, Nov 10 2015, Linus Torvalds wrote:
>>> Al, ping?
>>>
>>> On Thu, Nov 5, 2015 at 7:38 PM, Linus Torvalds
>>> <torvalds@linux-foundation.org> wrote:
>>>> On Thu, Nov 5, 2015 at 6:19 PM, Al Viro <viro@zeniv.linux.org.uk> 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@zeniv.linux.org.uk>
>      ---
>      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?

-- 
Jens Axboe