Re: [PATCH] Fix bus error when accessing MBR partition records

From
NeilBrown <hidden>
Date
2016-10-02 22:32:23
Reply to
James Clarke, 2016-09-29
On Thu, Sep 29 2016, James Clarke wrote:
Since the MBR layout only has partition records as 2-byte aligned, the 32-bit
fields in them are not aligned. Thus, they cannot be accessed on some
architectures (such as SPARC) by using a "struct MBR_part_record *" pointer,
as the compiler can assume that the pointer is properly aligned. Instead, the
records must be accessed by going through the MBR struct itself every time.
Weird....

Can you see if adding "__attribute__((packed))" to struct
MBR_part_record also fixes the problem?

It seems strange that the compiler lets you take a pointer, but then
doesn't use it correctly.  Maybe it is an inconsistency in the types.

I don't necessarily disagree with your fix, but I'd like to understand
why the current code is wrong.

Thanks,
NeilBrown

quoted hunk ↗ jump to hunk
Signed-off-by: James Clarke <redacted>
---
 super-mbr.c |  6 ++++++
 util.c      | 14 +++++++-------
 2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/super-mbr.c b/super-mbr.c
index 62b3f03..303dde4 100644
--- a/super-mbr.c
+++ b/super-mbr.c
@@ -57,6 +57,9 @@ static void examine_mbr(struct supertype *st, char *homehost)
 
 	printf("   MBR Magic : %04x\n", sb->magic);
 	for (i = 0; i < MBR_PARTITIONS; i++)
+		/* Have to make every access through sb rather than using a pointer to
+		 * the partition table (or an entry), since the entries are not
+		 * properly aligned. */
 		if (sb->parts[i].blocks_num)
 			printf("Partition[%d] : %12lu sectors at %12lu (type %02x)\n",
 			       i,
@@ -151,6 +154,9 @@ static void getinfo_mbr(struct supertype *st, struct mdinfo *info, char *map)
 	info->component_size = 0;
 
 	for (i = 0; i < MBR_PARTITIONS ; i++)
+		/* Have to make every access through sb rather than using a pointer to
+		 * the partition table (or an entry), since the entries are not
+		 * properly aligned. */
 		if (sb->parts[i].blocks_num) {
 			unsigned long last =
 				(unsigned long)__le32_to_cpu(sb->parts[i].blocks_num)
diff --git a/util.c b/util.c
index a238a21..08adbd5 100644
--- a/util.c
+++ b/util.c
@@ -1412,7 +1412,6 @@ static int get_gpt_last_partition_end(int fd, unsigned long long *endofpart)
 static int get_last_partition_end(int fd, unsigned long long *endofpart)
 {
 	struct MBR boot_sect;
-	struct MBR_part_record *part;
 	unsigned long long curr_part_end;
 	unsigned part_nr;
 	int retval = 0;
@@ -1429,21 +1428,22 @@ static int get_last_partition_end(int fd, unsigned long long *endofpart)
 	if (boot_sect.magic == MBR_SIGNATURE_MAGIC) {
 		retval = 1;
 		/* found the correct signature */
-		part = boot_sect.parts;
 
 		for (part_nr = 0; part_nr < MBR_PARTITIONS; part_nr++) {
+			/* Have to make every access through boot_sect rather than using a
+			 * pointer to the partition table (or an entry), since the entries
+			 * are not properly aligned. */
+
 			/* check for GPT type */
-			if (part->part_type == MBR_GPT_PARTITION_TYPE) {
+			if (boot_sect.parts[part_nr].part_type == MBR_GPT_PARTITION_TYPE) {
 				retval = get_gpt_last_partition_end(fd, endofpart);
 				break;
 			}
 			/* check the last used lba for the current partition  */
-			curr_part_end = __le32_to_cpu(part->first_sect_lba) +
-				__le32_to_cpu(part->blocks_num);
+			curr_part_end = __le32_to_cpu(boot_sect.parts[part_nr].first_sect_lba) +
+				__le32_to_cpu(boot_sect.parts[part_nr].blocks_num);
 			if (curr_part_end > *endofpart)
 				*endofpart = curr_part_end;
-
-			part++;
 		}
 	} else {
 		/* Unknown partition table */
-- 
2.10.0

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Attachments

Thread (4 messages) 4 messages, 2 authors, last reply 2016-10-05
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help