From patchwork Fri Feb 10 23:03:35 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Omar Sandoval X-Patchwork-Id: 9567593 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 0FF0E602B6 for ; Fri, 10 Feb 2017 23:10:54 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 00342285F0 for ; Fri, 10 Feb 2017 23:10:54 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E9331285F3; Fri, 10 Feb 2017 23:10:53 +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.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID 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 35E8B285F0 for ; Fri, 10 Feb 2017 23:10:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751174AbdBJXKv (ORCPT ); Fri, 10 Feb 2017 18:10:51 -0500 Received: from mail-pg0-f47.google.com ([74.125.83.47]:36536 "EHLO mail-pg0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750903AbdBJXKu (ORCPT ); Fri, 10 Feb 2017 18:10:50 -0500 Received: by mail-pg0-f47.google.com with SMTP id v184so14147258pgv.3 for ; Fri, 10 Feb 2017 15:10:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=osandov-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id; bh=eN/Jjd79KzCtVLGCEnuh16Or/IQCD00sKxo6LNHBbz0=; b=kVHBz2NKjJZNizl5frO3in6bCRIsBA7DZeYvjNxW5pMBk0CbRerEBQfcV0Vr2ptTET iCHcsINtTElLZVFnrUMBCrF/NF/gxjKv7wIboxXq3MngbrdOh/zsq2Y8Bby1JU3Eg9HZ KVIg+gX69w6hR/NHIGLDCSxl7v4a2LE/QiLnLDNl9PB475eSv5bh1wACafqrs0WD6Wwl Pz+oNc9SsOln9d14ar9wcaH9/LMBL9DBPm1cJrV+lE6eUh9P5FF72Uahnyo4nZOxNywE xLklWmye05lzostj5ad4bj2p+IIevsVLlTeM1ybmlvFfGWIms/K5M65Fyf7BmRcut25f Wouw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=eN/Jjd79KzCtVLGCEnuh16Or/IQCD00sKxo6LNHBbz0=; b=RqYetkwpl4Bp4t8sP8yAOOjGFCci6gsGQm5eme6kinvjkLfqbcQgNVgR6ghYxmIGZc ZRZlkdU1WFgO6vbh62ESGwt/M4TNSraDivLh7HHcmqbeLHLeoZHD4k/IjtVZMK7Pn59b QzHowTKmdKdiJjGKorNizWeo5Sd3gX/4nbRisii3PaNi3eSknAXyspEsgzZ4NbvfXLFZ NDZZbpiUhB6DXe7nEYftiyl35pQ+tj6UiPsgjsSJK2VG6ZGrhyAMskvCV0xr7yZh7BCu 3+y6Yc8dsn2fjuDDRDkT7f9MIauCIr67bR2Cn6NwrjlEVItX6eUf+/rSb42t2w2KH6hx 8XKA== X-Gm-Message-State: AMke39lMBwMmzvv/JyWt9dQifB823B3jN6ddzSqUvIvCPb/W1crMf5ytS71T6fExNUhHXFvO X-Received: by 10.84.138.165 with SMTP id 34mr14597579plp.37.1486767856682; Fri, 10 Feb 2017 15:04:16 -0800 (PST) Received: from vader.thefacebook.com ([2620:10d:c090:200::a:8eff]) by smtp.gmail.com with ESMTPSA id q80sm7537101pfi.13.2017.02.10.15.04.15 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 10 Feb 2017 15:04:16 -0800 (PST) From: Omar Sandoval To: Chris Mason , linux-btrfs@vger.kernel.org Cc: Liu Bo , Christoph Hellwig , Pat Erley , kernel-team@fb.com Subject: [PATCH v2] Btrfs: fix btrfs_decompress_buf2page() Date: Fri, 10 Feb 2017 15:03:35 -0800 Message-Id: <35a7eac2a55a330a27639e0b38f8d052e6dbee8b.1486767707.git.osandov@fb.com> X-Mailer: git-send-email 2.11.1 Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Omar Sandoval If btrfs_decompress_buf2page() is handed a bio with its page in the middle of the working buffer, then we adjust the offset into the working buffer. After we copy into the bio, we advance the iterator by the number of bytes we copied. Then, we have some logic to handle the case of discontiguous pages and adjust the offset into the working buffer again. However, if we didn't advance the bio to a new page, we may enter this case in error, essentially repeating the adjustment that we already made when we entered the function. The end result is bogus data in the bio. Previously, we only checked for this case when we advanced to a new page, but the conversion to bio iterators changed that. This restores the old, correct behavior. A case I saw when testing with zlib was: buf_start = 42769 total_out = 46865 working_bytes = total_out - buf_start = 4096 start_byte = 45056 The condition (total_out > start_byte && buf_start < start_byte) is true, so we adjust the offset: buf_offset = start_byte - buf_start = 2287 working_bytes -= buf_offset = 1809 current_buf_start = buf_start = 42769 Then, we copy bytes = min(bvec.bv_len, PAGE_SIZE - buf_offset, working_bytes) = 1809 buf_offset += bytes = 4096 working_bytes -= bytes = 0 current_buf_start += bytes = 44578 After bio_advance(), we are still in the same page, so start_byte is the same. Then, we check (total_out > start_byte && current_buf_start < start_byte), which is true! So, we adjust the values again: buf_offset = start_byte - buf_start = 2287 working_bytes = total_out - start_byte = 1809 current_buf_start = buf_start + buf_offset = 45056 But note that working_bytes was already zero before this, so we should have stopped copying. Fixes: 974b1adc3b10 ("btrfs: use bio iterators for the decompression handlers") Reported-by: Pat Erley Reviewed-by: Chris Mason Signed-off-by: Omar Sandoval --- Changed from v1: - Check if start_byte changed instead of whether bv_offset == 0. This is a little more foolproof. fs/btrfs/compression.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c index 7f390849343b..25168852ccde 100644 --- a/fs/btrfs/compression.c +++ b/fs/btrfs/compression.c @@ -1024,6 +1024,7 @@ int btrfs_decompress_buf2page(char *buf, unsigned long buf_start, unsigned long buf_offset; unsigned long current_buf_start; unsigned long start_byte; + unsigned long prev_start_byte; unsigned long working_bytes = total_out - buf_start; unsigned long bytes; char *kaddr; @@ -1071,26 +1072,28 @@ int btrfs_decompress_buf2page(char *buf, unsigned long buf_start, if (!bio->bi_iter.bi_size) return 0; bvec = bio_iter_iovec(bio, bio->bi_iter); - + prev_start_byte = start_byte; start_byte = page_offset(bvec.bv_page) - disk_start; - /* - * make sure our new page is covered by this - * working buffer - */ - if (total_out <= start_byte) - return 1; + if (start_byte != prev_start_byte) { + /* + * make sure our new page is covered by this + * working buffer + */ + if (total_out <= start_byte) + return 1; - /* - * the next page in the biovec might not be adjacent - * to the last page, but it might still be found - * inside this working buffer. bump our offset pointer - */ - if (total_out > start_byte && - current_buf_start < start_byte) { - buf_offset = start_byte - buf_start; - working_bytes = total_out - start_byte; - current_buf_start = buf_start + buf_offset; + /* + * the next page in the biovec might not be adjacent + * to the last page, but it might still be found + * inside this working buffer. bump our offset pointer + */ + if (total_out > start_byte && + current_buf_start < start_byte) { + buf_offset = start_byte - buf_start; + working_bytes = total_out - start_byte; + current_buf_start = buf_start + buf_offset; + } } }