All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Use disk sector size value to set offset for reading GPT
@ 2016-12-08 11:13 Mariusz Dabrowski
  2016-12-12 19:26 ` Jes Sorensen
  2016-12-13 16:38 ` Wols Lists
  0 siblings, 2 replies; 5+ messages in thread
From: Mariusz Dabrowski @ 2016-12-08 11:13 UTC (permalink / raw)
  To: linux-raid; +Cc: jes.sorensen, Mariusz Dabrowski

mdadm is using invalid byte-offset while reading GPT header to get
partition info (size, first sector, last sector etc.). Now this offset
is hardcoded to 512 bytes and it is not valid for disks with sector
size different than 512 bytes because MBR and GPT headers are aligned
to LBA, so valid offset for 4k drives is 4096 bytes.

Signed-off-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
---
 super-gpt.c | 10 ++++++++++
 util.c      |  7 ++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/super-gpt.c b/super-gpt.c
index 1a2adce..8b080a0 100644
--- a/super-gpt.c
+++ b/super-gpt.c
@@ -73,6 +73,7 @@ static int load_gpt(struct supertype *st, int fd, char *devname)
 	struct MBR *super;
 	struct GPT *gpt_head;
 	int to_read;
+	unsigned int sector_size;
 
 	free_gpt(st);
 
@@ -81,6 +82,11 @@ static int load_gpt(struct supertype *st, int fd, char *devname)
 		return 1;
 	}
 
+	if (!get_dev_sector_size(fd, devname, &sector_size)) {
+		free(super);
+		return 1;
+	}
+
 	lseek(fd, 0, 0);
 	if (read(fd, super, sizeof(*super)) != sizeof(*super)) {
 	no_read:
@@ -100,6 +106,8 @@ static int load_gpt(struct supertype *st, int fd, char *devname)
 		free(super);
 		return 1;
 	}
+	/* Set offset to second block (GPT header) */
+	lseek(fd, sector_size, SEEK_SET);
 	/* Seem to have GPT, load the header */
 	gpt_head = (struct GPT*)(super+1);
 	if (read(fd, gpt_head, sizeof(*gpt_head)) != sizeof(*gpt_head))
@@ -111,6 +119,8 @@ static int load_gpt(struct supertype *st, int fd, char *devname)
 
 	to_read = __le32_to_cpu(gpt_head->part_cnt) * sizeof(struct GPT_part_entry);
 	to_read =  ((to_read+511)/512) * 512;
+	/* Set offset to third block (GPT entries) */
+	lseek(fd, sector_size*2, SEEK_SET);
 	if (read(fd, gpt_head+1, to_read) != to_read)
 		goto no_read;
 
diff --git a/util.c b/util.c
index 883eaa4..818f839 100644
--- a/util.c
+++ b/util.c
@@ -1378,12 +1378,15 @@ static int get_gpt_last_partition_end(int fd, unsigned long long *endofpart)
 	unsigned long long curr_part_end;
 	unsigned all_partitions, entry_size;
 	unsigned part_nr;
+	unsigned int sector_size = 0;
 
 	*endofpart = 0;
 
 	BUILD_BUG_ON(sizeof(gpt) != 512);
 	/* skip protective MBR */
-	lseek(fd, 512, SEEK_SET);
+	if (!get_dev_sector_size(fd, NULL, &sector_size))
+		return 0;
+	lseek(fd, sector_size, SEEK_SET);
 	/* read GPT header */
 	if (read(fd, &gpt, 512) != 512)
 		return 0;
@@ -1403,6 +1406,8 @@ static int get_gpt_last_partition_end(int fd, unsigned long long *endofpart)
 
 	part = (struct GPT_part_entry *)buf;
 
+	/* set offset to third block (GPT entries) */
+	lseek(fd, sector_size*2, SEEK_SET);
 	for (part_nr = 0; part_nr < all_partitions; part_nr++) {
 		/* read partition entry */
 		if (read(fd, buf, entry_size) != (ssize_t)entry_size)
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] Use disk sector size value to set offset for reading GPT
  2016-12-08 11:13 [PATCH] Use disk sector size value to set offset for reading GPT Mariusz Dabrowski
@ 2016-12-12 19:26 ` Jes Sorensen
  2016-12-13 16:38 ` Wols Lists
  1 sibling, 0 replies; 5+ messages in thread
From: Jes Sorensen @ 2016-12-12 19:26 UTC (permalink / raw)
  To: Mariusz Dabrowski; +Cc: linux-raid

Mariusz Dabrowski <mariusz.dabrowski@intel.com> writes:
> mdadm is using invalid byte-offset while reading GPT header to get
> partition info (size, first sector, last sector etc.). Now this offset
> is hardcoded to 512 bytes and it is not valid for disks with sector
> size different than 512 bytes because MBR and GPT headers are aligned
> to LBA, so valid offset for 4k drives is 4096 bytes.
>
> Signed-off-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
> ---
>  super-gpt.c | 10 ++++++++++
>  util.c      |  7 ++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)

