27 Jul, 2020

1 commit

  • clang static analysis flags this error

    fs/btrfs/ref-verify.c:290:3: warning: Potential leak of memory pointed to by 're' [unix.Malloc]
    kfree(be);
    ^~~~~

    The problem is in this block of code:

    if (root_objectid) {
    struct root_entry *exist_re;

    exist_re = insert_root_entry(&exist->roots, re);
    if (exist_re)
    kfree(re);
    }

    There is no 'else' block freeing when root_objectid is 0. Add the
    missing kfree to the else branch.

    Fixes: fd708b81d972 ("Btrfs: add a extent ref verify tool")
    CC: stable@vger.kernel.org # 4.19+
    Signed-off-by: Tom Rix
    Reviewed-by: David Sterba
    Signed-off-by: David Sterba

    Tom Rix
     

02 Jul, 2020

1 commit


24 Mar, 2020

1 commit

  • While debugging I noticed I wasn't getting ref verify errors before
    everything blew up. Turns out it's because we don't warn when we try to
    add a normal ref via btrfs_inc_ref() if the block entry exists but has 0
    references. This is incorrect, we should never be doing anything other
    than adding a new extent once a block entry drops to 0 references.

    Signed-off-by: Josef Bacik
    Reviewed-by: David Sterba
    Signed-off-by: David Sterba

    Josef Bacik
     

13 Feb, 2020

1 commit

  • In btrfs_ref_tree_mod(), 'ref' and 'ra' are allocated through kzalloc() and
    kmalloc(), respectively. In the following code, if an error occurs, the
    execution will be redirected to 'out' or 'out_unlock' and the function will
    be exited. However, on some of the paths, 'ref' and 'ra' are not
    deallocated, leading to memory leaks. For example, if 'action' is
    BTRFS_ADD_DELAYED_EXTENT, add_block_entry() will be invoked. If the return
    value indicates an error, the execution will be redirected to 'out'. But,
    'ref' is not deallocated on this path, causing a memory leak.

    To fix the above issues, deallocate both 'ref' and 'ra' before exiting from
    the function when an error is encountered.

    CC: stable@vger.kernel.org # 4.15+
    Signed-off-by: Wenwen Wang
    Reviewed-by: David Sterba
    Signed-off-by: David Sterba

    Wenwen Wang
     

03 Oct, 2019

1 commit

  • Coverity caught a case where we could return with a uninitialized value
    in ret in process_leaf. This is actually pretty likely because we could
    very easily run into a block group item key and have a garbage value in
    ret and think there was an errror. Fix this by initializing ret to 0.

    Reported-by: Colin Ian King
    Fixes: fd708b81d972 ("Btrfs: add a extent ref verify tool")
    CC: stable@vger.kernel.org # 4.19+
    Signed-off-by: Josef Bacik
    Reviewed-by: David Sterba
    Signed-off-by: David Sterba

    Josef Bacik
     

08 May, 2019

