mdadm 2.6.3 segfaults on assembly (v1 superblocks)

mdadm 2.6.3 segfaults on assembly (v1 superblocks)

[martin f krafft]

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

Hi,

preparing the Debian package for mdadm 2.6.3, I found a segfault in
mdadm/Assemble.c:254, in the line:

  } else if (tst->ss->load_super(tst,dfd, &super, NULL)) {

the problem is that tst->ss is NULL, due to reasons I have not yet
uncovered. The segfault happens only in the second iteration of the
for loop at line 212 and the load_super1 call, caused by the above
load_super in the first iteration, causes tst->ss to be set to NULL.

This happens in the first recursion (load_super1 calls itself), at
which point the

  if (dsize < 24) {

check in super1.c:1033 fails and thus returns 1, which causes the
outer load_super1 function to return 1 after setting st->ss to NULL
in line super1.c:1013.

This all happens while the dfd variable in Assemble.c:254 has value
8, and assuming this is a file descriptor, then lsof says:

  mdadm     25664     root    8r      BLK       22,3 2806 /dev/hdc3

/dev/hdc3 is an extended partition on the disk.

/dev/hdc1   *           1           8       64228+  83  Linux
/dev/hdc2               9         132      996030   82  Linux swap / Solaris
/dev/hdc3             133       30401   243135742+   5  Extended
/dev/hdc5             133         256      995998+  83  Linux
/dev/hdc6             257         505     2000061   83  Linux
/dev/hdc7             506       28347   223640833+  83  Linux
/dev/hdc8           28348       30339    16000708+  83  Linux
/dev/hdc9           30340       30401      497983+  83  Linux

I am failing to reproduce this on v0.9 superblock systems.

Neil, could this be a bug?

-- 
martin;              (greetings from the heart of the sun.)
  \____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
 
"nothing can cure the soul but the senses,
 just as nothing can cure the senses but the soul."
                                                        -- oscar wilde
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: mdadm 2.6.3 segfaults on assembly (v1 superblocks)

[Neil Brown]

On Friday September 7, madduck@madduck.net wrote:
> 
> Neil, could this be a bug?
> 

Sure could.  Thanks for the report.

This patch (already in .git) should fix it.

NeilBrown

---------------------------
Don't corrupt 'supertype' when speculatively calling load_super1

When load_super1 is trying to see which sub-version of v1 superblock
is present, failure will cause it to clear st->ss, which is not good.

So use a temporary 'super_type' for the 'test if this version works'
calls, then copy that into 'st' on success.

### Diffstat output
 ./super1.c |   19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff .prev/super1.c ./super1.c
--- .prev/super1.c	2007-09-24 14:26:19.000000000 +1000
+++ ./super1.c	2007-09-24 14:23:11.000000000 +1000
@@ -996,34 +996,35 @@ static int load_super1(struct supertype 
 
 	if (st->ss == NULL || st->minor_version == -1) {
 		int bestvers = -1;
+		struct supertype tst;
 		__u64 bestctime = 0;
 		/* guess... choose latest ctime */
-		st->ss = &super1;
-		for (st->minor_version = 0; st->minor_version <= 2 ; st->minor_version++) {
+		tst.ss = &super1;
+		for (tst.minor_version = 0; tst.minor_version <= 2 ; tst.minor_version++) {
 			switch(load_super1(st, fd, sbp, devname)) {
 			case 0: super = *sbp;
 				if (bestvers == -1 ||
 				    bestctime < __le64_to_cpu(super->ctime)) {
-					bestvers = st->minor_version;
+					bestvers = tst.minor_version;
 					bestctime = __le64_to_cpu(super->ctime);
 				}
 				free(super);
 				*sbp = NULL;
 				break;
-			case 1: st->ss = NULL; return 1; /*bad device */
+			case 1: return 1; /*bad device */
 			case 2: break; /* bad, try next */
 			}
 		}
 		if (bestvers != -1) {
 			int rv;
-			st->minor_version = bestvers;
-			st->ss = &super1;
-			st->max_devs = 384;
+			tst.minor_version = bestvers;
+			tst.ss = &super1;
+			tst.max_devs = 384;
 			rv = load_super1(st, fd, sbp, devname);
-			if (rv) st->ss = NULL;
+			if (rv == 0)
+				*st = tst;
 			return rv;
 		}
-		st->ss = NULL;
 		return 2;
 	}
 	if (!get_dev_size(fd, devname, &dsize))

Re: mdadm 2.6.3 segfaults on assembly (v1 superblocks)

[martin f krafft]

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

also sprach Neil Brown <neilb@suse.de> [2007.09.24.0528 +0100]:
> Sure could.  Thanks for the report.
> 
> This patch (already in .git) should fix it.

Apparently it does not, and it seems to be amd64-only since I saw it
on amd64 and a bunch of people reported success on i386:

  http://bugs.debian.org/444682

Any help appreciated. I don't have an amd64 system around for
another three weeks...

-- 
martin;              (greetings from the heart of the sun.)
  \____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
 
scientists will study your brain to learn
more about your distant cousin, man.
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]