Re: [PATCH] mdadm: fix --detail for cases where device count > max_devices/2

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2533 bytes --]

On Fri, 31 Oct 2014 11:10:33 +0200 Or Sagi <ors@reduxio.com> wrote:

> mdadm misreports (less devices then there are, and as a result decides the array is degraded) in cases where there are > max_devices/2 devices.
> 
> This seems to fix it.
> 
> ---
> Detail.c |    4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Detail.c b/Detail.c
> index c4fcad9..62e5867 100644
> --- a/Detail.c
> +++ b/Detail.c
> @@ -72,7 +72,7 @@ int Detail(char *dev, struct context *c)
> 	int failed = 0;
> 	struct supertype *st;
> 	char *subarray = NULL;
> -	int max_disks = MD_SB_DISKS; /* just a default */
> +	int max_disks = MD_SB_DISKS * 2; /* just a default */
> 	struct mdinfo *info = NULL;
> 	struct mdinfo *sra;
> 	struct mdinfo *subdev;
> @@ -124,7 +124,7 @@ int Detail(char *dev, struct context *c)
> 	rv = 0;
> 
> 	if (st)
> -		max_disks = st->max_devs;
> +		max_disks = st->max_devs * 2;
> 
> 	if (subarray) {
> 		/* This is a subarray of some container.

Thanks  for the report, but this patch is too simplistic.
Setting "max_disks" to twice the maximum is clearly wrong.
Then it should be called twice_max_disks.

The problem is that the 'disks' array has two entries for each 'raid_disk',
the primary disk and a possible replacement.  So we need to check that all
references to the size of the array, or indices to it, are treated properly.

I think this patch should fix it.  Can you test please?

Thanks,
NeilBrown

diff --git a/Detail.c b/Detail.c
index c4fcad9620ba..dd72eded995d 100644
--- a/Detail.c
+++ b/Detail.c
@@ -295,8 +295,8 @@ int Detail(char *dev, struct context *c)
 		goto out;
 	}
 
-	disks = xmalloc(max_disks * sizeof(mdu_disk_info_t));
-	for (d = 0; d < max_disks; d++) {
+	disks = xmalloc(max_disks * 2 * sizeof(mdu_disk_info_t));
+	for (d = 0; d < max_disks * 2; d++) {
 		disks[d].state = (1<<MD_DISK_REMOVED);
 		disks[d].major = disks[d].minor = 0;
 		disks[d].number = disks[d].raid_disk = d;
@@ -327,7 +327,7 @@ int Detail(char *dev, struct context *c)
 		else if (disk.raid_disk >= 0 && disk.raid_disk < array.raid_disks
 			 && disks[disk.raid_disk*2+1].state == (1<<MD_DISK_REMOVED))
 			disks[disk.raid_disk*2+1] = disk;
-		else if (next < max_disks)
+		else if (next < max_disks*2)
 			disks[next++] = disk;
 	}
 
@@ -602,7 +602,7 @@ This is pretty boring
 	}
 	free(info);
 
-	for (d= 0; d < max_disks; d++) {
+	for (d= 0; d < max_disks * 2; d++) {
 		char *dv;
 		mdu_disk_info_t disk = disks[d];
 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]