From patchwork Fri Feb 10 20:15:11 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Omar Sandoval X-Patchwork-Id: 9567429 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 C2C1860216 for ; Fri, 10 Feb 2017 20:17:17 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B01002853F for ; Fri, 10 Feb 2017 20:17:17 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A17D228545; Fri, 10 Feb 2017 20:17:17 +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.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,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 E2DA02853F for ; Fri, 10 Feb 2017 20:17:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753282AbdBJURO (ORCPT ); Fri, 10 Feb 2017 15:17:14 -0500 Received: from mail-pg0-f54.google.com ([74.125.83.54]:35341 "EHLO mail-pg0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752596AbdBJURN (ORCPT ); Fri, 10 Feb 2017 15:17:13 -0500 Received: by mail-pg0-f54.google.com with SMTP id 194so13153387pgd.2 for ; Fri, 10 Feb 2017 12:16:03 -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=jNKDtks14tGY+uA8GFdlEKuuYAj46myX4pP4o8J8+Nk=; b=qvN66b6W03B8CuwpA7IUwSTJFTQbUBsHXq54XX+d9LtUFWJygrTndITG7wRKsZnHWp kV5QhR/BLL2IYerhQqmoIwpdacd/YOY7dvwl79YcsYHNYkbFyZHCJXvYymZ5ctyilh0G e6Qez+eHXCoyzHFHRT70Zh4udjgG31QHP/8jxfQDp2+mNa0l6/iSYiMeocYnuiu62K4N wkpHmd4xKGPU9bLQ//ZnnGIZmwSNMJvHQ+yS3fCC6dpN0+6+6v40tOz/xuXWCBUmeJdQ 6yrrfVneE3kNShh4yUmFEJB++Z6NFXtMko4VwhGTwFIXR4Y2HxXZr9frqoBjRtWzIlda mL4Q== 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=jNKDtks14tGY+uA8GFdlEKuuYAj46myX4pP4o8J8+Nk=; b=tSwReQwDELNorlPY8urJRnMnUkmPHMtsPR17XricEPo8mq/Q9LuoPajoRqH0wVBa8d bkFBtAXc0FqVhr8Yl+W8WYNP/afmrxgG+bIY48LYCWY+yb3g6qw3kbK5HgSWr9WJHCbZ A6Um417KUz5hEmZEgUu2CKXfT5VvaoZiUZmxK16xbLUWSJNVtsm4ekUrurGRuPYuhqJ9 B4sB8rRcJ9VV08m8boHl95r0IxG+nmiF8G8uQJNpke3hXq22r66meEayd9iQ8mOIAloJ xdGiKlcFkJm9kQed76qDu0QsJUCZloWG0DBtEZtnoFdXdBCisi0CLMTAy9Sapq8HUkQl LZxg== X-Gm-Message-State: AMke39lSytWL+jB4Km85o58Zp2k5K/qMFQEHm2goguhkFr8dt419tNfXNmgJqRxNHil3BEcw X-Received: by 10.99.154.9 with SMTP id o9mr12870271pge.69.1486757762407; Fri, 10 Feb 2017 12:16:02 -0800 (PST) Received: from vader.thefacebook.com ([2620:10d:c090:200::a:8eff]) by smtp.gmail.com with ESMTPSA id u14sm7255868pfg.18.2017.02.10.12.16.01 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 10 Feb 2017 12:16:01 -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] Btrfs: fix btrfs_decompress_buf2page() Date: Fri, 10 Feb 2017 12:15:11 -0800 Message-Id: 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. Fixes: 974b1adc3b10 ("btrfs: use bio iterators for the decompression handlers") Reported-by: Pat Erley Signed-off-by: Omar Sandoval Reviewed-by: Liu Bo Tested-by: Liu Bo --- 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. fs/btrfs/compression.c | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c index 7f390849343b..f9f22976d77d 100644 --- a/fs/btrfs/compression.c +++ b/fs/btrfs/compression.c @@ -1072,25 +1072,27 @@ int btrfs_decompress_buf2page(char *buf, unsigned long buf_start, return 0; bvec = bio_iter_iovec(bio, bio->bi_iter); - start_byte = page_offset(bvec.bv_page) - disk_start; + if (bvec.bv_offset == 0) { + 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; + /* + * 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; + } } }