drbd: fix potential data corruption and protocol error
authorLars Ellenberg <lars.ellenberg@linbit.com>
Thu, 8 Mar 2012 15:43:45 +0000 (16:43 +0100)
committerPhilipp Reisner <philipp.reisner@linbit.com>
Wed, 9 May 2012 13:16:39 +0000 (15:16 +0200)
We assumed only bios with bi_idx == 0 would end up
in drbd_make_request().

That is wrong.

At least device mapper, in __clone_and_map(), may submit
clones only covering a partial bio, but sharing
the original bvec, by adjusting bi_idx and relevant
other bio members of the clone.

We used __bio_for_each_segment() in various places,
even though that is documented as
 * drivers should not use the __ version unless they _really_ want to
 * run through the entire bio and not just pending pieces

Impact: we would send the full bio bvec, even for the clone
with bi_idx > 0, which will cause data corruption on the
peer (because we submit wrong data at the clone offset),
and will cause a DRBD protocol error, disconnect/reconnect
and resync (thus fixing the corruption),
because the next package header would be expected right
in the middle of the sent data, causing DRBD magic mismatch.

Fix: drop the assert, and use bio_for_each_segment()
instead of the __ version.

Conflicts:

drbd/drbd_tracing.c

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>
drivers/block/drbd/drbd_main.c
drivers/block/drbd/drbd_req.c
drivers/block/drbd/drbd_worker.c

index 8ca8925520ad866220ff92706602e57d80c2665c..f6cfc45b862a02a4f1485a804a825d6c49a88964 100644 (file)
@@ -2734,7 +2734,7 @@ static int _drbd_send_bio(struct drbd_conf *mdev, struct bio *bio)
        struct bio_vec *bvec;
        int i;
        /* hint all but last page with MSG_MORE */
-       __bio_for_each_segment(bvec, bio, i, 0) {
+       bio_for_each_segment(bvec, bio, i) {
                if (!_drbd_no_send_page(mdev, bvec->bv_page,
                                     bvec->bv_offset, bvec->bv_len,
                                     i == bio->bi_vcnt -1 ? 0 : MSG_MORE))
@@ -2748,7 +2748,7 @@ static int _drbd_send_zc_bio(struct drbd_conf *mdev, struct bio *bio)
        struct bio_vec *bvec;
        int i;
        /* hint all but last page with MSG_MORE */
-       __bio_for_each_segment(bvec, bio, i, 0) {
+       bio_for_each_segment(bvec, bio, i) {
                if (!_drbd_send_page(mdev, bvec->bv_page,
                                     bvec->bv_offset, bvec->bv_len,
                                     i == bio->bi_vcnt -1 ? 0 : MSG_MORE))
index 4fc7f98d9856c5988a6a48a68f1a76acb090472a..de319ba54241d895e34dc0de568701b1b4f9b3e9 100644 (file)
@@ -1106,7 +1106,6 @@ void drbd_make_request(struct request_queue *q, struct bio *bio)
         */
        D_ASSERT(bio->bi_size > 0);
        D_ASSERT((bio->bi_size & 0x1ff) == 0);
-       D_ASSERT(bio->bi_idx == 0);
 
        /* to make some things easier, force alignment of requests within the
         * granularity of our hash tables */
index 56fd69e38298ab172a5b57869bb63ddbcae8ece3..14aa4a2d4dc439241b58c57fcad039e5475908d9 100644 (file)
@@ -308,7 +308,7 @@ void drbd_csum_bio(struct drbd_conf *mdev, struct crypto_hash *tfm, struct bio *
        sg_init_table(&sg, 1);
        crypto_hash_init(&desc);
 
-       __bio_for_each_segment(bvec, bio, i, 0) {
+       bio_for_each_segment(bvec, bio, i) {
                sg_set_page(&sg, bvec->bv_page, bvec->bv_len, bvec->bv_offset);
                crypto_hash_update(&desc, &sg, sg.length);
        }