Skip to contentSkip to conversation

Btrfs-progs: fix unaligned acces for ioctl search header

PATCH New David Sterba 1 participant 4 messages May 3, 2016 Btrfs file system

lore
Share & tools
lore

David Sterba

author maintainer
#0 lore raw

cover Btrfs-progs: fix unaligned acces for ioctl search header

A user reports that some commands fail with SIGBUS on SPARC, due to unaligned access. We should really use the helpers as the search header data are read from a random position in the buffer that's returned from the TREE_SEARCH ioctl.

David Sterba (3):
btrfs-progs: kerncompat: introduce get_unaligned helpers
btrfs-progs: add getters for ioctl search_header
btrfs-progs: use ioctl search headers everywhere

 btrfs-fragments.c | 62 ++++++++++++++++++++++++++++++++++---------------------
 btrfs-list.c      | 21 ++++++++++---------
 cmds-fi-usage.c   |  8 +++----
 cmds-inspect.c    | 23 +++++++++++----------
 cmds-subvolume.c  | 13 ++++++------
 ctree.h           | 26 +++++++++++++++++++++++
 kerncompat.h      |  4 ++++
 qgroup.c          | 44 +++++++++++++++++++++++----------------
 send-utils.c      | 60 ++++++++++++++++++++++++++++++-----------------------
 uuid-tree.c       |  2 +-
 10 files changed, 163 insertions(+), 100 deletions(-)
Signature · 1 line
2.7.1
author maintainer
#1 lore raw

PATCH 1/3 btrfs-progs: kerncompat: introduce get_unaligned helpers

---
 kerncompat.h | 4 ++++
 1 file changed, 4 insertions(+)
kerncompat.h +4 -0
index ee65aa72ad6d..574f468226c2 100644
@@ -334,12 +334,16 @@ struct __una_u32 { __le32 x; } __attribute__((__packed__));
 struct __una_u64 { __le64 x; } __attribute__((__packed__));
 
 #define get_unaligned_le8(p) (*((u8 *)(p)))
+#define get_unaligned_8(p) (*((u8 *)(p)))
 #define put_unaligned_le8(val,p) ((*((u8 *)(p))) = (val))
 #define get_unaligned_le16(p) le16_to_cpu(((const struct __una_u16 *)(p))->x)
+#define get_unaligned_16(p) (((const struct __una_u16 *)(p))->x)
 #define put_unaligned_le16(val,p) (((struct __una_u16 *)(p))->x = cpu_to_le16(val))
 #define get_unaligned_le32(p) le32_to_cpu(((const struct __una_u32 *)(p))->x)
+#define get_unaligned_32(p) (((const struct __una_u32 *)(p))->x)
 #define put_unaligned_le32(val,p) (((struct __una_u32 *)(p))->x = cpu_to_le32(val))
 #define get_unaligned_le64(p) le64_to_cpu(((const struct __una_u64 *)(p))->x)
+#define get_unaligned_64(p) (((const struct __una_u64 *)(p))->x)
 #define put_unaligned_le64(val,p) (((struct __una_u64 *)(p))->x = cpu_to_le64(val))
 
 #ifndef true
Signature · 1 line
2.7.1
author maintainer
#2 lore raw

PATCH 2/3 btrfs-progs: add getters for ioctl search_header

In reply to David Sterba

The search header is usually accessed in an unaligned way, we could trigger errors (SIGBUS) on architectures that do not support that.

---
 ctree.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
ctree.h +26 -0
index 2da6f7786a78..13ed05052301 100644
@@ -2196,6 +2196,32 @@ static inline u32 btrfs_file_extent_inline_item_len(struct extent_buffer *eb,
        return btrfs_item_size(eb, e) - offset;
 }
 
+/* struct btrfs_ioctl_search_header */
+static inline u64 btrfs_search_header_transid(struct btrfs_ioctl_search_header *sh)
+{
+	return get_unaligned_64(&sh->transid);
+}
+
+static inline u64 btrfs_search_header_objectid(struct btrfs_ioctl_search_header *sh)
+{
+	return get_unaligned_64(&sh->objectid);
+}
+
+static inline u64 btrfs_search_header_offset(struct btrfs_ioctl_search_header *sh)
+{
+	return get_unaligned_64(&sh->offset);
+}
+
+static inline u32 btrfs_search_header_type(struct btrfs_ioctl_search_header *sh)
+{
+	return get_unaligned_32(&sh->type);
+}
+
+static inline u32 btrfs_search_header_len(struct btrfs_ioctl_search_header *sh)
+{
+	return get_unaligned_32(&sh->len);
+}
+
 /* this returns the number of file bytes represented by the inline item.
  * If an item is compressed, this is the uncompressed size
  */
Signature · 1 line
2.7.1
author maintainer
#3 lore raw

