Re: [PATCH 11/19] block: implement bio helper to add iter bvec pages to bio

[Jens Axboe <axboe@kernel.dk>]

On 2/25/19 9:34 PM, Jens Axboe wrote:
> On 2/25/19 8:46 PM, Eric Biggers wrote:
>> Hi Jens,
>>
>> On Thu, Feb 21, 2019 at 10:45:27AM -0700, Jens Axboe wrote:
>>> On 2/20/19 3:58 PM, Ming Lei wrote:
>>>> On Mon, Feb 11, 2019 at 12:00:41PM -0700, Jens Axboe wrote:
>>>>> For an ITER_BVEC, we can just iterate the iov and add the pages
>>>>> to the bio directly. This requires that the caller doesn't releases
>>>>> the pages on IO completion, we add a BIO_NO_PAGE_REF flag for that.
>>>>>
>>>>> The current two callers of bio_iov_iter_get_pages() are updated to
>>>>> check if they need to release pages on completion. This makes them
>>>>> work with bvecs that contain kernel mapped pages already.
>>>>>
>>>>> Reviewed-by: Hannes Reinecke <hare@suse.com>
>>>>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>>>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>>> ---
>>>>>  block/bio.c               | 59 ++++++++++++++++++++++++++++++++-------
>>>>>  fs/block_dev.c            |  5 ++--
>>>>>  fs/iomap.c                |  5 ++--
>>>>>  include/linux/blk_types.h |  1 +
>>>>>  4 files changed, 56 insertions(+), 14 deletions(-)
>>>>>
>>>>> diff --git a/block/bio.c b/block/bio.c
>>>>> index 4db1008309ed..330df572cfb8 100644
>>>>> --- a/block/bio.c
>>>>> +++ b/block/bio.c
>>>>> @@ -828,6 +828,23 @@ int bio_add_page(struct bio *bio, struct page *page,
>>>>>  }
>>>>>  EXPORT_SYMBOL(bio_add_page);
>>>>>  
>>>>> +static int __bio_iov_bvec_add_pages(struct bio *bio, struct iov_iter *iter)
>>>>> +{
>>>>> +	const struct bio_vec *bv = iter->bvec;
>>>>> +	unsigned int len;
>>>>> +	size_t size;
>>>>> +
>>>>> +	len = min_t(size_t, bv->bv_len, iter->count);
>>>>> +	size = bio_add_page(bio, bv->bv_page, len,
>>>>> +				bv->bv_offset + iter->iov_offset);
>>>>
>>>> iter->iov_offset needs to be subtracted from 'len', looks
>>>> the following delta change[1] is required, otherwise memory corruption
>>>> can be observed when running xfstests over loop/dio.
>>>
>>> Thanks, I folded this in.
>>>
>>> -- 
>>> Jens Axboe
>>>
>>
>> syzkaller started hitting a crash on linux-next starting with this commit, and
>> it still occurs even with your latest version that has Ming's fix folded in.
>> Specifically, commit a566653ab5ab80a from your io_uring branch with commit date
>> Sun Feb 24 08:20:53 2019 -0700.
>>
>> Reproducer:
>>
>> #define _GNU_SOURCE
>> #include <fcntl.h>
>> #include <linux/loop.h>
>> #include <sys/ioctl.h>
>> #include <sys/sendfile.h>
>> #include <sys/syscall.h>
>> #include <unistd.h>
>>
>> int main(void)
>> {
>>         int memfd, loopfd;
>>
>>         memfd = syscall(__NR_memfd_create, "foo", 0);
>>
>>         pwrite(memfd, "\xa8", 1, 4096);
>>
>>         loopfd = open("/dev/loop0", O_RDWR|O_DIRECT);
>>
>>         ioctl(loopfd, LOOP_SET_FD, memfd);
>>
>>         sendfile(loopfd, loopfd, NULL, 1000000);
>> }
>>
>>
>> Crash:
>>
>> page:ffffea0001a6aab8 count:0 mapcount:0 mapping:0000000000000000 index:0x0
>> flags: 0x100000000000000()
>> raw: 0100000000000000 ffffea0001ad2c50 ffff88807fca49d0 0000000000000000
>> raw: 0000000000000000 0000000000000000 00000000ffffffff
>> page dumped because: VM_BUG_ON_PAGE(page_ref_count(page) == 0)
> 
> I see what this is, I'll cut a fix for this tomorrow.

Folded in a fix for this, it's in my current io_uring branch and my for-next
branch.

-- 
Jens Axboe