From patchwork Tue Aug 15 00:01:09 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mikulas Patocka X-Patchwork-Id: 9900405 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A84D46038C for ; Tue, 15 Aug 2017 00:01:16 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9A9762877E for ; Tue, 15 Aug 2017 00:01:16 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8F1DF28781; Tue, 15 Aug 2017 00:01:16 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F24862877E for ; Tue, 15 Aug 2017 00:01:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752744AbdHOABP (ORCPT ); Mon, 14 Aug 2017 20:01:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49058 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752740AbdHOABP (ORCPT ); Mon, 14 Aug 2017 20:01:15 -0400 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E293DC05A1D9; Tue, 15 Aug 2017 00:01:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E293DC05A1D9 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=mpatocka@redhat.com Received: from file01.intranet.prod.int.rdu2.redhat.com (file01.intranet.prod.int.rdu2.redhat.com [10.11.5.7]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 192F8600CC; Tue, 15 Aug 2017 00:01:13 +0000 (UTC) Received: from file01.intranet.prod.int.rdu2.redhat.com (localhost [127.0.0.1]) by file01.intranet.prod.int.rdu2.redhat.com (8.14.4/8.14.4) with ESMTP id v7F01CEw003485; Mon, 14 Aug 2017 20:01:12 -0400 Received: from localhost (mpatocka@localhost) by file01.intranet.prod.int.rdu2.redhat.com (8.14.4/8.14.4/Submit) with ESMTP id v7F0192r003481; Mon, 14 Aug 2017 20:01:10 -0400 X-Authentication-Warning: file01.intranet.prod.int.rdu2.redhat.com: mpatocka owned process doing -bs Date: Mon, 14 Aug 2017 20:01:09 -0400 (EDT) From: Mikulas Patocka X-X-Sender: mpatocka@file01.intranet.prod.int.rdu2.redhat.com To: Damien Le Moal cc: "hch@lst.de" , "tom.ty89@gmail.com" , "dm-devel@redhat.com" , "linux-block@vger.kernel.org" , "gmazyland@gmail.com" , "axboe@fb.com" , "axboe@kernel.dk" Subject: [PATCH] fix an integer overflow in __blkdev_sectors_to_bio_pages In-Reply-To: <1502683489.903.1.camel@wdc.com> Message-ID: References: <20170809133833.GA25681@lst.de> <1502683489.903.1.camel@wdc.com> User-Agent: Alpine 2.02 (LRH 1266 2009-07-14) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Tue, 15 Aug 2017 00:01:15 +0000 (UTC) Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, 14 Aug 2017, Damien Le Moal wrote: > On Sun, 2017-08-13 at 22:47 -0400, Mikulas Patocka wrote: > > > > On Wed, 9 Aug 2017, hch@lst.de wrote: > > > > > Does commit 615d22a51c04856efe62af6e1d5b450aaf5cc2c0 > > > "block: Fix __blkdev_issue_zeroout loop" fix the issue for you? > > > > > > -- > > > dm-devel mailing list > > > dm-devel@redhat.com > > > https://www.redhat.com/mailman/listinfo/dm-devel > > > > I think that patch is incorrect. sector_t may be a 32-bit type and > > nr_sects << 9 may overflow. > > > > static unsigned int __blkdev_sectors_to_bio_pages(sector_t nr_sects) > > { > > sector_t bytes = (nr_sects << 9) + PAGE_SIZE - 1; > > > > return min(bytes >> PAGE_SHIFT, (sector_t)BIO_MAX_PAGES); > > } > > > > Mikulas > > Mikulas, > > Does the follwing patch fix the problem ? > > From 947b3cf41e759b2b23f684e215e651d0c8037f88 Mon Sep 17 00:00:00 2001 > From: Damien Le Moal > Date: Mon, 14 Aug 2017 13:01:16 +0900 > Subject: [PATCH] block: Fix __blkdev_sectors_to_bio_pages() > > On 32bit systems where sector_t is a 32bits type, the calculation of > bytes may overflow. Use the u64 type for the local calculation to avoid > overflows. > > Signed-off-by: Damien Le Moal > --- > block/blk-lib.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/block/blk-lib.c b/block/blk-lib.c > index 3fe0aec90597..ccf22dba21f0 100644 > --- a/block/blk-lib.c > +++ b/block/blk-lib.c > @@ -269,9 +269,9 @@ static int __blkdev_issue_write_zeroes(struct block_device > *bdev, > */ > static unsigned int __blkdev_sectors_to_bio_pages(sector_t nr_sects) > { > - sector_t bytes = (nr_sects << 9) + PAGE_SIZE - 1; > + u64 bytes = ((u64)nr_sects << 9) + PAGE_SIZE - 1; > > - return min(bytes >> PAGE_SHIFT, (sector_t)BIO_MAX_PAGES); > + return min(bytes >> PAGE_SHIFT, (u64)BIO_MAX_PAGES); > } > It's OK, but it is not needed to use 64-bit arithmetic here if all we need is to shift the value right. Here I submit a simplified patch, using the macro DIV_ROUND_UP_SECTOR_T (the macro gets optimized to just an addition and right shift). From: Mikulas Patocka Fix possible integer overflow in __blkdev_sectors_to_bio_pages if sector_t is 32-bit. Signed-off-by: Mikulas Patocka Fixes: 615d22a51c04 ("block: Fix __blkdev_issue_zeroout loop") Reviewed-by: Damien Le Moal --- block/blk-lib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Index: linux-2.6/block/blk-lib.c =================================================================== --- linux-2.6.orig/block/blk-lib.c +++ linux-2.6/block/blk-lib.c @@ -269,9 +269,9 @@ static int __blkdev_issue_write_zeroes(s */ static unsigned int __blkdev_sectors_to_bio_pages(sector_t nr_sects) { - sector_t bytes = (nr_sects << 9) + PAGE_SIZE - 1; + sector_t pages = DIV_ROUND_UP_SECTOR_T(nr_sects, PAGE_SIZE / 512); - return min(bytes >> PAGE_SHIFT, (sector_t)BIO_MAX_PAGES); + return min(pages, (sector_t)BIO_MAX_PAGES); } /**