PATCH 3/3 btrfs-progs: use ioctl search headers everywhere

In reply to David Sterba

Generated by following semantic patch and manually tweaked.

<SmPL>
@@
struct btrfs_ioctl_search_header *SH;
@@
(
- SH->objectid
+ btrfs_search_header_objectid(SH)
|
- SH->offset
+ btrfs_search_header_offset(SH)
|
- SH->transid
+ btrfs_search_header_transid(SH)
|
- SH->len
+ btrfs_search_header_len(SH)
|
- SH->type
+ btrfs_search_header_type(SH)
)
</SmPL>

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=112131

---
 btrfs-fragments.c | 62 ++++++++++++++++++++++++++++++++++---------------------
 btrfs-list.c      | 21 ++++++++++---------
 cmds-fi-usage.c   |  8 +++----
 cmds-inspect.c    | 23 +++++++++++----------
 cmds-subvolume.c  | 13 ++++++------
 qgroup.c          | 44 +++++++++++++++++++++++----------------
 send-utils.c      | 60 ++++++++++++++++++++++++++++++-----------------------
 uuid-tree.c       |  2 +-
 8 files changed, 133 insertions(+), 100 deletions(-)
btrfs-fragments.c +38 -24
index 9db1861e9c7b..eb75eb7a7704 100644
@@ -245,7 +245,8 @@ list_fragments(int fd, u64 flags, char *dir)
 			sh = (struct btrfs_ioctl_search_header *)(args.buf +
 								  off);
 			off += sizeof(*sh);
-			if (sh->type == BTRFS_BLOCK_GROUP_ITEM_KEY) {
+			if (btrfs_search_header_type(sh)
+			    == BTRFS_BLOCK_GROUP_ITEM_KEY) {
 				struct btrfs_block_group_item *bg;
 
 				if (im) {
@@ -262,20 +263,24 @@ list_fragments(int fd, u64 flags, char *dir)
 						(args.buf + off);
 				bgflags = btrfs_block_group_flags(bg);
 				bgused = btrfs_block_group_used(bg);
-				
+
 				printf("found block group %lld len %lld "
-					"flags %lld\n", sh->objectid,
-					sh->offset, bgflags);
+					"flags %lld\n",
+					btrfs_search_header_objectid(sh),
+					btrfs_search_header_offset(sh),
+					bgflags);
 				if (!(bgflags & flags)) {
 					/* skip this block group */
-					sk->min_objectid = sh->objectid +
-							   sh->offset;
+					sk->min_objectid =
+					    btrfs_search_header_objectid(sh) +
+					    btrfs_search_header_offset(sh);
 					sk->min_type = 0;
 					sk->min_offset = 0;
 					break;
 				}
 				im = gdImageCreate(width,
-					(sh->offset / 4096 + 799) / width);
+					(btrfs_search_header_offset(sh)
+					 / 4096 + 799) / width);
 
 				black = gdImageColorAllocate(im, 0, 0, 0);  
 
@@ -283,8 +288,8 @@ list_fragments(int fd, u64 flags, char *dir)
 					colors[j] = black;
 
 				init_colors(im, colors);
-				bgstart = sh->objectid;
-				bglen = sh->offset;
+				bgstart = btrfs_search_header_objectid(sh);
+				bglen = btrfs_search_header_offset(sh);
 				bgend = bgstart + bglen;
 
 				snprintf(name, sizeof(name), "bg%d.png", bgnum);
@@ -303,7 +308,8 @@ list_fragments(int fd, u64 flags, char *dir)
 				areas = 0;
 				saved_len = 0;
 			}
