Skip to contentSkip to conversation

Btrfs: Fix uninitialized root flags for subvolumes

RFC PATCH In review Li Zefan 2 participants 2 messages Mar 28, 2011 – Mar 30, 2011 Btrfs file system

lore
Share & tools
lore

Li Zefan

author
#0 lore raw

RFC PATCH 1/1 Btrfs: Fix uninitialized root flags for subvolumes

root_item->flags and root_item->byte_limit are not initialized when a subvolume is created. This bug is not revealed until we added readonly snapshot support - now you mount a btrfs filesystem and you may find the subvolumes in it are readonly.

To work around this problem, we steal a bit from root_item->inode_item->flags, and use it to indicate if those fields have been properly initialized. When we read a tree root from disk, we check if the bit is set, and if not we'll set the flag and initialize the two fields of the root item.

Reported-by Andreas Philipp <[email protected]>
Signed-off-by Li Zefan <[email protected]>
---
 fs/btrfs/ctree.h       |    4 ++++
 fs/btrfs/disk-io.c     |    4 +++-
 fs/btrfs/ioctl.c       |    4 ++++
 fs/btrfs/root-tree.c   |   18 ++++++++++++++++++
 fs/btrfs/transaction.c |    1 +
 5 files changed, 30 insertions(+), 1 deletions(-)
