linux-kernel - Re: [PATCH] block: copy bi_vcnt in __bio_clone

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACVXFVO+LBseOCmQuHaVz5LnK0hgvp1iPPhkGWFZc_iOxjBnfg@mail.gmail.com>
Date:	Thu, 9 Oct 2014 23:25:22 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	Jeff Mahoney <jeffm@...e.com>
Cc:	Jeff Moyer <jmoyer@...hat.com>, Jens Axboe <axboe@...nel.dk>,
	Linux Kernel Maling List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] block: copy bi_vcnt in __bio_clone_fast

On Thu, Oct 9, 2014 at 10:26 PM, Jeff Mahoney <jeffm@...e.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 10/9/14, 9:53 AM, Jeff Moyer wrote:
>> Jeff Mahoney <jeffm@...e.com> writes:
>>
>>> Commit 05f1dd53152173 (block: add queue flag for disabling SG
>>> merging) uses bi_vcnt to assign bio->bi_phys_segments if sg
>>> merging is disabled. When using device mapper on top of a blk-mq
>>> device (virtio_blk in my test), we'd end up overflowing the
>>> scatterlist in __blk_bios_map_sg.
>>>
>>> __bio_clone_fast copies bi_iter and bi_io_vec but not bi_vcnt,
>>> so blk_recount_segments would report bi_phys_segments as 0.
>>> Since rq->nr_phys_segments is 0 as well, the checks to ensure
>>> that we don't exceed the queue's segment limit end up allowing
>>> more bios (and segments) to attach the a request until we finally
>>> map it. That also means we pass the BUG_ON at the beginning of
>>> virtio_queue_rq, ultimately causing memory corruption and a
>>> crash.
>>>
>>> If we copy bi_vcnt in __bio_clone_fast, the bios and requests
>>> properly report the number of segments and everything works as
>>> expected.
>>>
>>> Originally reported at
>>> http://bugzilla.opensuse.org/show_bug.cgi?id=888259
>>
>> Hi, Jeff,
>>
>> Did you manage to reproduce this problem with commit 0738854
>> (blk-merge: fix blk_recount_segments) applied?  Or perhaps with
>> commit 200612e (dm table: propagate QUEUE_FLAG_NO_SG_MERGE)?
>
> Yep. I was able to reproduce it with 3.17. I did try 0738854 when I
> was still using 3.16 since it looked like a good candidate. Neither of
> those patches affect the problem here. bio->bi_phys_segments never
> gets a value set in the fast clone case and that translates to
> req->nr_phys_segments never getting properly accumulated. That might
> not be a problem except that the NO_SG_MERGE behavior bypasses the
> iteration that would come up with the correct value. In either case,

This patch may get incorrect rq->nr_phys_segments because
bio cloning is often used in case of I/O splitting, so could you
test if the attached patch fixes your problem?

Thanks,
--
Ming Lei

View attachment "0001-blk-merge-don-t-compute-bi_phys_segments-from-bi_vcn.patch" of type "text/x-patch" (913 bytes)