stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Filipe Manana <fdmanana@suse.com>,
	Chris Mason <clm@fb.com>
Subject: [PATCH 3.10 39/55] Btrfs: fix csum tree corruption, duplicate and outdated checksums
Date: Wed,  3 Sep 2014 15:05:29 -0700	[thread overview]
Message-ID: <20140903220500.230486932@linuxfoundation.org> (raw)
In-Reply-To: <20140903220453.653576908@linuxfoundation.org>

3.10-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Filipe Manana <fdmanana@suse.com>

commit 27b9a8122ff71a8cadfbffb9c4f0694300464f3b upstream.

Under rare circumstances we can end up leaving 2 versions of a checksum
for the same file extent range.

The reason for this is that after calling btrfs_next_leaf we process
slot 0 of the leaf it returns, instead of processing the slot set in
path->slots[0]. Most of the time (by far) path->slots[0] is 0, but after
btrfs_next_leaf() releases the path and before it searches for the next
leaf, another task might cause a split of the next leaf, which migrates
some of its keys to the leaf we were processing before calling
btrfs_next_leaf(). In this case btrfs_next_leaf() returns again the
same leaf but with path->slots[0] having a slot number corresponding
to the first new key it got, that is, a slot number that didn't exist
before calling btrfs_next_leaf(), as the leaf now has more keys than
it had before. So we must really process the returned leaf starting at
path->slots[0] always, as it isn't always 0, and the key at slot 0 can
have an offset much lower than our search offset/bytenr.

For example, consider the following scenario, where we have:

