linux-kernel - Significant brokenness in DIO loopback path

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-next>] [day] [month] [year] [list]

Message-ID: <YilX4PHgulMi3vhb@moria.home.lan>
Date:   Wed, 9 Mar 2022 20:44:00 -0500
From:   Kent Overstreet <kent.overstreet@...il.com>
To:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        Pavel Begunkov <asml.silence@...il.com>
Cc:     Christoph Hellwig <hch@....de>, Ming Lei <ming.lei@...hat.com>,
        Jens Axboe <axboe@...nel.dk>, Al Viro <viro@...iv.linux.org.uk>
Subject: Significant brokenness in DIO loopback path

So I'm testing bcachefs with the loopback driver in dio mode, and noticing
_significant_ brokenness in the bio_iov_iter_get_pages() path and elsewhere.

1) We don't check that we're not asking for more pages than we're in the
original bio

Noticed this because of another bug:

2) the loopback driver appears to never look at the underlying filesystem's
block size, meaning if the filesystem advertises a block size of 4k the loopback
device's blocksize will still be 512, and we'll end up issuing IOs the DIO path
shouldn't allow due to alignment.

3) iov_iter_bvec_advance() looks like utter nonsense. We're synthesizing a fake
bvec_iter and never using or even looking at one from the original bio, looking
at the construction in iov_iter_bvec().

This is broken; you're assuming you're never going to see bios with partially
completed bvec_iters, or things are going to explode.

Try putting a md raid0 on top of two loopback devices with a sub page block
size, things are just going to explode.

iov_iter_bvec() needs to be changed to take a bio, not a bvec array, and
iov_iter_bvec_advance() should probably just call bio_advance() - and
bio_iov_bvec_set() needs to be changed to just copy bi_iter from the original
bio into the dest bio. You guys made this way more complicated than it needed to
be.