Re: [RFC PATCH v7 1/2] Btrfs: Add a new ioctl to get the label of a mounted file system
On 12/21/2012 07:42, Jeff Liu wrote:
On 12/21/2012 04:18 AM, Goffredo Baroncelli wrote:
On 12/20/2012 09:43 AM, Jeff Liu wrote:
+static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg)
+{
+ struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root;
+ const char *label = root-fs_info-super_copy-label;
+ int ret;
+
+ mutex_lock(root-fs_info-volume_mutex);
+ ret = copy_to_user(arg, label, strlen(label));
Sorry for pointing out my doubt too late, but should we trust
super_copy-label ?
An user could insert a usb-key with a btrfs filesystem with a label
without zero. In this case strlen() could access outside
super_copy-label[].
Thank you for letting me be aware of this situation.
First of all, if the user set label via btrfs tools, he can not make it
length exceeding BTRFS_LABLE_SIZE - 1.
If the user does that through codes wrote by himself like:
btrfslabel.c-set_label_unmounted(), he can do that.
However, it most likely he did that for evil purpose or any other reasons?
I think that it should be quite easy to alter artificially a filesystem
to crash the kernel. So I not consider this as big problem. However *in
case* of a further cycle of this patch I suggest to replace strlen()
with strnlen().
I don't think we should replace strlen() with strnlen() since it's
totally wrong if the length of label is more than BTRFS_LABEL_SIZE -1,
we can not just truncating the label and return it in this case.
Add BUG_ON(strlen(label) BTRFS_LABEL_SIZE - 1) is reasonable instead.
Don't allow users to attack the kernel! This would add a severe security
issue. A BUG_ON() is something that you can use before the code would
crash anyway, to prevent any additional damage and to help in debugging.
A BUG() is not a method to report or handle user errors.
A Linux system is supposed to run until it is shutdown by the
administrator, not until somebody inserts an USB stick.
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] Btrfs: use wrapper page_offset
Use wrapper page_offset to get byte-offset into filesystem object for page.
Signed-off-by: Liu Bo liubo2...@cn.fujitsu.com
Signed-off-by: Miao Xie mi...@cn.fujitsu.com
---
fs/btrfs/disk-io.c| 2 +-
fs/btrfs/extent_io.c | 24 +++-
fs/btrfs/inode.c | 5 ++---
fs/btrfs/relocation.c | 2 +-
4 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 65f0367..e045203 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -420,7 +420,7 @@ static int btree_read_extent_buffer_pages(struct btrfs_root
*root,
static int csum_dirty_buffer(struct btrfs_root *root, struct page *page)
{
struct extent_io_tree *tree;
- u64 start = (u64)page-index PAGE_CACHE_SHIFT;
+ u64 start = page_offset(page);
u64 found_start;
struct extent_buffer *eb;
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 1b319df..bda36fe 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1834,7 +1834,7 @@ int test_range_bit(struct extent_io_tree *tree, u64
start, u64 end,
*/
static void check_page_uptodate(struct extent_io_tree *tree, struct page *page)
{
- u64 start = (u64)page-index PAGE_CACHE_SHIFT;
+ u64 start = page_offset(page);
u64 end = start + PAGE_CACHE_SIZE - 1;
if (test_range_bit(tree, start, end, EXTENT_UPTODATE, 1, NULL))
SetPageUptodate(page);
@@ -1846,7 +1846,7 @@ static void check_page_uptodate(struct extent_io_tree
*tree, struct page *page)
*/
static void check_page_locked(struct extent_io_tree *tree, struct page *page)
{
- u64 start = (u64)page-index PAGE_CACHE_SHIFT;
+ u64 start = page_offset(page);
u64 end = start + PAGE_CACHE_SIZE - 1;
if (!test_range_bit(tree, start, end, EXTENT_LOCKED, 0, NULL))
unlock_page(page);
@@ -1960,7 +1960,7 @@ int repair_io_failure(struct btrfs_fs_info *fs_info, u64
start,
return -EIO;
}
bio-bi_bdev = dev-bdev;
- bio_add_page(bio, page, length, start-page_offset(page));
+ bio_add_page(bio, page, length, start - page_offset(page));
btrfsic_submit_bio(WRITE_SYNC, bio);
wait_for_completion(compl);
@@ -2293,8 +2293,7 @@ static void end_bio_extent_writepage(struct bio *bio, int
err)
struct page *page = bvec-bv_page;
tree = BTRFS_I(page-mapping-host)-io_tree;
- start = ((u64)page-index PAGE_CACHE_SHIFT) +
-bvec-bv_offset;
+ start = page_offset(page) + bvec-bv_offset;
end = start + bvec-bv_len - 1;
if (bvec-bv_offset == 0 bvec-bv_len == PAGE_CACHE_SIZE)
@@ -2353,8 +2352,7 @@ static void end_bio_extent_readpage(struct bio *bio, int
err)
(long int)bio-bi_bdev);
tree = BTRFS_I(page-mapping-host)-io_tree;
- start = ((u64)page-index PAGE_CACHE_SHIFT) +
- bvec-bv_offset;
+ start = page_offset(page) + bvec-bv_offset;
end = start + bvec-bv_len - 1;
if (bvec-bv_offset == 0 bvec-bv_len == PAGE_CACHE_SIZE)
@@ -2471,7 +2469,7 @@ static int __must_check submit_one_bio(int rw, struct bio
*bio,
struct extent_io_tree *tree = bio-bi_private;
u64 start;
- start = ((u64)page-index PAGE_CACHE_SHIFT) + bvec-bv_offset;
+ start = page_offset(page) + bvec-bv_offset;
bio-bi_private = NULL;
@@ -2595,7 +2593,7 @@ static int __extent_read_full_page(struct extent_io_tree
*tree,
unsigned long *bio_flags)
{
struct inode *inode = page-mapping-host;
- u64 start = (u64)page-index PAGE_CACHE_SHIFT;
+ u64 start = page_offset(page);
u64 page_end = start + PAGE_CACHE_SIZE - 1;
u64 end;
u64 cur = start;
@@ -2806,7 +2804,7 @@ static int __extent_writepage(struct page *page, struct
writeback_control *wbc,
struct inode *inode = page-mapping-host;
struct extent_page_data *epd = data;
struct extent_io_tree *tree = epd-tree;
- u64 start = (u64)page-index PAGE_CACHE_SHIFT;
+ u64 start = page_offset(page);
u64 delalloc_start;
u64 page_end = start + PAGE_CACHE_SIZE - 1;
u64 end;
@@ -3674,7 +3672,7 @@ int extent_invalidatepage(struct extent_io_tree *tree,
struct page *page, unsigned long offset)
{
struct extent_state *cached_state = NULL;
- u64 start = ((u64)page-index PAGE_CACHE_SHIFT);
+ u64 start = page_offset(page);
u64 end = start + PAGE_CACHE_SIZE - 1;
size_t blocksize = page-mapping-host-i_sb-s_blocksize;
@@ -3700,7 +3698,7 @@ int try_release_extent_state(struct extent_map_tree *map,
struct extent_io_tree *tree, struct page *page,
gfp_t mask)
{
- u64 start = (u64)page-index
[PATCH 1/3] Btrfs: fix missing write access release in btrfs_ioctl_resize()
We forget to give up the write access after we find some device operation
is going on. Fix it.
Signed-off-by: Miao Xie mi...@cn.fujitsu.com
---
fs/btrfs/ioctl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 7624212..679b82c 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1338,7 +1338,9 @@ static noinline int btrfs_ioctl_resize(struct file *file,
if (atomic_xchg(root-fs_info-mutually_exclusive_operation_running,
1)) {
- pr_info(btrfs: dev add/delete/balance/replace/resize operation
in progress\n);
+ pr_info(btrfs: dev add/delete/balance/replace/resize operation
+in progress\n);
+ mnt_drop_write_file(file);
return -EINPROGRESS;
}
--
1.7.11.7
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] Btrfs: fix resize a readonly device
We should not resize a readonly device, fix it.
Signed-off-by: Miao Xie mi...@cn.fujitsu.com
---
fs/btrfs/ioctl.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 679b82c..668475c 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1364,6 +1364,7 @@ static noinline int btrfs_ioctl_resize(struct file *file,
printk(KERN_INFO btrfs: resizing devid %llu\n,
(unsigned long long)devid);
}
+
device = btrfs_find_device(root-fs_info, devid, NULL, NULL);
if (!device) {
printk(KERN_INFO btrfs: resizer unable to find device %llu\n,
@@ -1371,9 +1372,10 @@ static noinline int btrfs_ioctl_resize(struct file *file,
ret = -EINVAL;
goto out_free;
}
- if (device-fs_devices device-fs_devices-seeding) {
+
+ if (!device-writeable) {
printk(KERN_INFO btrfs: resizer unable to apply on
- seeding device %llu\n,
+ readonly device %llu\n,
(unsigned long long)devid);
ret = -EINVAL;
goto out_free;
--
1.7.11.7
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] Btrfs: fix trivial error in btrfs_ioctl_resize()
This patch fixes the following problem:
- improper return value
- unnecessary read-only check
Signed-off-by: Miao Xie mi...@cn.fujitsu.com
---
fs/btrfs/ioctl.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 668475c..773865f 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1326,9 +1326,6 @@ static noinline int btrfs_ioctl_resize(struct file *file,
int ret = 0;
int mod = 0;
- if (root-fs_info-sb-s_flags MS_RDONLY)
- return -EROFS;
-
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
@@ -1361,6 +1358,10 @@ static noinline int btrfs_ioctl_resize(struct file *file,
*devstr = '\0';
devstr = vol_args-name;
devid = simple_strtoull(devstr, end, 10);
+ if (!devid) {
+ ret = -EINVAL;
+ goto out_free;
+ }
printk(KERN_INFO btrfs: resizing devid %llu\n,
(unsigned long long)devid);
}
@@ -1369,7 +1370,7 @@ static noinline int btrfs_ioctl_resize(struct file *file,
if (!device) {
printk(KERN_INFO btrfs: resizer unable to find device %llu\n,
(unsigned long long)devid);
- ret = -EINVAL;
+ ret = -ENODEV;
goto out_free;
}
@@ -1377,7 +1378,7 @@ static noinline int btrfs_ioctl_resize(struct file *file,
printk(KERN_INFO btrfs: resizer unable to apply on
readonly device %llu\n,
(unsigned long long)devid);
- ret = -EINVAL;
+ ret = -EPERM;
goto out_free;
}
@@ -1399,7 +1400,7 @@ static noinline int btrfs_ioctl_resize(struct file *file,
}
if (device-is_tgtdev_for_dev_replace) {
- ret = -EINVAL;
+ ret = -EPERM;
goto out_free;
}
--
1.7.11.7
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] Btrfs: fix missing write access release in btrfs_ioctl_resize()
On Fri, 21 Dec 2012 11:22:52 +0100, Stefan Behrens wrote: On Fri, 21 Dec 2012 17:19:20 +0800, Miao Xie wrote: -pr_info(btrfs: dev add/delete/balance/replace/resize operation in progress\n); +pr_info(btrfs: dev add/delete/balance/replace/resize operation + in progress\n); +mnt_drop_write_file(file); grep and vi cannot do searches spanning multiple lines. That's why I created this pr_info() line with more than 80 columns (in accordance with Documentation/CodingStyle BTW (never break user-visible strings)). You are right. I will update the patch soon. Thanks Miao -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v7 1/2] Btrfs: Add a new ioctl to get the label of a mounted file system
On 12/21/2012 07:42 AM, Jeff Liu wrote:
Hi Goffredo,
On 12/21/2012 04:18 AM, Goffredo Baroncelli wrote:
HI Jeff,
On 12/20/2012 09:43 AM, Jeff Liu wrote:
With the new ioctl(2) BTRFS_IOC_GET_FSLABEL we can fetch the label of a
mounted file system.
Signed-off-by: Jie Liu jeff@oracle.com
Signed-off-by: Anand Jain anand.j...@oracle.com
Cc: Miao Xie mi...@cn.fujitsu.com
Cc: Goffredo Baroncelli kreij...@inwind.it
Cc: David Sterba dste...@suse.cz
[...]
+static int btrfs_ioctl_get_fslabel(struct file *file, void __user *arg)
+{
+ struct btrfs_root *root = BTRFS_I(fdentry(file)-d_inode)-root;
+ const char *label = root-fs_info-super_copy-label;
+ int ret;
+
+ mutex_lock(root-fs_info-volume_mutex);
+ ret = copy_to_user(arg, label, strlen(label));
Sorry for pointing out my doubt too late, but should we trust
super_copy-label ?
An user could insert a usb-key with a btrfs filesystem with a label
without zero. In this case strlen() could access outside
super_copy-label[].
Thank you for letting me be aware of this situation.
First of all, if the user set label via btrfs tools, he can not make it
length exceeding BTRFS_LABLE_SIZE - 1.
If the user does that through codes wrote by himself like:
btrfslabel.c-set_label_unmounted(), he can do that.
However, it most likely he did that for evil purpose or any other reasons?
I think the most likely case is the evil purpose.
I think that it should be quite easy to alter artificially a filesystem
to crash the kernel. So I not consider this as big problem. However *in
case* of a further cycle of this patch I suggest to replace strlen()
with strnlen().
I don't think we should replace strlen() with strnlen() since it's
totally wrong if the length of label is more than BTRFS_LABEL_SIZE -1,
we can not just truncating the label and return it in this case.
This for me is sufficient, or we could copy all the label buffer,
without further check:
copy_to_user(arg, label, BTRFS_LABEL_SIZE)
Add BUG_ON(strlen(label) BTRFS_LABEL_SIZE - 1) is reasonable instead.
I agree with Stefan, this is not a correct use of BUG_ON; a warning is
sufficient (there is un-correct data read from disk).
Thanks,
-Jeff
+ mutex_unlock(root-fs_info-volume_mutex);
+
+ return ret ? -EFAULT : 0;
+}
+
long btrfs_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
{
@@ -3797,6 +3810,8 @@ long btrfs_ioctl(struct file *file, unsigned int
return btrfs_ioctl_qgroup_create(root, argp);
case BTRFS_IOC_QGROUP_LIMIT:
return btrfs_ioctl_qgroup_limit(root, argp);
+ case BTRFS_IOC_GET_FSLABEL:
+ return btrfs_ioctl_get_fslabel(file, argp);
}
return -ENOTTY;
diff --git a/fs/btrfs/ioctl.h b/fs/btrfs/ioctl.h
index 731e287..5b2cbef 100644
--- a/fs/btrfs/ioctl.h
+++ b/fs/btrfs/ioctl.h
@@ -451,6 +451,8 @@ struct btrfs_ioctl_send_args {
struct btrfs_ioctl_qgroup_create_args)
#define BTRFS_IOC_QGROUP_LIMIT _IOR(BTRFS_IOCTL_MAGIC, 43, \
struct btrfs_ioctl_qgroup_limit_args)
+#define BTRFS_IOC_GET_FSLABEL _IOR(BTRFS_IOCTL_MAGIC, 49, \
+ char[BTRFS_LABEL_SIZE])
#define BTRFS_IOC_GET_DEV_STATS _IOWR(BTRFS_IOCTL_MAGIC, 52, \
struct btrfs_ioctl_get_dev_stats)
#endif
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
add, remove; how about merge?
I am new at this btrfs stuff! As I understand it, you can add a physical disk or partition and have data spread into the new space. You can also move data off a disk or partition and then remove it. Has any though been given to being able merge to existing (separate) btrfs volumes into a single volume? Alternatively, how about at tool for transfering a subvolume from one pool to another? I tried using fsarchiver but that did not work out too well. Gene -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: add, remove; how about merge?
On Fri, Dec 21, 2012 at 01:16:59PM -0500, Gene Czarcinski wrote: I am new at this btrfs stuff! As I understand it, you can add a physical disk or partition and have data spread into the new space. You can also move data off a disk or partition and then remove it. Has any though been given to being able merge to existing (separate) btrfs volumes into a single volume? This would be difficult to do in kernel space -- you'd have to change the UUIDs on all the metadata on at least one of the filesystems, plus handle (correctly) all of the numberspace clashes correctly -- subvolume trees, virtual address space, and probably a load that I've forgotten as well. Given what I'm about to say, I can't see this happening any time soon. Alternatively, how about at tool for transfering a subvolume from one pool to another? I tried using fsarchiver but that did not work out too well. You want send/receive. It's in the mainline kernel, and you'll need a recent userspace, but it allows you to transfer subvolumes cleanly and losslessly between filesystems. Hugo. -- === Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk === PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk --- You are not stuck in traffic: you are traffic. --- signature.asc Description: Digital signature
Re: add, remove; how about merge?
I think thats possible with btrfs-send/receive Remco On Dec 21, 2012, at 7:16 PM, Gene Czarcinski g...@czarc.net wrote: I am new at this btrfs stuff! As I understand it, you can add a physical disk or partition and have data spread into the new space. You can also move data off a disk or partition and then remove it. Has any though been given to being able merge to existing (separate) btrfs volumes into a single volume? Alternatively, how about at tool for transfering a subvolume from one pool to another? I tried using fsarchiver but that did not work out too well. Gene -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Btrfs: overwrite existing csums when logging
Users were complaining about the warning in btrfs_log_parent_inode() because
we're returning -EEXIST. This warning is gone in another patch but it's
still making us commit a transaction when we don't have to. Basically
because we will log all csums in an ordered extent for an em we have the
chance of logging the same csums multiple times. Since it's not that big of
a deal just allow csums to be over written in logs and that way we are safe.
This gets rid of the EEXIST and we log like we normally would. Thanks,
Signed-off-by: Josef Bacik jba...@fusionio.com
---
fs/btrfs/file-item.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index bd38cef..93ddc2e 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -835,6 +835,13 @@ insert:
ret = btrfs_insert_empty_item(trans, root, path, file_key,
ins_size);
path-leave_spinning = 0;
+
+ /*
+* We can sometimes already have copied a csum into the log, that's ok
+* just overwrite it.
+*/
+ if (root-objectid == BTRFS_TREE_LOG_OBJECTID ret == -EEXIST)
+ ret = 0;
if (ret 0)
goto fail_unlock;
if (ret != 0) {
--
1.7.7.6
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
