As the link reported, btrfs fi sh may crash while a device is removing.
valgrind reported:
======================================================================
...
==883== Invalid write of size 8
==883== at 0x13C99A: get_device_info (in /usr/bin/btrfs)
==883== by 0x13D715: get_fs_info (in /usr/bin/btrfs)
==883== by 0x153B5F: ??? (in /usr/bin/btrfs)
==883== by 0x11B0C1: main (in /usr/bin/btrfs)
==883== Address 0x4d8c7a0 is 0 bytes after a block of size 12,288 alloc'd
==883== at 0x483877F: malloc (vg_replace_malloc.c:299)
==883== by 0x13D861: get_fs_info (in /usr/bin/btrfs)
==883== by 0x153B5F: ??? (in /usr/bin/btrfs)
==883== by 0x11B0C1: main (in /usr/bin/btrfs)
==883==
==883== Invalid write of size 8
==883== at 0x13C99D: get_device_info (in /usr/bin/btrfs)
==883== by 0x13D715: get_fs_info (in /usr/bin/btrfs)
==883== by 0x153B5F: ??? (in /usr/bin/btrfs)
==883== by 0x11B0C1: main (in /usr/bin/btrfs)
==883== Address 0x4d8c7a8 is 8 bytes after a block of size 12,288 alloc'd
==883== at 0x483877F: malloc (vg_replace_malloc.c:299)
==883== by 0x13D861: get_fs_info (in /usr/bin/btrfs)
==883== by 0x153B5F: ??? (in /usr/bin/btrfs)
==883== by 0x11B0C1: main (in /usr/bin/btrfs)
==883==
==883== Syscall param ioctl(generic) points to unaddressable byte(s)
==883== at 0x4CA9CBB: ioctl (in /usr/lib/libc-2.29.so)
==883== by 0x13C9AB: get_device_info (in /usr/bin/btrfs)
==883== by 0x13D715: get_fs_info (in /usr/bin/btrfs)
==883== by 0x153B5F: ??? (in /usr/bin/btrfs)
==883== by 0x11B0C1: main (in /usr/bin/btrfs)
==883== Address 0x4d8c7a0 is 0 bytes after a block of size 12,288 alloc'd
==883== at 0x483877F: malloc (vg_replace_malloc.c:299)
==883== by 0x13D861: get_fs_info (in /usr/bin/btrfs)
==883== by 0x153B5F: ??? (in /usr/bin/btrfs)
==883== by 0x11B0C1: main (in /usr/bin/btrfs)
==883==
--883-- VALGRIND INTERNAL ERROR: Valgrind received a signal 11 (SIGSEGV) - exiting
--883-- si_code=1; Faulting address: 0x284D8C7B8; sp: 0x1002eb5e50
valgrind: the 'impossible' happened:
Killed by fatal signal
host stacktrace:
==883== at 0x5805261C: get_bszB_as_is (m_mallocfree.c:303)
==883== by 0x5805261C: get_bszB (m_mallocfree.c:315)
==883== by 0x5805261C: vgPlain_arena_malloc (m_mallocfree.c:1799)
==883== by 0x58005AD2: vgMemCheck_new_block (mc_malloc_wrappers.c:372)
==883== by 0x58005AD2: vgMemCheck_malloc (mc_malloc_wrappers.c:407)
==883== by 0x580A7373: do_client_request (scheduler.c:1925)
==883== by 0x580A7373: vgPlain_scheduler (scheduler.c:1488)
==883== by 0x580F57A0: thread_wrapper (syswrap-linux.c:103)
==883== by 0x580F57A0: run_a_thread_NORETURN (syswrap-linux.c:156)
sched status:
running_tid=1
Thread 1: status = VgTs_Runnable (lwpid 883)
==883== at 0x483877F: malloc (vg_replace_malloc.c:299)
==883== by 0x1534AA: ??? (in /usr/bin/btrfs)
==883== by 0x153C49: ??? (in /usr/bin/btrfs)
==883== by 0x11B0C1: main (in /usr/bin/btrfs)
client stack range: [0x1FFEFFA000 0x1FFF000FFF] client SP: 0x1FFEFFDCE0
valgrind stack range: [0x1002DB6000 0x1002EB5FFF] top usage: 7520 of 1048576
======================================================================
The above log says that invalid write to allocated @di_args happened
in get_device_info() called in get_fs_info().
The size of @di_args is allocated according by fi_args->num_devices.
And fi_args->num_devices is *the number of dev_items in chunk_tree*.
However, in the loop to get devices info, btrfs-progs calls ioctl
BTRFS_IOC_DEV_INFO which just finds device in
fs_info->fs_devices->devices.
Let's look at kernel side.
In btrfs_rm_device(), btrfs_rm_dev_item() causes removal of
related dev_items in chunk_tree. *Do something*.
Then delete the device from device->fs_devices.
So the case is:
Userspace kernel
get_fs_info() btrfs_rm_device()
...
btrfs_rm_dev_item()
determine fi_args->num_devices and
fi_args->max_id by seraching chunk_tree.
malloc() ...
Loop(Crashed): call get_device_info() by devid
from 1 to fi_args->max_id.
mutex_lock(&fs_devices->device_list_mutex);
list_del_rcu(&device->dev_list);
...
In the loop of get_device_info(), get_device_info() still can get info
of the removing device since it's still in fs_info->fs_devices->devices.
Then the iterator value @ndev increaments causes invalid access out of
bounds.
Solved it by adding the check of @ndev while looping.
Reported-by: Peter Hjalmarsson <kanelxake@gmail.com>
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1711787
Signed-off-by: Su Yue <Damenly_Su@gmx.com>
Signed-off-by: David Sterba <dsterba@suse.com>
blkid_get_cache() returns error code which is -errno. So we can use them
directly.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: David Sterba <dsterba@suse.com>
This reverts commit cb8abddb20.
There are several reports in IRC that this patch breaks in some
send/receive environments. There are no exact steps to reproduce, only
approximate descroptions. Until a proper reproducer is known, the patch
is temporarily reverted due to the user-visible impact.
Issue: #162
Signed-off-by: David Sterba <dsterba@suse.com>
The strdup() function can fail, in which case it returns NULL and sets
errno [1]. Therefore add the missing error check after calling strdup()
at find_mount_root().
[1] - http://man7.org/linux/man-pages/man3/strdup.3p.html
Reviewed-by: David Disseldorp <ddiss@suse.de>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
When attempting to find the mount point of a path we can end up returning
an incorrect mount point. This happens because we consider a mount point
valid for the given path even if it only partially matches the path.
Consider the following example, which makes btrfs receive fail:
$ truncate -s 1G disk1
$ truncate -s 1G disk2
$ losetup /dev/loop1 disk1
$ losetup /dev/loop2 disk2
$ mkfs.btrfs -f /dev/loop1
$ mkfs.btrfs -f /dev/loop2
$ mount /dev/loop1 /mnt
$ mkdir /mnt/ddis
$ mkdir /mnt/ddis-not-a-mount
$ mount /dev/loop2 /mnt/ddis
$ echo "some data" > /mnt/ddis/file
$ btrfs subvolume snapshot -r /mnt/ddis /mnt/ddis/snap
$ btrfs send -f /tmp/send.data /mnt/ddis/snap
$ btrfs receive -f /tmp/send.data /mnt/ddis-not-a-mount
At subvol snap
ERROR: chown failed: No such file or directory
In that example btrfs receive passes the path "/mnt/ddis-not-a-mount" to
find_mount_root() which picks "/mnt/ddis" as the mount point instead of
"/mnt". The wrong decision happens because "/mnt/ddis" is the longest
string found that is a prefix of "/mnt/ddis-not-a-mount", however it
shouldn't be considered valid because what follows the substring "ddis"
in the given path is not a path separator ("/") nor the null character
('\0'). So fix find_mount_root() to check for the presence of a path
separator or a null byte character after if finds a mount point string
that matches the given path.
A test case will follow soon in a separate patch.
Reported-by: David Disseldorp <ddiss@suse.com>
Reviewed-by: David Disseldorp <ddiss@suse.de>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
And fsfeatures.c is indeed a better location for that function.
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Although most fallthrough cases are pretty obvious, we still need to
teach hint the compiler that it's an explicit fallthrough. The
annotation is not standardized and sometimes comments are used, the
attribute is getting more widespread so we're going to use it too.
Unknown attributes are ignored by old compilers.
Also reformat the code to use common indentation.
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Similar to the changes where strerror(errno) was converted, continue
with the remaining cases where the argument was stored in another
variable.
The savings in object size are about 4500 bytes:
$ size btrfs.old btrfs.new
text data bss dec hex filename
805055 24248 19748 849051 cf49b btrfs.old
804527 24248 19748 848523 cf28b btrfs.new
Signed-off-by: David Sterba <dsterba@suse.com>
Before this patch:
$ ls nothingness
ls: cannot access 'nothingness': No such file or directory
$ btrfs inspect-internal dump-tree nothingness
ERROR: not a block device or regular file: nothingness
The confusing error message makes users think that nonexistent file
exiss but is of a wrong type.
This patch lets check_arg_type return -errno if realpath failed. And
print strerror if check_arg_type failed and the returned code is
negative. Like:
$ btrfs inspect-internal dump-tree nothingness
ERROR: invalid argument: nothingness: No such file or directory
Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Extend check_mounted_where so we can pass additional flags that would
allow us to open filesystem in some specific state. This will be used
for a filesystem that has a partially changed uuid.
Signed-off-by: David Sterba <dsterba@suse.com>
Fix subvol_strip_mountpoint for mnt="/" (len=1). In this case, skip
check for trailing slash on full_path (leading slash on full_path is
already asserted by strncmp).
Issue: #122
Pull-request: #138
Fixes: c5dc299aff ("btrfs-progs: prevent incorrect use of subvol_strip_mountpoint")
Signed-off-by: Axel Burri <axel@tty0.ch>
Signed-off-by: David Sterba <dsterba@suse.com>
The old flag OPEN_CTREE_FS_PARTIAL is in fact quite easy to be confused
with OPEN_CTREE_PARTIAL, which allow btrfs-progs to open damaged
filesystem (like corrupted extent/csum tree).
However OPEN_CTREE_FS_PARTIAL, unlike its name, is just allowing
btrfs-progs to open fs with temporary superblocks (which only has 6
basic trees on SINGLE meta/sys chunks).
The usage of FS_PARTIAL is really confusing here.
So rename OPEN_CTREE_FS_PARTIAL to OPEN_CTREE_TEMPORARY_SUPER, and add
extra comment for its behavior.
Also rename BTRFS_MAGIC_PARTIAL to BTRFS_MAGIC_TEMPORARY to keep the
naming consistent.
And with above comment, the usage of FS_PARTIAL in dump-tree is
obviously incorrect, fix it.
Fixes: 8698a2b9ba ("btrfs-progs: Allow inspect dump-tree to show specified tree block even some tree roots are corrupted")
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
This gets the remaining occurrences that weren't covered by previous
conversions.
Signed-off-by: Omar Sandoval <osandov@fb.com>
[ fixup test_issubvolume due to removed dependency patch ]
Signed-off-by: David Sterba <dsterba@suse.com>
Now implemented with btrfs_util_subvolume_path(),
btrfs_util_subvolume_info(), and subvolume iterators.
Signed-off-by: Omar Sandoval <osandov@fb.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Add additional bound checks to prevent memory corruption on incorrect
usage of subvol_strip_mountpoint. Assert sane return value by properly
comparing the mount point to the full_path before stripping it off.
Mitigates issue: "btrfs send -p" fails if source and parent subvolumes
are on different mountpoints (memory corruption):
https://github.com/kdave/btrfs-progs/issues/96
Note that this does not properly fix this bug, but prevents a possible
security issue by unexpected usage of "btrfs send -p".
Issue: #96
Pull-request: #98
Signed-off-by: Axel Burri <axel@tty0.ch>
Signed-off-by: David Sterba <dsterba@suse.com>
As btrfs is specific to Linux, %m can be used instead of strerror(errno)
in format strings. This has some size reduction benefits for embedded
systems.
glibc, musl, and uclibc-ng all support %m as a modifier to printf.
A quick glance at the BIONIC libc source indicates that it has
support for %m as well. BSDs and Windows do not but I do believe
them to be beyond the scope of btrfs-progs.
Compiled sizes on Ubuntu 16.04:
Before:
3916512 btrfs
233688 libbtrfs.so.0.1
4899 bcp
2367672 btrfs-convert
2208488 btrfs-corrupt-block
13302 btrfs-debugfs
2152160 btrfs-debug-tree
2136024 btrfs-find-root
2287592 btrfs-image
2144600 btrfs-map-logical
2130760 btrfs-select-super
2152608 btrfstune
2131760 btrfs-zero-log
2277752 mkfs.btrfs
9166 show-blocks
After:
3908744 btrfs
233256 libbtrfs.so.0.1
4899 bcp
2366560 btrfs-convert
2207432 btrfs-corrupt-block
13302 btrfs-debugfs
2151104 btrfs-debug-tree
2134968 btrfs-find-root
2281864 btrfs-image
2143536 btrfs-map-logical
2129704 btrfs-select-super
2151552 btrfstune
2130696 btrfs-zero-log
2276272 mkfs.btrfs
9166 show-blocks
Total savings: 23928 (24 kilo)bytes
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Preserve the errno value for the caller in case closing happens in the
middle of eg. an ioctl and reporing the failure. The errors that could
happen in close/closedir do not bother us.
Signed-off-by: David Sterba <dsterba@suse.com>
There are a couple of places where instead of the more succinct
list_for_each_entry the code uses list_for_each. This results in
slightly more code with no additional benefit as well as no
coherent pattern. This patch makes the code uniform. No functional
changes.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
[ remove unused variable in uuid_search ]
Signed-off-by: David Sterba <dsterba@suse.com>
This function has been copied twice in chunk-recover and super-recover. Factor
it out into utils.c/h and use it. No functional changes.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
For --rootdir, even for large existing file or block device, it will
always shrink the resulting filesystem.
The problem is, mkfs.btrfs will try to calculate the dir size, and use
it as @block_count to mkfs, which makes the filesystem shrunk.
Fix it by trying to get the original block device or file size as
@block_count, so mkfs.btrfs can use the full file/block device for
--rootdir option.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Commit 460e93f25754 ("btrfs-progs: mkfs: check the status of file at mkfs")
will try to check the file state before creating fs on it.
The check is mostly fine for normal mkfs case, while for --rootdir
option, it's allowed to create a new file if the destination file
doesn't exist.
Fix it by allowing non-existent file if --rootdir is specified.
Fixes: 460e93f25754 ("btrfs-progs: mkfs: check the status of file at mkfs")
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>
We have the infrastructure to cache extent buffers but we don't actually
do the caching. As soon as the last reference is dropped, the buffer
is dropped. This patch keeps the extent buffers around until the max
cache size is reached (defaults to 25% of memory) and then it drops
the last 10% of the LRU to free up cache space for reallocation. The
cache size is configurable (for use by e.g. lowmem) when the cache is
initialized.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
[ update codingstyle, switch total_memory to bytes ]
Signed-off-by: David Sterba <dsterba@suse.com>
Change seen_fsid to hold fd and DIR in order to keep access to each fs.
This will be used for 'subvol delete --commit-after'.
Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
Reviewed-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Move is_seen_fsid()/add_seen_fsid()/free_seen_fsid() to common functions.
This will be used for 'subvol delete --commit-after'.
Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
Reviewed-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Make get_fsid() to a common functions.
This will be used for 'subvol delete --commit-after'.
Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
Reviewed-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Since cmd_inspect_rootid() calls btrfs_open_dir(), it rejects a file to
be specified. But as the document says, a file should be supported.
This patch introduces btrfs_open_file_or_dir(), which is a counterpart
of btrfs_open_dir(), to safely check and open btrfs file or directory.
The original btrfs_open_dir() content is moved to btrfs_open() and shared
by both function.
Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Currently transaction bugs out insided btrfs_start_transaction in case
of error, we want to lift the error handling to the callers. This patch
adds the BUG_ON anywhere it's been missing so far. This is not the best
way of course. Transforming BUG_ON to a proper error handling highly
depends on the caller and should be dealt with case by case.
Signed-off-by: David Sterba <dsterba@suse.com>
Unless the top level is mounted there is no way to know the
details of all the subvolume. For example:
mount -o subvol=sv1/newsv1 /dev/sdb /btrfs
btrfs su list /btrfs
ID 257 gen 12 top level 5 path sv1
ID 258 gen 9 top level 257 path sv1/snap
ID 259 gen 11 top level 257 path sv1/newsv1
You can't subvol show for sv1 and sv1/snap as its paths aren't
accessible to the user unless the its top level is mounted.
This patch adds two new options to the existing btrfs subvol show
cli. They are --rootid/-r or --uuid/-u, with this now the user will
be able to look for a subvolume using the rootid OR the uuid.
./btrfs su show -r 257 /btrfs
sv1
Name: sv1
UUID: 30129358-c69d-3e4a-a662-29509cc69c95
Parent UUID: -
Received UUID: -
Creation time: 2017-07-11 20:32:57 +0800
Subvolume ID: 257
Generation: 12
Gen at creation: 7
Parent ID: 5
Top level ID: 5
Flags: -
Snapshot(s):
sv1/snap
Signed-off-by: Anand Jain <anand.jain@oracle.com>
[ minor adjustments in the help text ]
Signed-off-by: David Sterba <dsterba@suse.com>
BTW, there is a duplicated definition of btrfs_add_device() in
volumes.h, also remove it.
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>
There are not performance critical static inlines, we can do the normal
declaration/definition split for various helpers.
Signed-off-by: David Sterba <dsterba@suse.com>
Large numbers like (1024 * 1024 * 1024) may cost reader/reviewer to
waste one second to convert to 1G.
Introduce kernel include/linux/sizes.h to replace any intermediate
number larger than 4096 (not including 4096) to SZ_*.
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>
The regulare mkfs_btrfs does not anything special about convert and just
passes the arguments. Make that two functions.
Signed-off-by: David Sterba <dsterba@suse.com>
Add a unit mode that will interpret the input number as a signed 64bit,
optionally and not by default for all numbers.
Signed-off-by: David Sterba <dsterba@suse.com>
I got tired of seeing "16.00EiB" whenever btrfs-progs encounters a
negative size value, e.g. during resize:
Unallocated:
/dev/mapper/datamd18 16.00EiB
This version is much more useful:
Unallocated:
/dev/mapper/datamd18 -26.29GiB
Signed-off-by: Zygo Blaxell <ce3g8jdj@umail.furryterror.org>
Reviewed-by: Omar Sandoval <osandov@fb.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Su Yue