fs/btrfs/ctree.h +4 -0
index 8b4b9d1..ff6b991 100644
@@ -1284,6 +1284,8 @@ struct btrfs_root {
 #define BTRFS_INODE_NOATIME		(1 << 9)
 #define BTRFS_INODE_DIRSYNC		(1 << 10)
 
+#define BTRFS_INODE_ROOT_ITEM_INIT	(1 << 31)
+
 /* some macros to generate set/get funcs for the struct fields.  This
  * assumes there is a lefoo_to_cpu for every type, so lets make a simple
  * one for u8:

@@ -2355,6 +2357,8 @@ int btrfs_find_dead_roots(struct btrfs_root *root, u64 objectid);
 int btrfs_find_orphan_roots(struct btrfs_root *tree_root);
 int btrfs_set_root_node(struct btrfs_root_item *item,
 			struct extent_buffer *node);
+void btrfs_check_and_init_root_item(struct btrfs_root_item *item);
+
 /* dir-item.c */
 int btrfs_insert_dir_item(struct btrfs_trans_handle *trans,
 			  struct btrfs_root *root, const char *name,
fs/btrfs/disk-io.c +3 -1
index 3e1ea3e..4f8dafc 100644
@@ -1184,8 +1184,10 @@ struct btrfs_root *btrfs_read_fs_root_no_radix(struct btrfs_root *tree_root,
 	root->commit_root = btrfs_root_node(root);
 	BUG_ON(!root->node);
 out:
-	if (location->objectid != BTRFS_TREE_LOG_OBJECTID)
+	if (location->objectid != BTRFS_TREE_LOG_OBJECTID) {
 		root->ref_cows = 1;
+		btrfs_check_and_init_root_item(&root->root_item);
+	}
 
 	return root;
 }
fs/btrfs/ioctl.c +4 -0
index 5fdb2ab..2ff51e6 100644
@@ -294,6 +294,10 @@ static noinline int create_subvol(struct btrfs_root *root,
 	inode_item->nbytes = cpu_to_le64(root->leafsize);
 	inode_item->mode = cpu_to_le32(S_IFDIR | 0755);
 
+	root_item.flags = 0;
+	root_item.byte_limit = 0;
+	inode_item->flags = cpu_to_le64(BTRFS_INODE_ROOT_ITEM_INIT);
+
 	btrfs_set_root_bytenr(&root_item, leaf->start);
 	btrfs_set_root_generation(&root_item, trans->transid);
 	btrfs_set_root_level(&root_item, 0);
fs/btrfs/root-tree.c +18 -0
index 6a1086e..3e45c32 100644
@@ -471,3 +471,21 @@ again:
 	btrfs_free_path(path);
 	return 0;
 }
+
+/*
+ * Old btrfs forgets to init root_item->flags and root_item->byte_limit
+ * for subvolumes. To work around this problem, we steal a bit from
+ * root_item->inode_item->flags, and use it to indicate if those fields
+ * have been properly initialized.
+ */
+void btrfs_check_and_init_root_item(struct btrfs_root_item *root_item)
+{
+	u64 inode_flags = le64_to_cpu(root_item->inode.flags);
+
+	if (!(inode_flags & BTRFS_INODE_ROOT_ITEM_INIT)) {
+		inode_flags |= BTRFS_INODE_ROOT_ITEM_INIT;
+		root_item->inode.flags = cpu_to_le64(inode_flags);
+		root_item->flags = 0;
+		root_item->byte_limit = 0;
+	}
+}
fs/btrfs/transaction.c +1 -0
index 3d73c8d..f3d6681 100644
@@ -970,6 +970,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 	record_root_in_trans(trans, root);
 	btrfs_set_root_last_snapshot(&root->root_item, trans->transid);
 	memcpy(new_root_item, &root->root_item, sizeof(*new_root_item));
+	btrfs_check_and_init_root_item(new_root_item);
 
 	root_flags = btrfs_root_flags(new_root_item);
 	if (pending->readonly)
Signature · 1 line
1.7.3.1
Tested-by on v1 1/1
#1 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 6 lines · Li Zefan

root_item->flags and root_item->byte_limit are not initialized when a subvolume is created. This bug is not revealed until we added readonly snapshot support - now you mount a btrfs filesystem and you may find the subvolumes in it are readonly.

To work around this problem, we steal a bit from

root_item->inode_item->flags,

Quoted 6 lines

and use it to indicate if those fields have been properly initialized. When we read a tree root from disk, we check if the bit is set, and if not we'll set the flag and initialize the two fields of the root item.

Reported-by Andreas Philipp <[email protected]>
Signed-off-by Li Zefan <[email protected]>
Tested-by Andreas Philipp <[email protected]>

fs/btrfs/ctree.h int btrfs_find_dead_roots(struct btrfs_root

Quoted 22 lines

---
fs/btrfs/ctree.h | 4 ++++
fs/btrfs/disk-io.c | 4 +++-
fs/btrfs/ioctl.c | 4 ++++
fs/btrfs/root-tree.c | 18 ++++++++++++++++++
fs/btrfs/transaction.c | 1 +

5 files changed, 30 insertions(+), 1 deletions(-)
fs/btrfs/ctree.h +2 -0
index 8b4b9d1..ff6b991 100644
@@ -1284,6 +1284,8 @@ struct btrfs_root {
 #define BTRFS_INODE_NOATIME (1 << 9)
 #define BTRFS_INODE_DIRSYNC (1 << 10)
 
+#define BTRFS_INODE_ROOT_ITEM_INIT (1 << 31)
+
 /* some macros to generate set/get funcs for the struct fields. This
 * assumes there is a lefoo_to_cpu for every type, so lets make a simple
 * one for u8:

@@ -2355,6 +2357,8 @@ int btrfs_find_dead_roots(struct btrfs_root

*root, u64 objectid);

fs/btrfs/disk-io.c struct btrfs_root

Quoted 13 lines
fs/btrfs/ctree.h +2 -0
 int btrfs_find_orphan_roots(struct btrfs_root *tree_root);
 int btrfs_set_root_node(struct btrfs_root_item *item,
 struct extent_buffer *node);
+void btrfs_check_and_init_root_item(struct btrfs_root_item *item);
+
 /* dir-item.c */
 int btrfs_insert_dir_item(struct btrfs_trans_handle *trans,
 struct btrfs_root *root, const char *name,
fs/btrfs/disk-io.c +0 -0
index 3e1ea3e..4f8dafc 100644
@@ -1184,8 +1184,10 @@ struct btrfs_root

*btrfs_read_fs_root_no_radix(struct btrfs_root *tree_root,

fs/btrfs/ioctl.c static noinline int create_subvol(struct

Quoted 16 lines
fs/btrfs/disk-io.c +0 -0
 root->commit_root = btrfs_root_node(root);
 BUG_ON(!root->node);

out:
- if (location->objectid != BTRFS_TREE_LOG_OBJECTID)
+ if (location->objectid != BTRFS_TREE_LOG_OBJECTID) { root->ref_cows = 1;
+ btrfs_check_and_init_root_item(&root->root_item);
+ }

return root;
}

fs/btrfs/ioctl.c +0 -0
index 5fdb2ab..2ff51e6 100644
@@ -294,6 +294,10 @@ static noinline int create_subvol(struct

btrfs_root *root,

fs/btrfs/transaction.c static noinline int create_pending_snapshot(struct

Quoted 41 lines
fs/btrfs/ioctl.c +4 -0
 inode_item->nbytes = cpu_to_le64(root->leafsize);
 inode_item->mode = cpu_to_le32(S_IFDIR | 0755);
 
+ root_item.flags = 0;
+ root_item.byte_limit = 0;
+ inode_item->flags = cpu_to_le64(BTRFS_INODE_ROOT_ITEM_INIT);
+
 btrfs_set_root_bytenr(&root_item, leaf->start);
 btrfs_set_root_generation(&root_item, trans->transid);
 btrfs_set_root_level(&root_item, 0);
fs/btrfs/root-tree.c +18 -0
index 6a1086e..3e45c32 100644
@@ -471,3 +471,21 @@ again:
 btrfs_free_path(path);
 return 0;
 }
+
+/*
+ * Old btrfs forgets to init root_item->flags and root_item->byte_limit
+ * for subvolumes. To work around this problem, we steal a bit from
+ * root_item->inode_item->flags, and use it to indicate if those fields
+ * have been properly initialized.
+ */
+void btrfs_check_and_init_root_item(struct btrfs_root_item *root_item)
+{
+ u64 inode_flags = le64_to_cpu(root_item->inode.flags);
+
+ if (!(inode_flags & BTRFS_INODE_ROOT_ITEM_INIT)) {
+ inode_flags |= BTRFS_INODE_ROOT_ITEM_INIT;
+ root_item->inode.flags = cpu_to_le64(inode_flags);
+ root_item->flags = 0;
+ root_item->byte_limit = 0;
+ }
+}
fs/btrfs/transaction.c +0 -0
index 3d73c8d..f3d6681 100644
@@ -970,6 +970,7 @@ static noinline int create_pending_snapshot(struct

btrfs_trans_handle *trans,

fs/btrfs/transaction.c static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans, · line 976

Quoted 7 lines
fs/btrfs/transaction.c +1 -0
 record_root_in_trans(trans, root);
 btrfs_set_root_last_snapshot(&root->root_item, trans->transid);
 memcpy(new_root_item, &root->root_item, sizeof(*new_root_item));
+ btrfs_check_and_init_root_item(new_root_item);
 
 root_flags = btrfs_root_flags(new_root_item);
 if (pending->readonly)

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJNktS2AAoJEJIcBJ3+XkgiB4gP/RqCnQQj2RcM0pxpjPqA0+GE HC5DqXGhqYa+6Y+WIKWZJIuhpxF+x5c885U7MBYU1f5UJB6bXttYVGE5Gq7pXgs5 aWezaX3WKIOsgfLWVRFezWtlmWvUuBgz3K2cLq9U+NcUNp13KjP4nI5QWwn0xo27 o4GjqdtHiN3Fen8/BCVUTHySUlFxqo2o7DvNB9KysW8Vz0zOS8Y0SXgUN+4u5p7L 7Y76UOiayMpVF6JaGQzcqGgf5WQyA8xWtYUAzuxBs+MA1Xfbogu4Oen59FKxR4BI c0Ihw2GddQfxHvGQLXXXK2BuCpZfbpTD3hdxRnAzRspk8OqpBU0DYnZv0YybsqLw XC+PM+oEUtsTAxaID0MWcoqfk40S54xv36PXgKF0RjIogJFRyxq1vNNCR5hzxVRX hjVvBc31QWyE+EkFZKBWnuVqnfVwHa0Ya9S93+dIVxD6uBcnQWSmuZagP3Awi7Xb S8BOwABMMVp4I17t0cdJYKzEI78NZ3Q3zt1W6whiYFH5xaP4YFhkXQAQJg40op3L h4bEcUo1+1o6QPyRV8IeiUGbNm+iU95I5tGWiU5Il2uYrbs5HYA4lvxwerNyFG34 mtJjBC/QdYJdMXa4DXri8vrxDYySJrcIVpBWu2VboxbeFpKrKZB+qLmzDaGO0soq 0xavNStqCTBWGGp4A0ux
=JVKt
-----END PGP SIGNATURE-----