From patchwork Mon Mar 21 15:00:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Mason X-Patchwork-Id: 8633761 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 0CDA4C0553 for ; Mon, 21 Mar 2016 15:00:31 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 167B720328 for ; Mon, 21 Mar 2016 15:00:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4294C200CA for ; Mon, 21 Mar 2016 15:00:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756195AbcCUPAY (ORCPT ); Mon, 21 Mar 2016 11:00:24 -0400 Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:1793 "EHLO mx0b-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755715AbcCUPAW (ORCPT ); Mon, 21 Mar 2016 11:00:22 -0400 Received: from pps.filterd (m0001303.ppops.net [127.0.0.1]) by m0001303.ppops.net (8.16.0.11/8.16.0.11) with SMTP id u2LEwo4J029411; Mon, 21 Mar 2016 08:00:18 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=fb.com; h=date : from : to : subject : message-id : mime-version : content-type; s=facebook; bh=tb4vIg7nRXmlg0ViOoDw8/8WM4YVUfGpL42/4RYjp3w=; b=E+JFq4rUkO2CqtQxE84A8mrs4Hs32K1vbGuK3nQoQNoAKS07Ru0n7Hga5ImblMe4dY/l A6GfAlEB1NIG+yUo5X7VwKFt/r0tvOEi5R4Y6apKPagEPfKZVDUZossUE773zBAoclg9 a18gMH7cKesPuGHQK/zxgE6M+LpnlyFKzNw= Received: from mail.thefacebook.com ([199.201.64.23]) by m0001303.ppops.net with ESMTP id 21tfdgh4rc-1 (version=TLSv1 cipher=AES128-SHA bits=128 verify=NOT); Mon, 21 Mar 2016 08:00:18 -0700 Received: from localhost (192.168.52.123) by mail.thefacebook.com (192.168.16.21) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 21 Mar 2016 08:00:16 -0700 Date: Mon, 21 Mar 2016 11:00:14 -0400 From: Chris Mason To: linux-btrfs , Chandan Rajendra Subject: [PATCH] btrfs: make sure we stay inside the bvec during __btrfs_lookup_bio_sums Message-ID: <20160321150014.hod3ktlnpaxbx2u7@floor.thefacebook.com> Mail-Followup-To: Chris Mason , linux-btrfs , Chandan Rajendra MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.23.1 (2014-03-12) X-Originating-IP: [192.168.52.123] X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-03-21_07:, , signatures=0 Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi everyone, I realized last week that CONFIG_DEBUG_PAGEALLOC had dropped out of my config, and hit a crash inside __btrfs_lookup_bio_sums once I enabled it again. It's hard for this bug to cause problems because Chandan's inner loop is always done at the same time the outer loop is done. Without my goto, it's just exiting normally, but only after reading bvec->bv_len (which isn't valid). I have this on top of my integration-4.6. Once things pass I'll send a pull later today or Tuesday morning: Commit c40a3d38aff4e1c (Btrfs: Compute and look up csums based on sectorsized blocks) changes around how we walk the bios while looking up crcs. There's an inner loop that is jumping to the next bvec based on sectors and before it derefs the next bvec, it needs to make sure we're still in the bio. In this case, the outer loop would have decided to stop moving forward too, and the bvec deref is never actually used for anything. But CONFIG_DEBUG_PAGEALLOC catches it because we're outside our bio. Signed-off-by: Chris Mason Reviewed-by: David Sterba Reviewed-by: Chandan Rajendra --- fs/btrfs/file-item.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c index 763fd17..b5baf5b 100644 --- a/fs/btrfs/file-item.c +++ b/fs/btrfs/file-item.c @@ -292,12 +292,22 @@ found: page_bytes_left -= root->sectorsize; if (!page_bytes_left) { bio_index++; + /* + * make sure we're still inside the + * bio before we update page_bytes_left + */ + if (bio_index >= bio->bi_vcnt) { + WARN_ON_ONCE(count); + goto done; + } bvec++; page_bytes_left = bvec->bv_len; } } } + +done: btrfs_free_path(path); return 0; }