sums->bytenr: 40157184, sums->len: 16384, sums end: 40173568
four 4kb file data blocks with offsets 40157184, 40161280, 40165376, 40169472

  Leaf N:

    slot = 0                           slot = btrfs_header_nritems() - 1
  |-------------------------------------------------------------------|
  | [(CSUM CSUM 39239680), size 8] ... [(CSUM CSUM 40116224), size 4] |
  |-------------------------------------------------------------------|

  Leaf N + 1:

      slot = 0                          slot = btrfs_header_nritems() - 1
  |--------------------------------------------------------------------|
  | [(CSUM CSUM 40161280), size 32] ... [((CSUM CSUM 40615936), size 8 |
  |--------------------------------------------------------------------|

Because we are at the last slot of leaf N, we call btrfs_next_leaf() to
find the next highest key, which releases the current path and then searches
for that next key. However after releasing the path and before finding that
next key, the item at slot 0 of leaf N + 1 gets moved to leaf N, due to a call
to ctree.c:push_leaf_left() (via ctree.c:split_leaf()), and therefore
btrfs_next_leaf() will returns us a path again with leaf N but with the slot
pointing to its new last key (CSUM CSUM 40161280). This new version of leaf N
is then:

    slot = 0                        slot = btrfs_header_nritems() - 2  slot = btrfs_header_nritems() - 1
  |----------------------------------------------------------------------------------------------------|
  | [(CSUM CSUM 39239680), size 8] ... [(CSUM CSUM 40116224), size 4]  [(CSUM CSUM 40161280), size 32] |
  |----------------------------------------------------------------------------------------------------|

And incorrecly using slot 0, makes us set next_offset to 39239680 and we jump
into the "insert:" label, which will set tmp to:

    tmp = min((sums->len - total_bytes) >> blocksize_bits,
        (next_offset - file_key.offset) >> blocksize_bits) =
    min((16384 - 0) >> 12, (39239680 - 40157184) >> 12) =
    min(4, (u64)-917504 = 18446744073708634112 >> 12) = 4

and

   ins_size = csum_size * tmp = 4 * 4 = 16 bytes.

In other words, we insert a new csum item in the tree with key
(CSUM_OBJECTID CSUM_KEY 40157184 = sums->bytenr) that contains the checksums
for all the data (4 blocks of 4096 bytes each = sums->len). Which is wrong,
because the item with key (CSUM CSUM 40161280) (the one that was moved from
leaf N + 1 to the end of leaf N) contains the old checksums of the last 12288
bytes of our data and won't get those old checksums removed.

So this leaves us 2 different checksums for 3 4kb blocks of data in the tree,
and breaks the logical rule:

   Key_N+1.offset >= Key_N.offset + length_of_data_its_checksums_cover

An obvious bad effect of this is that a subsequent csum tree lookup to get
the checksum of any of the blocks with logical offset of 40161280, 40165376
or 40169472 (the last 3 4kb blocks of file data), will get the old checksums.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Chris Mason <clm@fb.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 fs/btrfs/file-item.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -754,7 +754,7 @@ again:
 				found_next = 1;
 			if (ret != 0)
 				goto insert;
-			slot = 0;
+			slot = path->slots[0];
 		}
 		btrfs_item_key_to_cpu(path->nodes[0], &found_key, slot);
 		if (found_key.objectid != BTRFS_EXTENT_CSUM_OBJECTID ||



  parent reply	other threads:[~2014-09-03 22:05 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-03 22:04 [PATCH 3.10 00/55] 3.10.54-stable review Greg Kroah-Hartman
2014-09-03 22:04 ` [PATCH 3.10 01/55] stable_kernel_rules: Add pointer to netdev-FAQ for network patches Greg Kroah-Hartman
2014-09-03 22:04 ` [PATCH 3.10 02/55] HID: logitech: perform bounds checking on device_id early enough Greg Kroah-Hartman
2014-09-03 22:04 ` [PATCH 3.10 03/55] HID: fix a couple of off-by-ones Greg Kroah-Hartman
2014-09-03 22:04 ` [PATCH 3.10 04/55] isofs: Fix unbounded recursion when processing relocated directories Greg Kroah-Hartman
2014-09-03 22:04 ` [PATCH 3.10 05/55] USB: OHCI: dont lose track of EDs when a controller dies Greg Kroah-Hartman
2014-09-03 22:04 ` [PATCH 3.10 06/55] USB: serial: ftdi_sio: Annotate the current Xsens PID assignments Greg Kroah-Hartman
2014-09-03 22:04 ` [PATCH 3.10 07/55] USB: serial: ftdi_sio: Add support for new Xsens devices Greg Kroah-Hartman
2014-09-03 22:04 ` [PATCH 3.10 08/55] USB: ehci-pci: USB host controller support for Intel Quark X1000 Greg Kroah-Hartman
2014-09-03 22:04 ` [PATCH 3.10 09/55] USB: Fix persist resume of some SS USB devices Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 10/55] ALSA: hda - fix an external mic jack problem on a HP machine Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 11/55] ALSA: virtuoso: add Xonar Essence STX II support Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 12/55] ALSA: hda/ca0132 - Dont try loading firmware at resume when already failed Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 13/55] ALSA: hda/realtek - Avoid setting wrong COEF on ALC269 & co Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 14/55] mei: start disconnect request timer consistently Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 16/55] drm: omapdrm: fix compiler errors Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 17/55] hwmon: (sis5595) Prevent overflow problem when writing large limits Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 18/55] hwmon: (lm78) Fix overflow problems seen when writing large temperature limits Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 19/55] hwmon: (gpio-fan) Prevent overflow problem when writing large limits Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 20/55] hwmon: (ads1015) Fix off-by-one for valid channel index checking Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 21/55] hwmon: (lm85) Fix various errors on attribute writes Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 22/55] hwmon: (ads1015) Fix out-of-bounds array access Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 23/55] hwmon: (dme1737) Prevent overflow problem when writing large limits Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 24/55] drivers/i2c/busses: use correct type for dma_map/unmap Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 25/55] ext4: fix ext4_discard_allocated_blocks() if we cant allocate the pa struct Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 26/55] serial: core: Preserve termios c_cflag for console resume Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 27/55] crypto: ux500 - make interrupt mode plausible Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 28/55] KVM: x86: Inter-privilege level ret emulation is not implemeneted Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 29/55] KVM: x86: always exit on EOIs for interrupts listed in the IOAPIC redir table Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 30/55] KVM: nVMX: fix "acknowledge interrupt on exit" when APICv is in use Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 31/55] Revert "KVM: x86: Increase the number of fixed MTRR regs to 10" Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 32/55] kvm: iommu: fix the third parameter of kvm_iommu_put_pages (CVE-2014-3601) Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 33/55] ext4: fix BUG_ON in mb_free_blocks() Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 34/55] drm/radeon: add additional SI pci ids Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 35/55] x86: dont exclude low BIOS area when allocating address space for non-PCI cards Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 36/55] x86_64/vsyscall: Fix warn_bad_vsyscall log output Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 38/55] hpsa: fix bad -ENOMEM return value in hpsa_big_passthru_ioctl Greg Kroah-Hartman
2014-09-03 22:05 ` Greg Kroah-Hartman [this message]
2014-09-03 22:05 ` [PATCH 3.10 40/55] mei: reset client state on queued connect request Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 41/55] mei: nfc: fix memory leak in error path Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 42/55] jbd2: fix infinite loop when recovering corrupt journal blocks Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 43/55] Staging: speakup: Update __speakup_paste_selection() tty (ab)usage to match vt Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 44/55] xhci: Treat not finding the event_seg on COMP_STOP the same as COMP_STOP_INVAL Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 45/55] usb: xhci: amd chipset also needs short TX quirk Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 46/55] ARM: OMAP2+: hwmod: Rearm wake-up interrupts for DT when MUSB is idled Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 47/55] USB: ftdi_sio: add Basic Micro ATOM Nano USB2Serial PID Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 49/55] USB: whiteheat: Added bounds checking for bulk command response Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 50/55] usb: hub: Prevent hub autosuspend if usbcore.autosuspend is -1 Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 51/55] NFSD: Decrease nfsd_users in nfsd_startup_generic fail Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 52/55] svcrdma: Select NFSv4.1 backchannel transport based on forward channel Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 53/55] NFSv3: Fix another acl regression Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 54/55] NFSv4: Fix problems with close in the presence of a delegation Greg Kroah-Hartman
2014-09-03 22:05 ` [PATCH 3.10 55/55] vm_is_stack: use for_each_thread() rather then buggy while_each_thread() Greg Kroah-Hartman
2014-09-03 22:41 ` [PATCH 3.10 00/55] 3.10.54-stable review Greg Kroah-Hartman
2014-09-03 23:44 ` Greg Kroah-Hartman
2014-09-04  4:50   ` Guenter Roeck
2014-09-04 14:02     ` Greg Kroah-Hartman
2014-09-04 13:36 ` Shuah Khan
2014-09-04 13:52   ` Usyskin, Alexander

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140903220500.230486932@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=clm@fb.com \
    --cc=fdmanana@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).