2 commits

  • …kernel/git/gustavoars/linux

    Pull Wimplicit-fallthrough updates from Gustavo A. R. Silva:
    "Mark switch cases where we are expecting to fall through.

    This is part of the ongoing efforts to enable -Wimplicit-fallthrough.

    Most of them have been baking in linux-next for a whole development
    cycle. And with Stephen Rothwell's help, we've had linux-next
    nag-emails going out for newly introduced code that triggers
    -Wimplicit-fallthrough to avoid gaining more of these cases while we
    work to remove the ones that are already present.

    We are getting close to completing this work. Currently, there are
    only 32 of 2311 of these cases left to be addressed in linux-next. I'm
    auditing every case; I take a look into the code and analyze it in
    order to determine if I'm dealing with an actual bug or a false
    positive, as explained here:

    https://lore.kernel.org/lkml/c2fad584-1705-a5f2-d63c-824e9b96cf50@embeddedor.com/

    While working on this, I've found and fixed the several missing
    break/return bugs, some of them introduced more than 5 years ago.

    Once this work is finished, we'll be able to universally enable
    "-Wimplicit-fallthrough" to avoid any of these kinds of bugs from
    entering the kernel again"

    * tag 'Wimplicit-fallthrough-5.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux: (27 commits)
    memstick: mark expected switch fall-throughs
    drm/nouveau/nvkm: mark expected switch fall-throughs
    NFC: st21nfca: Fix fall-through warnings
    NFC: pn533: mark expected switch fall-throughs
    block: Mark expected switch fall-throughs
    ASN.1: mark expected switch fall-through
    lib/cmdline.c: mark expected switch fall-throughs
    lib: zstd: Mark expected switch fall-throughs
    scsi: sym53c8xx_2: sym_nvram: Mark expected switch fall-through
    scsi: sym53c8xx_2: sym_hipd: mark expected switch fall-throughs
    scsi: ppa: mark expected switch fall-through
    scsi: osst: mark expected switch fall-throughs
    scsi: lpfc: lpfc_scsi: Mark expected switch fall-throughs
    scsi: lpfc: lpfc_nvme: Mark expected switch fall-through
    scsi: lpfc: lpfc_nportdisc: Mark expected switch fall-through
    scsi: lpfc: lpfc_hbadisc: Mark expected switch fall-throughs
    scsi: lpfc: lpfc_els: Mark expected switch fall-throughs
    scsi: lpfc: lpfc_ct: Mark expected switch fall-throughs
    scsi: imm: mark expected switch fall-throughs
    scsi: csiostor: csio_wr: mark expected switch fall-through
    ...

    Linus Torvalds
     
  • Pull btrfs updates from David Sterba:
    "This time the majority of changes are cleanups, though there's still a
    number of changes of user interest.

    User visible changes:

    - better read time and write checks to catch errors early and before
    writing data to disk (to catch potential memory corruption on data
    that get checksummed)

    - qgroups + metadata relocation: last speed up patch int the series
    to address the slowness, there should be no overhead comparing
    balance with and without qgroups

    - FIEMAP ioctl does not start a transaction unnecessarily, this can
    result in a speed up and less blocking due to IO

    - LOGICAL_INO (v1, v2) does not start transaction unnecessarily, this
    can speed up the mentioned ioctl and scrub as well

    - fsync on files with many (but not too many) hardlinks is faster,
    finer decision if the links should be fsynced individually or
    completely

    - send tries harder to find ranges to clone

    - trim/discard will skip unallocated chunks that haven't been touched
    since the last mount

    Fixes:

    - send flushes delayed allocation before start, otherwise it could
    miss some changes in case of a very recent rw->ro switch of a
    subvolume

    - fix fallocate with qgroups that could lead to space accounting
    underflow, reported as a warning

    - trim/discard ioctl honours the requested range

    - starting send and dedupe on a subvolume at the same time will let
    only one of them succeed, this is to prevent changes that send
    could miss due to dedupe; both operations are restartable

    Core changes:

    - more tree-checker validations, errors reported by fuzzing tools:
    - device item
    - inode item
    - block group profiles

    - tracepoints for extent buffer locking

    - async cow preallocates memory to avoid errors happening too deep in
    the call chain

    - metadata reservations for delalloc reworked to better adapt in
    many-writers/low-space scenarios

    - improved space flushing logic for intense DIO vs buffered workloads

    - lots of cleanups
    - removed unused struct members
    - redundant argument removal
    - properties and xattrs
    - extent buffer locking
    - selftests
    - use common file type conversions
    - many-argument functions reduction"

    * tag 'for-5.2-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux: (227 commits)
    btrfs: Use kvmalloc for allocating compressed path context
    btrfs: Factor out common extent locking code in submit_compressed_extents
    btrfs: Set io_tree only once in submit_compressed_extents
    btrfs: Replace clear_extent_bit with unlock_extent
    btrfs: Make compress_file_range take only struct async_chunk
    btrfs: Remove fs_info from struct async_chunk
    btrfs: Rename async_cow to async_chunk
    btrfs: Preallocate chunks in cow_file_range_async
    btrfs: reserve delalloc metadata differently
    btrfs: track DIO bytes in flight
    btrfs: merge calls of btrfs_setxattr and btrfs_setxattr_trans in btrfs_set_prop
    btrfs: delete unused function btrfs_set_prop_trans
    btrfs: start transaction in xattr_handler_set_prop
    btrfs: drop local copy of inode i_mode
    btrfs: drop old_fsflags in btrfs_ioctl_setflags
    btrfs: modify local copy of btrfs_inode flags
    btrfs: drop useless inode i_flags copy and restore
    btrfs: start transaction in btrfs_ioctl_setflags()
    btrfs: export btrfs_set_prop
    btrfs: refactor btrfs_set_props to validate externally
    ...

    Linus Torvalds
     