-			if (im && sh->type == BTRFS_EXTENT_ITEM_KEY) {
+			if (im && btrfs_search_header_type(sh)
+					== BTRFS_EXTENT_ITEM_KEY) {
 				int c;
 				struct btrfs_extent_item *item;
 
@@ -311,40 +317,48 @@ list_fragments(int fd, u64 flags, char *dir)
 						(args.buf + off);
 
 				if (use_color)
-					c = colors[get_color(item, sh->len)];
+					c = colors[get_color(item,
+						btrfs_search_header_len(sh))];
 				else
 					c = black;
-				if (sh->objectid > bgend) {
+				if (btrfs_search_header_objectid(sh) > bgend) {
 					printf("WARN: extent %lld is without "
-						"block group\n", sh->objectid);
+						"block group\n",
+						btrfs_search_header_objectid(sh));
 					goto skip;
 				}
-				if (sh->objectid == bgend) {
-					saved_extent = sh->objectid;
-					saved_len = sh->offset;
+				if (btrfs_search_header_objectid(sh) == bgend) {
+					saved_extent =
+						btrfs_search_header_objectid(sh);
+					saved_len =
+						btrfs_search_header_offset(sh);
 					saved_color = c;
 					goto skip;
 				}
-				px = (sh->objectid - bgstart) / 4096;
-				for (j = 0; j < sh->offset / 4096; ++j) {
+				px = (btrfs_search_header_objectid(sh)
+					- bgstart) / 4096;
+				for (j = 0;
+				     j < btrfs_search_header_offset(sh) / 4096;
+				     ++j) {
 					int x = (px + j) % width;
 					int y = (px + j) / width;
 					gdImageSetPixel(im, x, y, c);
 				}
-				if (sh->objectid != last_end)
+				if (btrfs_search_header_objectid(sh) != last_end)
 					++areas;
-				last_end = sh->objectid + sh->offset;
+				last_end = btrfs_search_header_objectid(sh)
+					+ btrfs_search_header_offset(sh);
 skip:;
 			}
-			off += sh->len;
+			off += btrfs_search_header_len(sh);
 
 			/*
 			 * record the mins in sk so we can make sure the
 			 * next search doesn't repeat this root
 			 */
-			sk->min_objectid = sh->objectid;
-			sk->min_type = sh->type;
-			sk->min_offset = sh->offset;
+			sk->min_objectid = btrfs_search_header_objectid(sh);
+			sk->min_type = btrfs_search_header_type(sh);
+			sk->min_offset = btrfs_search_header_offset(sh);
 		}
 		sk->nr_items = 4096;
 
btrfs-list.c +11 -10
index 2da54bf706f3..27b23721b6c1 100644
@@ -876,9 +876,9 @@ static char *ino_resolve(int fd, u64 ino, u64 *cache_dirid, char **cache_name)
 	off = 0;
 	sh = (struct btrfs_ioctl_search_header *)(args.buf + off);
 
-	if (sh->type == BTRFS_INODE_REF_KEY) {
+	if (btrfs_search_header_type(sh) == BTRFS_INODE_REF_KEY) {
 		struct btrfs_inode_ref *ref;
-		dirid = sh->offset;
+		dirid = btrfs_search_header_offset(sh);
 
 		ref = (struct btrfs_inode_ref *)(sh + 1);
 		namelen = btrfs_stack_inode_ref_name_len(ref);
@@ -947,7 +947,7 @@ int btrfs_list_get_default_subvolume(int fd, u64 *default_id)
 
 	sh = (struct btrfs_ioctl_search_header *)args.buf;
 
-	if (sh->type == BTRFS_DIR_ITEM_KEY) {
+	if (btrfs_search_header_type(sh) == BTRFS_DIR_ITEM_KEY) {
 		struct btrfs_dir_item *di;
 		int name_len;
 		char *name;
@@ -1602,17 +1602,18 @@ static int print_one_extent(int fd, struct btrfs_ioctl_search_header *sh,
 	int flags = 0;
 	char *name = NULL;
 
-	if (sh->objectid == *cache_ino) {
+	if (btrfs_search_header_objectid(sh) == *cache_ino) {
 		name = *cache_full_name;
 	} else if (*cache_full_name) {
 		free(*cache_full_name);
 		*cache_full_name = NULL;
 	}
 	if (!name) {
-		name = ino_resolve(fd, sh->objectid, cache_dirid,
+		name = ino_resolve(fd, btrfs_search_header_objectid(sh),
+				   cache_dirid,
 				   cache_dir_name);
 		*cache_full_name = name;
-		*cache_ino = sh->objectid;
+		*cache_ino = btrfs_search_header_objectid(sh);
 	}
 	if (!name)
 		return -EIO;
@@ -1633,16 +1634,16 @@ static int print_one_extent(int fd, struct btrfs_ioctl_search_header *sh,
 		printf("unhandled extent type %d for inode %llu "
 		       "file offset %llu gen %llu\n",
 			type,
-			(unsigned long long)sh->objectid,
-			(unsigned long long)sh->offset,
+			(unsigned long long)btrfs_search_header_objectid(sh),
+			(unsigned long long)btrfs_search_header_offset(sh),
 			(unsigned long long)found_gen);
 
 		return -EIO;
 	}
 	printf("inode %llu file offset %llu len %llu disk start %llu "
 	       "offset %llu gen %llu flags ",
-	       (unsigned long long)sh->objectid,
-	       (unsigned long long)sh->offset,
+	       (unsigned long long)btrfs_search_header_objectid(sh),
+	       (unsigned long long)btrfs_search_header_offset(sh),
 	       (unsigned long long)len,
 	       (unsigned long long)disk_start,
 	       (unsigned long long)disk_offset,
cmds-fi-usage.c +4 -4
index 33bf403af569..32000ea0683b 100644
@@ -185,11 +185,11 @@ static int load_chunk_info(int fd, struct chunk_info **info_ptr, int *info_count
 				return 1;
 			}
 
-			off += sh->len;
+			off += btrfs_search_header_len(sh);
 
-			sk->min_objectid = sh->objectid;
-			sk->min_type = sh->type;
-			sk->min_offset = sh->offset+1;
+			sk->min_objectid = btrfs_search_header_objectid(sh);
+			sk->min_type = btrfs_search_header_type(sh);
+			sk->min_offset = btrfs_search_header_offset(sh)+1;
 
 		}
 		if (!sk->min_offset)	/* overflow */
cmds-inspect.c +12 -11
index abe4edf10f70..25e8c016136f 100644
@@ -537,32 +537,33 @@ static int print_min_dev_size(int fd, u64 devid)
 								  off);
 			off += sizeof(*sh);
 			extent = (struct btrfs_dev_extent *)(args.buf + off);
-			off += sh->len;
+			off += btrfs_search_header_len(sh);
 
-			sk->min_objectid = sh->objectid;
-			sk->min_type = sh->type;
-			sk->min_offset = sh->offset + 1;
+			sk->min_objectid = btrfs_search_header_objectid(sh);
+			sk->min_type = btrfs_search_header_type(sh);
+			sk->min_offset = btrfs_search_header_offset(sh) + 1;
 
-			if (sh->objectid != devid ||
-			    sh->type != BTRFS_DEV_EXTENT_KEY)
+			if (btrfs_search_header_objectid(sh) != devid ||
+			    btrfs_search_header_type(sh) != BTRFS_DEV_EXTENT_KEY)
 				continue;
 
 			len = btrfs_stack_dev_extent_length(extent);
 			min_size += len;
-			ret = add_dev_extent(&extents, sh->offset,
-					     sh->offset + len - 1, 0);
+			ret = add_dev_extent(&extents,
+				btrfs_search_header_offset(sh),
+				btrfs_search_header_offset(sh) + len - 1, 0);
 
 			if (!ret && last_pos != (u64)-1 &&
-			    last_pos != sh->offset)
+			    last_pos != btrfs_search_header_offset(sh))
 				ret = add_dev_extent(&holes, last_pos,
-						     sh->offset - 1, 1);
+					btrfs_search_header_offset(sh) - 1, 1);
 			if (ret) {
 				error("add device extent: %s", strerror(-ret));
 				ret = 1;
 				goto out;
 			}
 
-			last_pos = sh->offset + len;
+			last_pos = btrfs_search_header_offset(sh) + len;
 		}
 
 		if (sk->min_type != BTRFS_DEV_EXTENT_KEY ||
cmds-subvolume.c +7 -6
index 2319684e1500..7c8ab8f21d0c 100644
@@ -1142,7 +1142,8 @@ static int enumerate_dead_subvols(int fd, u64 **ids)
 			sh = (struct btrfs_ioctl_search_header*)(args.buf + off);
 			off += sizeof(*sh);
 
-			if (sh->type == BTRFS_ORPHAN_ITEM_KEY) {
+			if (btrfs_search_header_type(sh)
+			    == BTRFS_ORPHAN_ITEM_KEY) {
 				if (idx >= count) {
 					u64 *newids;
 
@@ -1153,14 +1154,14 @@ static int enumerate_dead_subvols(int fd, u64 **ids)
 						return -ENOMEM;
 					*ids = newids;
 				}
-				(*ids)[idx] = sh->offset;
+				(*ids)[idx] = btrfs_search_header_offset(sh);
 				idx++;
 			}
-			off += sh->len;
+			off += btrfs_search_header_len(sh);
 
-			sk->min_objectid = sh->objectid;
-			sk->min_type = sh->type;
-			sk->min_offset = sh->offset;
+			sk->min_objectid = btrfs_search_header_objectid(sh);
+			sk->min_type = btrfs_search_header_type(sh);
+			sk->min_offset = btrfs_search_header_offset(sh);
 		}
 		if (sk->min_offset < (u64)-1)
 			sk->min_offset++;
qgroup.c +26 -18

84 lines not on this page · raw message

send-utils.c +34 -26

131 lines not on this page · raw message

uuid-tree.c +1 -1
index 39c3f3fd21e6..8d0b9173238d 100644
@@ -72,7 +72,7 @@ static int btrfs_uuid_tree_lookup_any(int fd, const u8 *uuid, u8 type,
 		goto out;
 	}
 	search_header = (struct btrfs_ioctl_search_header *)(search_arg.buf);
-	item_size = search_header->len;
+	item_size = btrfs_search_header_len(search_header);
 	if ((item_size & (sizeof(u64) - 1)) || item_size == 0) {
 		printf("btrfs: uuid item with illegal size %lu!\n",
 		       (unsigned long)item_size);
Signature · 1 line
2.7.1