)]}'
{
  "commit": "978b63f7464abcfd364a6c95f734282c50f3decf",
  "tree": "09655300bbc3a18172f63c90efd4bbc42a5f65ea",
  "parents": [
    "ae6bd7f9b46a29af52ebfac25d395757e2031d0d"
  ],
  "author": {
    "name": "Filipe Manana",
    "email": "fdmanana@suse.com",
    "time": "Wed Feb 28 11:37:56 2024 +0000"
  },
  "committer": {
    "name": "David Sterba",
    "email": "dsterba@suse.com",
    "time": "Tue Mar 05 18:12:37 2024 +0100"
  },
  "message": "btrfs: fix race when detecting delalloc ranges during fiemap\n\nFor fiemap we recently stopped locking the target extent range for the\nwhole duration of the fiemap call, in order to avoid a deadlock in a\nscenario where the fiemap buffer happens to be a memory mapped range of\nthe same file. This use case is very unlikely to be useful in practice but\nit may be triggered by fuzz testing (syzbot, etc).\n\nThis however introduced a race that makes us miss delalloc ranges for\nfile regions that are currently holes, so the caller of fiemap will not\nbe aware that there\u0027s data for some file regions. This can be quite\nserious for some use cases - for example in coreutils versions before 9.0,\nthe cp program used fiemap to detect holes and data in the source file,\ncopying only regions with data (extents or delalloc) from the source file\nto the destination file in order to preserve holes (see the documentation\nfor its --sparse command line option). This means that if cp was used\nwith a source file that had delalloc in a hole, the destination file could\nend up without that data, which is effectively a data loss issue, if it\nhappened to hit the race described below.\n\nThe race happens like this:\n\n1) Fiemap is called, without the FIEMAP_FLAG_SYNC flag, for a file that\n   has delalloc in the file range [64M, 65M[, which is currently a hole;\n\n2) Fiemap locks the inode in shared mode, then starts iterating the\n   inode\u0027s subvolume tree searching for file extent items, without having\n   the whole fiemap target range locked in the inode\u0027s io tree - the\n   change introduced recently by commit b0ad381fa769 (\"btrfs: fix\n   deadlock with fiemap and extent locking\"). It only locks ranges in\n   the io tree when it finds a hole or prealloc extent since that\n   commit;\n\n3) Note that fiemap clones each leaf before using it, and this is to\n   avoid deadlocks when locking a file range in the inode\u0027s io tree and\n   the fiemap buffer is memory mapped to some file, because writing\n   to the page with btrfs_page_mkwrite() will wait on any ordered extent\n   for the page\u0027s range and the ordered extent needs to lock the range\n   and may need to modify the same leaf, therefore leading to a deadlock\n   on the leaf;\n\n4) While iterating the file extent items in the cloned leaf before\n   finding the hole in the range [64M, 65M[, the delalloc in that range\n   is flushed and its ordered extent completes - meaning the corresponding\n   file extent item is in the inode\u0027s subvolume tree, but not present in\n   the cloned leaf that fiemap is iterating over;\n\n5) When fiemap finds the hole in the [64M, 65M[ range by seeing the gap in\n   the cloned leaf (or a file extent item with disk_bytenr \u003d\u003d 0 in case\n   the NO_HOLES feature is not enabled), it will lock that file range in\n   the inode\u0027s io tree and then search for delalloc by checking for the\n   EXTENT_DELALLOC bit in the io tree for that range and ordered extents\n   (with btrfs_find_delalloc_in_range()). But it finds nothing since the\n   delalloc in that range was already flushed and the ordered extent\n   completed and is gone - as a result fiemap will not report that there\u0027s\n   delalloc or an extent for the range [64M, 65M[, so user space will be\n   mislead into thinking that there\u0027s a hole in that range.\n\nThis could actually be sporadically triggered with test case generic/094\nfrom fstests, which reports a missing extent/delalloc range like this:\n\n  generic/094 2s ... - output mismatch (see /home/fdmanana/git/hub/xfstests/results//generic/094.out.bad)\n      --- tests/generic/094.out\t2020-06-10 19:29:03.830519425 +0100\n      +++ /home/fdmanana/git/hub/xfstests/results//generic/094.out.bad\t2024-02-28 11:00:00.381071525 +0000\n      @@ -1,3 +1,9 @@\n       QA output created by 094\n       fiemap run with sync\n       fiemap run without sync\n      +ERROR: couldn\u0027t find extent at 7\n      +map is \u0027HHDDHPPDPHPH\u0027\n      +logical: [       5..       6] phys:   301517..  301518 flags: 0x800 tot: 2\n      +logical: [       8..       8] phys:   301520..  301520 flags: 0x800 tot: 1\n      ...\n      (Run \u0027diff -u /home/fdmanana/git/hub/xfstests/tests/generic/094.out /home/fdmanana/git/hub/xfstests/results//generic/094.out.bad\u0027  to see the entire diff)\n\nSo in order to fix this, while still avoiding deadlocks in the case where\nthe fiemap buffer is memory mapped to the same file, change fiemap to work\nlike the following:\n\n1) Always lock the whole range in the inode\u0027s io tree before starting to\n   iterate the inode\u0027s subvolume tree searching for file extent items,\n   just like we did before commit b0ad381fa769 (\"btrfs: fix deadlock with\n   fiemap and extent locking\");\n\n2) Now instead of writing to the fiemap buffer every time we have an extent\n   to report, write instead to a temporary buffer (1 page), and when that\n   buffer becomes full, stop iterating the file extent items, unlock the\n   range in the io tree, release the search path, submit all the entries\n   kept in that buffer to the fiemap buffer, and then resume the search\n   for file extent items after locking again the remainder of the range in\n   the io tree.\n\n   The buffer having a size of a page, allows for 146 entries in a system\n   with 4K pages. This is a large enough value to have a good performance\n   by avoiding too many restarts of the search for file extent items.\n   In other words this preserves the huge performance gains made in the\n   last two years to fiemap, while avoiding the deadlocks in case the\n   fiemap buffer is memory mapped to the same file (useless in practice,\n   but possible and exercised by fuzz testing and syzbot).\n\nFixes: b0ad381fa769 (\"btrfs: fix deadlock with fiemap and extent locking\")\nReviewed-by: Josef Bacik \u003cjosef@toxicpanda.com\u003e\nSigned-off-by: Filipe Manana \u003cfdmanana@suse.com\u003e\nSigned-off-by: David Sterba \u003cdsterba@suse.com\u003e\n",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "e6a2b6eb89e19b9259f17f154e6ed9604202114d",
      "old_mode": 33188,
      "old_path": "fs/btrfs/extent_io.c",
      "new_id": "fbb05b0f7ebc5c6bc75781b12d0cc6a322ea99cc",
      "new_mode": 33188,
      "new_path": "fs/btrfs/extent_io.c"
    }
  ]
}