From patchwork Mon Oct 1 21:42:41 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kent Overstreet X-Patchwork-Id: 1532721 Return-Path: X-Original-To: patchwork-dm-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from mx4-phx2.redhat.com (mx4-phx2.redhat.com [209.132.183.25]) by patchwork1.kernel.org (Postfix) with ESMTP id 4EA9E3FE1C for ; Mon, 1 Oct 2012 21:46:17 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx4-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q91LhWap026223; Mon, 1 Oct 2012 17:43:34 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q91Lgp1r016414 for ; Mon, 1 Oct 2012 17:42:51 -0400 Received: from mx1.redhat.com (ext-mx14.extmail.prod.ext.phx2.redhat.com [10.5.110.19]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q91Lgkeb008391 for ; Mon, 1 Oct 2012 17:42:46 -0400 Received: from mail-pa0-f46.google.com (mail-pa0-f46.google.com [209.85.220.46]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q91LgieL006968 for ; Mon, 1 Oct 2012 17:42:44 -0400 Received: by padhz1 with SMTP id hz1so4523898pad.33 for ; Mon, 01 Oct 2012 14:42:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=OqKQTRMRuTnDspwoMUQxOPyuoGxkpsrNTZP/KAF54lI=; b=AYgIjjzkJnohP/CKymEo/5a/RyStxUI6O3T2XFZOKftBUCrWbFig81Bp9IK7ivmXIH /mzc4MIpQXInQUM19ZFsS4n6l5Zz4P9jOsgS4BHg1kRJ649huU3S5TYF31abJ7Zu0fZb GHbIEjeK5oXci0Dsr4+K0M+rjBJLLZu7TsUCEhBEXLO3u8VgPbYScY1sHhuXnDXo0KMg 5evaK43w10AiYmlCdEalnRaN7rRLkBglYKoca32/ru81dnVhYHCKLimpBb8/IkOvMetB TcW7H6oW8gtwHv64RfWAxe0z6YehiT51RdHEULsGb5eVV5Kk2upUP8Ul7zsr5XMe89zS YMHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent :x-gm-message-state; bh=OqKQTRMRuTnDspwoMUQxOPyuoGxkpsrNTZP/KAF54lI=; b=BEeZeviNDGB1Wyf6Qmn553xzjktVqCwL2U5lRvn9O5YoeqXEJfHF9whprXHr5Jhzvd Pfbe3sJLoyCjp/8GGlOy/9BSPvuYPt0h1k15GmFgXkURjFA0+lsx/gsee62N7g0nu54y Btj93Z4Zv8sZi/dsjGE4WDKXKLPUKG+cGq7PNYSedktVW3806mXpAG/qZFpGJ74IC3Sg usLQF3loCWdQlv8Dci8rHet9YKmw3qcOwGpzbSQ9BAlOYu49sdnJb8Xe5p85WEyJilQS PhKeMCThj/SzGWIA3U63zjGD2Py1xYa0DFH2cT8z6o1DbpMX37oeVXwUI0l/WslJ8N3b MEXA== Received: by 10.68.197.194 with SMTP id iw2mr43950758pbc.121.1349127764203; Mon, 01 Oct 2012 14:42:44 -0700 (PDT) Received: from google.com ([2620:0:1000:2300:be30:5bff:fed1:fc17]) by mx.google.com with ESMTPS id jw14sm10843421pbb.36.2012.10.01.14.42.42 (version=SSLv3 cipher=OTHER); Mon, 01 Oct 2012 14:42:43 -0700 (PDT) Date: Mon, 1 Oct 2012 14:42:41 -0700 From: Kent Overstreet To: Vivek Goyal Message-ID: <20121001214241.GE26488@google.com> References: <1348526106-17074-1-git-send-email-koverstreet@google.com> <1348526106-17074-2-git-send-email-koverstreet@google.com> <20121001212336.GA17165@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20121001212336.GA17165@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Gm-Message-State: ALoCoQkOgTbtMToZLpKvtm0a640KeirCXyeLjpa0+dU8DgjmJ3jI1hD14UXUlJy6PqNSebheZZveTsGVvXzIv2CjmVoznnpDgKvCc9KggaNavwtZEwlymAsz8s4LxdpoyZy3XBPzxCPGtOxb6YXf0Tn8dJslLUNxgOf90ifXwWOiW4t1RG4t5i8LW3vaQUjLG9wcnqY96ec4 X-RedHat-Spam-Score: -4.845 (BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Scanned-By: MIMEDefang 2.68 on 10.5.110.19 X-loop: dm-devel@redhat.com Cc: axboe@kernel.dk, "Martin K. Petersen" , dm-devel@redhat.com, linux-kernel@vger.kernel.org, linux-bcache@vger.kernel.org, tj@kernel.org Subject: Re: [dm-devel] [PATCH v3 01/26] block: Fix a buffer overrun in bio_integrity_split() X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk Reply-To: device-mapper development List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com On Mon, Oct 01, 2012 at 05:23:36PM -0400, Vivek Goyal wrote: > On Mon, Sep 24, 2012 at 03:34:41PM -0700, Kent Overstreet wrote: > > bio_integrity_split() seemed to be confusing pointers and arrays - > > bip_vec in bio_integrity_payload is an array appended to the end of the > > payload, so the bio_vecs in struct bio_pair need to come immediately > > after the bio_integrity_payload they're for, and there was an assignment > > in bio_integrity_split() that didn't make any sense. > > > > Signed-off-by: Kent Overstreet > > CC: Jens Axboe > > CC: Martin K. Petersen > > --- > > fs/bio-integrity.c | 3 --- > > include/linux/bio.h | 6 ++++-- > > 2 files changed, 4 insertions(+), 5 deletions(-) > > > > diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c > > index a3f28f3..c7b6b52 100644 > > --- a/fs/bio-integrity.c > > +++ b/fs/bio-integrity.c > > @@ -697,9 +697,6 @@ void bio_integrity_split(struct bio *bio, struct bio_pair *bp, int sectors) > > bp->iv1 = bip->bip_vec[0]; > > bp->iv2 = bip->bip_vec[0]; > > > > - bp->bip1.bip_vec[0] = bp->iv1; > > - bp->bip2.bip_vec[0] = bp->iv2; > > - > > bp->iv1.bv_len = sectors * bi->tuple_size; > > bp->iv2.bv_offset += sectors * bi->tuple_size; > > bp->iv2.bv_len -= sectors * bi->tuple_size; > > diff --git a/include/linux/bio.h b/include/linux/bio.h > > index b31036f..8e2d108 100644 > > --- a/include/linux/bio.h > > +++ b/include/linux/bio.h > > @@ -200,8 +200,10 @@ struct bio_pair { > > struct bio bio1, bio2; > > struct bio_vec bv1, bv2; > > #if defined(CONFIG_BLK_DEV_INTEGRITY) > > - struct bio_integrity_payload bip1, bip2; > > - struct bio_vec iv1, iv2; > > + struct bio_integrity_payload bip1; > > + struct bio_vec iv1; > > + struct bio_integrity_payload bip2; > > + struct bio_vec iv2; > > #endif > > I think it probably is a good idea to put a comment here so that we > know that certain elements of structure assume ordering. > > Also I am wondering that what's the gurantee that there are no padding > bytes between bipi1 and iv1 (or bip2 or iv2). I think if there are padding > bytes then the assumption that bio_vec is always following bip will be > broken? Here's the new patch: commit e270c9ca843b5c86d59431b0d7a676b7846946d6 Author: Kent Overstreet Date: Mon Oct 1 14:41:08 2012 -0700 block: Fix a buffer overrun in bio_integrity_split() bio_integrity_split() seemed to be confusing pointers and arrays - bip_vec in bio_integrity_payload is an array appended to the end of the payload, so the bio_vecs in struct bio_pair need to come immediately after the bio_integrity_payload they're for, and there was an assignment in bio_integrity_split() that didn't make any sense. Also, changed bio_integrity_split() to not refer to the bvecs embedded in struct bio_pair, in case there's padding between them and bip->bip_vec. Signed-off-by: Kent Overstreet CC: Jens Axboe CC: Martin K. Petersen --- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c index a3f28f3..4ae22a8 100644 --- a/fs/bio-integrity.c +++ b/fs/bio-integrity.c @@ -694,15 +694,12 @@ void bio_integrity_split(struct bio *bio, struct bio_pair *bp, int sectors) bp->bio1.bi_integrity = &bp->bip1; bp->bio2.bi_integrity = &bp->bip2; - bp->iv1 = bip->bip_vec[0]; - bp->iv2 = bip->bip_vec[0]; + *bp->bip1.bip_vec = bip->bip_vec[0]; + *bp->bip2.bip_vec = bip->bip_vec[0]; - bp->bip1.bip_vec[0] = bp->iv1; - bp->bip2.bip_vec[0] = bp->iv2; - - bp->iv1.bv_len = sectors * bi->tuple_size; - bp->iv2.bv_offset += sectors * bi->tuple_size; - bp->iv2.bv_len -= sectors * bi->tuple_size; + bp->bip1.bip_vec->bv_len = sectors * bi->tuple_size; + bp->bip2.bip_vec->bv_offset += sectors * bi->tuple_size; + bp->bip2.bip_vec->bv_len -= sectors * bi->tuple_size; bp->bip1.bip_sector = bio->bi_integrity->bip_sector; bp->bip2.bip_sector = bio->bi_integrity->bip_sector + nr_sectors; diff --git a/include/linux/bio.h b/include/linux/bio.h index b31036f..8e2d108 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -200,8 +200,10 @@ struct bio_pair { struct bio bio1, bio2; struct bio_vec bv1, bv2; #if defined(CONFIG_BLK_DEV_INTEGRITY) - struct bio_integrity_payload bip1, bip2; - struct bio_vec iv1, iv2; + struct bio_integrity_payload bip1; + struct bio_vec iv1; + struct bio_integrity_payload bip2; + struct bio_vec iv2; #endif atomic_t cnt; int error;