Applied!

Thanks,
Jes

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Use disk sector size value to set offset for reading GPT
  2016-12-08 11:13 [PATCH] Use disk sector size value to set offset for reading GPT Mariusz Dabrowski
  2016-12-12 19:26 ` Jes Sorensen
@ 2016-12-13 16:38 ` Wols Lists
  2016-12-14 11:04   ` Mariusz Dabrowski
  1 sibling, 1 reply; 5+ messages in thread
From: Wols Lists @ 2016-12-13 16:38 UTC (permalink / raw)
  To: Mariusz Dabrowski, linux-raid; +Cc: jes.sorensen

On 08/12/16 11:13, Mariusz Dabrowski wrote:
> mdadm is using invalid byte-offset while reading GPT header to get
> partition info (size, first sector, last sector etc.). Now this offset
> is hardcoded to 512 bytes and it is not valid for disks with sector
> size different than 512 bytes because MBR and GPT headers are aligned
> to LBA, so valid offset for 4k drives is 4096 bytes.
> 
> Signed-off-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>

Could this be behind the couple of incidents recently, where an array
has been moved from one machine to another, and the GPT has disappeared?

I know I've been following the threads, and I've been puzzled in that
I've thought "nothing should be writing there!"

If it is, what mdadm/kernels are affected, and I can put it on the wiki,
writing the page up about corrupted disks is next on my list.

Cheers,
Wol


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Use disk sector size value to set offset for reading GPT
  2016-12-13 16:38 ` Wols Lists
@ 2016-12-14 11:04   ` Mariusz Dabrowski
  2016-12-14 18:38     ` Wols Lists
  0 siblings, 1 reply; 5+ messages in thread
From: Mariusz Dabrowski @ 2016-12-14 11:04 UTC (permalink / raw)
  To: Wols Lists, linux-raid; +Cc: jes.sorensen

Hi,
this patch affects only reading GPT from disk. It fixes minor issue when 
there
were no warning from mdadm when user was trying to create 1-disk RAID 0 with
external metadata on disk with some partitions and given array size was 
smaller
than total size of all partitions on this disk (mdadm failed to read GPT
because it was using wrong offset).

I couldn't reproduce those incidents so I can't tell that this patch 
fixes the
root cause of this issue.

Regards,
Mariusz

On 12/13/2016 05:38 PM, Wols Lists wrote:
> On 08/12/16 11:13, Mariusz Dabrowski wrote:
>> mdadm is using invalid byte-offset while reading GPT header to get
>> partition info (size, first sector, last sector etc.). Now this offset
>> is hardcoded to 512 bytes and it is not valid for disks with sector
>> size different than 512 bytes because MBR and GPT headers are aligned
>> to LBA, so valid offset for 4k drives is 4096 bytes.
>>
>> Signed-off-by: Mariusz Dabrowski <mariusz.dabrowski@intel.com>
> Could this be behind the couple of incidents recently, where an array
> has been moved from one machine to another, and the GPT has disappeared?
>
> I know I've been following the threads, and I've been puzzled in that
> I've thought "nothing should be writing there!"
>
> If it is, what mdadm/kernels are affected, and I can put it on the wiki,
> writing the page up about corrupted disks is next on my list.
>
> Cheers,
> Wol
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Use disk sector size value to set offset for reading GPT
  2016-12-14 11:04   ` Mariusz Dabrowski
@ 2016-12-14 18:38     ` Wols Lists
  0 siblings, 0 replies; 5+ messages in thread
From: Wols Lists @ 2016-12-14 18:38 UTC (permalink / raw)
  To: Mariusz Dabrowski, linux-raid; +Cc: jes.sorensen

On 14/12/16 11:04, Mariusz Dabrowski wrote:
> Hi,
> this patch affects only reading GPT from disk. It fixes minor issue when
> there
> were no warning from mdadm when user was trying to create 1-disk RAID 0
> with
> external metadata on disk with some partitions and given array size was
> smaller
> than total size of all partitions on this disk (mdadm failed to read GPT
> because it was using wrong offset).
> 
In both instances I'm thinking of, iirc, a user pulled a working array
from one machine, tried to assemble it in another, then went back to the
original machine and the GPT had disappeared.

I'm clutching a bit at straws, but when the same bug bites more than one
user, you think something must be behind it. And as you can tell from
the scenario I describe, nothing *should* have been writing to disk, yet
the disk got corrupted.

> I couldn't reproduce those incidents so I can't tell that this patch
> fixes the
> root cause of this issue.

It'll have to stay on the "mysterious" list then :-)
> 
> Regards,
> Mariusz

Cheers,
Wol


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-12-14 18:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-08 11:13 [PATCH] Use disk sector size value to set offset for reading GPT Mariusz Dabrowski
2016-12-12 19:26 ` Jes Sorensen
2016-12-13 16:38 ` Wols Lists
2016-12-14 11:04   ` Mariusz Dabrowski
2016-12-14 18:38     ` Wols Lists

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.