30 Apr, 2019

1 commit


29 Apr, 2019

1 commit

  • Replace the indirection through struct stack_trace with an invocation of
    the storage array based interface.

    Signed-off-by: Thomas Gleixner
    Reviewed-by: Johannes Thumshirn
    Reviewed-by: Josh Poimboeuf
    Acked-by: David Sterba
    Cc: Andy Lutomirski
    Cc: Chris Mason
    Cc: Josef Bacik
    Cc: linux-btrfs@vger.kernel.org
    Cc: Steven Rostedt
    Cc: Alexander Potapenko
    Cc: Alexey Dobriyan
    Cc: Andrew Morton
    Cc: Christoph Lameter
    Cc: Pekka Enberg
    Cc: linux-mm@kvack.org
    Cc: David Rientjes
    Cc: Catalin Marinas
    Cc: Dmitry Vyukov
    Cc: Andrey Ryabinin
    Cc: kasan-dev@googlegroups.com
    Cc: Mike Rapoport
    Cc: Akinobu Mita
    Cc: Christoph Hellwig
    Cc: iommu@lists.linux-foundation.org
    Cc: Robin Murphy
    Cc: Marek Szyprowski
    Cc: dm-devel@redhat.com
    Cc: Mike Snitzer
    Cc: Alasdair Kergon
    Cc: Daniel Vetter
    Cc: intel-gfx@lists.freedesktop.org
    Cc: Joonas Lahtinen
    Cc: Maarten Lankhorst
    Cc: dri-devel@lists.freedesktop.org
    Cc: David Airlie
    Cc: Jani Nikula
    Cc: Rodrigo Vivi
    Cc: Tom Zanussi
    Cc: Miroslav Benes
    Cc: linux-arch@vger.kernel.org
    Link: https://lkml.kernel.org/r/20190425094802.338890064@linutronix.de

    Thomas Gleixner
     

09 Apr, 2019

1 commit

  • In preparation to enabling -Wimplicit-fallthrough, mark switch cases
    where we are expecting to fall through.

    This patch fixes the following warnings:

    fs/affs/affs.h:124:38: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/configfs/dir.c:1692:11: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/configfs/dir.c:1694:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/ceph/file.c:249:3: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/ext4/hash.c:233:15: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/ext4/hash.c:246:15: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/ext2/inode.c:1237:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/ext2/inode.c:1244:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/ext4/indirect.c:1182:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/ext4/indirect.c:1188:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/ext4/indirect.c:1432:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/ext4/indirect.c:1440:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/f2fs/node.c:618:8: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/f2fs/node.c:620:8: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/btrfs/ref-verify.c:522:15: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/gfs2/bmap.c:711:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/gfs2/bmap.c:722:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/jffs2/fs.c:339:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/nfsd/nfs4proc.c:429:12: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/ufs/util.h:62:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/ufs/util.h:43:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/fcntl.c:770:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/seq_file.c:319:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/libfs.c:148:11: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/libfs.c:150:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/signalfd.c:178:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
    fs/locks.c:1473:16: warning: this statement may fall through [-Wimplicit-fallthrough=]

    Warning level 3 was used: -Wimplicit-fallthrough=3

    This patch is part of the ongoing efforts to enabling
    -Wimplicit-fallthrough.

    Reviewed-by: Kees Cook
    Signed-off-by: Gustavo A. R. Silva

    Gustavo A. R. Silva
     

25 Feb, 2019

1 commit


17 Dec, 2018

1 commit


15 Oct, 2018

1 commit

  • There are two members in struct btrfs_root which indicate root's
    objectid: objectid and root_key.objectid.

    They are both set to the same value in __setup_root():

    static void __setup_root(struct btrfs_root *root,
    struct btrfs_fs_info *fs_info,
    u64 objectid)
    {
    ...
    root->objectid = objectid;
    ...
    root->root_key.objectid = objecitd;
    ...
    }

    and not changed to other value after initialization.

    grep in btrfs directory shows both are used in many places:
    $ grep -rI "root->root_key.objectid" | wc -l
    133
    $ grep -rI "root->objectid" | wc -l
    55
    (4.17, inc. some noise)

    It is confusing to have two similar variable names and it seems
    that there is no rule about which should be used in a certain case.

    Since ->root_key itself is needed for tree reloc tree, let's remove
    'objecitd' member and unify code to use ->root_key.objectid in all places.

    Signed-off-by: Misono Tomohiro
    Reviewed-by: Qu Wenruo
    Reviewed-by: David Sterba
    Signed-off-by: David Sterba

    Misono Tomohiro
     

12 Apr, 2018

1 commit


31 Mar, 2018

1 commit

  • We have several reports about node pointer points to incorrect child
    tree blocks, which could have even wrong owner and level but still with
    valid generation and checksum.

    Although btrfs check could handle it and print error message like:
    leaf parent key incorrect 60670574592

    Kernel doesn't have enough check on this type of corruption correctly.
    At least add such check to read_tree_block() and btrfs_read_buffer(),
    where we need two new parameters @level and @first_key to verify the
    child tree block.

    The new @level check is mandatory and all call sites are already
    modified to extract expected level from its call chain.

    While @first_key is optional, the following call sites are skipping such
    check:
    1) Root node/leaf
    As ROOT_ITEM doesn't contain the first key, skip @first_key check.
    2) Direct backref
    Only parent bytenr and level is known and we need to resolve the key
    all by ourselves, skip @first_key check.

    Another note of this verification is, it needs extra info from nodeptr
    or ROOT_ITEM, so it can't fit into current tree-checker framework, which
    is limited to node/leaf boundary.

    Signed-off-by: Qu Wenruo
    Signed-off-by: David Sterba

    Qu Wenruo
     

22 Jan, 2018

1 commit

  • With gcc-4.1.2:

    fs/btrfs/ref-verify.c: In function ‘btrfs_build_ref_tree’:
    fs/btrfs/ref-verify.c:1017: warning: ‘root’ is used uninitialized in this function

    The variable is indeed passed uninitialized, but it is never used by the
    callee. However, not all versions of gcc are smart enough to notice.

    Hence remove the unused parameter from walk_up_tree() to silence the
    compiler warning.

    Signed-off-by: Geert Uytterhoeven
    Signed-off-by: David Sterba

    Geert Uytterhoeven
     

30 Oct, 2017

1 commit

  • We were having corruption issues that were tied back to problems with
    the extent tree. In order to track them down I built this tool to try
    and find the culprit, which was pretty successful. If you compile with
    this tool on it will live verify every ref update that the fs makes and
    make sure it is consistent and valid. I've run this through with
    xfstests and haven't gotten any false positives. Thanks,

    Signed-off-by: Josef Bacik
    Reviewed-by: David Sterba
    [ update error messages, add fixup from Dan Carpenter to handle errors
    of read_tree_block ]
    Signed-off-by: David Sterba

    Josef Bacik