From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1PAfbl-0001s7-82 for mharc-grub-devel@gnu.org; Tue, 26 Oct 2010 05:14:17 -0400 Received: from [140.186.70.92] (port=55199 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PAfUw-0005vI-Js for grub-devel@gnu.org; Tue, 26 Oct 2010 05:07:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PAfMM-00005I-Ng for grub-devel@gnu.org; Tue, 26 Oct 2010 04:58:24 -0400 Received: from mail-fx0-f41.google.com ([209.85.161.41]:54428) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PAfMM-00005A-Cs for grub-devel@gnu.org; Tue, 26 Oct 2010 04:58:22 -0400 Received: by fxm2 with SMTP id 2so4102755fxm.0 for ; Tue, 26 Oct 2010 01:58:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from :user-agent:mime-version:to:subject:references:in-reply-to :x-enigmail-version:content-type; bh=zNCgvOqveRvoX7IKkBCIKmPHVd4d7xLcTCvKahyi/z8=; b=XJZrFkPCOrtTuPbnlXWzLhMJUXfJXBDhlRDpYS1jTiouPhv5ViB7ICCGkDmc8TFFGH PmoRQUwdwJW/cVr1WIXlewUEhOEINmXYVetdNjPjkwmuElVTow9q4a7+jL7Sm+h0lfG3 fiLb3mGdJd1xUUY9YPxMDVDw5PgzD+ndaNnZE= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:subject:references :in-reply-to:x-enigmail-version:content-type; b=w+YmUCwx+s/YlJL+07gjsC36pj2K70S7fzr+KSGRMmkGhEI/dUcZjENONLEZQYLjw9 zNbfkxGfchSxSJQR70YsM5vuhwKbJbgCcH8x+bi5h1mJrDnXgD74njbXJJYTvDtRo/eT rjeagqKzIBJjHEZDBHlDGJ3a6E99ziWzGmNkE= Received: by 10.103.219.14 with SMTP id w14mr1023741muq.21.1288083498389; Tue, 26 Oct 2010 01:58:18 -0700 (PDT) Received: from debian.bg45.phnet (cx-public-docking-1-059.ethz.ch [129.132.149.59]) by mx.google.com with ESMTPS id z25sm3402877fam.2.2010.10.26.01.58.16 (version=TLSv1/SSLv3 cipher=RC4-MD5); Tue, 26 Oct 2010 01:58:16 -0700 (PDT) Message-ID: <4CC69821.20605@gmail.com> Date: Tue, 26 Oct 2010 10:58:09 +0200 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.12) Gecko/20100917 Icedove/3.0.8 MIME-Version: 1.0 To: grub-devel@gnu.org References: <4CA0FF76.90207@mur.at> In-Reply-To: <4CA0FF76.90207@mur.at> X-Enigmail-Version: 1.0.1 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="------------enig152259694F0EAEAECB73DAB8" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) Subject: Re: [PATCH] segfault in grub-probe when spare or faulty disks are found X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Oct 2010 09:14:12 -0000 This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig152259694F0EAEAECB73DAB8 Content-Type: multipart/mixed; boundary="------------010102000002000909020208" This is a multi-part message in MIME format. --------------010102000002000909020208 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 09/27/2010 10:32 PM, Martin Schitter wrote: > the actual raid implementation doesn't handle spare or faulty drives > in a linux software raid array in a sane way. grub-probe will > reproducible crash in such a setup. > > please include the attached trivial patch to fix this problem. > Please try the attached patch > for more information see: > > https://savannah.gnu.org/bugs/?31119 > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=3D593652 > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > =20 --=20 Regards Vladimir '=CF=86-coder/phcoder' Serbinenko --------------010102000002000909020208 Content-Type: text/x-diff; name="raid.diff" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="raid.diff" =3D=3D=3D modified file 'grub-core/disk/mdraid1x_linux.c' --- grub-core/disk/mdraid1x_linux.c 2010-09-20 18:09:31 +0000 +++ grub-core/disk/mdraid1x_linux.c 2010-10-26 08:54:53 +0000 @@ -188,12 +188,14 @@ array->total_devs =3D grub_le_to_cpu32 (real_sb->raid_disks); array->disk_size =3D grub_le_to_cpu64 (real_sb->size); array->chunk_size =3D grub_le_to_cpu32 (real_sb->chunksize); - if (grub_le_to_cpu32 (real_sb->dev_number) < + + if (grub_le_to_cpu32 (real_sb->dev_number) >=3D grub_le_to_cpu32 (real_sb->max_dev)) - array->index =3D grub_le_to_cpu16 - (real_sb->dev_roles[grub_le_to_cpu32 (real_sb->dev_number)]); - else - array->index =3D 0xffff; /* disk will be later not used! */ + return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, + "spares aren't implemented"); + + array->index =3D grub_le_to_cpu16 + (real_sb->dev_roles[grub_le_to_cpu32 (real_sb->dev_number)]); array->uuid_len =3D 16; array->uuid =3D grub_malloc (16); if (!array->uuid) =3D=3D=3D modified file 'grub-core/disk/mdraid_linux.c' --- grub-core/disk/mdraid_linux.c 2010-09-20 18:09:31 +0000 +++ grub-core/disk/mdraid_linux.c 2010-10-26 08:53:39 +0000 @@ -194,6 +194,9 @@ sb.level !=3D 5 && sb.level !=3D 6 && sb.level !=3D 10) return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, "unsupported RAID level: %d", sb.level); + if (sb.this_disk.number =3D=3D 0xffff || sb.this_disk.number =3D=3D 0x= fffe) + return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, + "spares aren't implemented"); =20 array->name =3D NULL; array->number =3D sb.md_minor; =3D=3D=3D modified file 'grub-core/disk/raid.c' --- grub-core/disk/raid.c 2010-10-08 21:27:27 +0000 +++ grub-core/disk/raid.c 2010-10-26 08:52:04 +0000 @@ -97,10 +97,10 @@ unsigned int i; =20 for (i =3D 0; i < array->total_devs; i++) - if (array->device[i]) + if (array->members[i].device) { tmp =3D grub_malloc (sizeof (*tmp)); - tmp->disk =3D array->device[i]; + tmp->disk =3D array->members[i].device; tmp->next =3D list; list =3D tmp; } @@ -255,13 +255,13 @@ k =3D disknr; for (j =3D 0; j < far; j++) { - if (array->device[k]) + if (array->members[k].device) { if (grub_errno =3D=3D GRUB_ERR_READ_ERROR) grub_errno =3D GRUB_ERR_NONE; =20 - err =3D grub_disk_read (array->device[k], - array->start_sector[k] + + err =3D grub_disk_read (array->members[k].device= , + array->members[k].start_se= ctor + read_sector + j * far_of= s + b, 0, read_size << GRUB_DISK_SEC= TOR_BITS, @@ -367,14 +367,14 @@ read_size =3D size; =20 e =3D 0; - if (array->device[disknr]) + if (array->members[disknr].device) { /* Reset read error. */ if (grub_errno =3D=3D GRUB_ERR_READ_ERROR) grub_errno =3D GRUB_ERR_NONE; =20 - err =3D grub_disk_read (array->device[disknr], - array->start_sector[disknr] + + err =3D grub_disk_read (array->members[disknr].device, + array->members[disknr].start_secto= r + read_sector + b, 0, read_size << GRUB_DISK_SECTOR_BITS= , buf); @@ -500,6 +500,21 @@ =20 /* Do some checks before adding the device to the array. */ =20 + if (new_array->index >=3D array->allocated_devs) + { + void *tmp; + unsigned int newnum =3D 2 * (new_array->index + 1); + tmp =3D grub_realloc (array->members, newnum + * sizeof (array->members[0])); + if (!tmp) + return grub_errno; + array->members =3D tmp; + grub_memset (array->members + array->allocated_devs, + 0, (newnum - array->allocated_devs) + * sizeof (array->members[0])); + array->allocated_devs =3D newnum; + } + /* FIXME: Check whether the update time of the superblocks are the same. */ =20 @@ -510,7 +525,7 @@ grub_dprintf ("raid", "array->nr_devs > array->total_devs (%d)= ?!?", array->total_devs); =20 - if (array->device[new_array->index] !=3D NULL) + if (array->members[new_array->index].device !=3D NULL) /* We found multiple devices with the same number. Again, this shouldn't happen. */ grub_dprintf ("raid", "Found two disks with the number %d?!?",= @@ -536,8 +551,18 @@ #ifdef GRUB_UTIL array->driver =3D raid; #endif - grub_memset (&array->device, 0, sizeof (array->device)); - grub_memset (&array->start_sector, 0, sizeof (array->start_sector)= ); + array->allocated_devs =3D 32; + if (new_array->index >=3D array->allocated_devs) + array->allocated_devs =3D 2 * (new_array->index + 1); + + array->members =3D grub_zalloc (array->allocated_devs + * sizeof (array->members[0])); + + if (!array->members) + { + grub_free (new_array->uuid); + return grub_errno; + } =20 if (! array->name) { @@ -582,6 +607,7 @@ array->name =3D grub_xasprintf ("md%d", array->number); if (! array->name) { + grub_free (array->members); grub_free (array->uuid); grub_free (array); =20 @@ -597,6 +623,7 @@ =20 if (! new_name) { + grub_free (array->members); grub_free (array->uuid); grub_free (array); =20 @@ -621,8 +648,8 @@ } =20 /* Add the device to the array. */ - array->device[new_array->index] =3D disk; - array->start_sector[new_array->index] =3D start_sector; + array->members[new_array->index].device =3D disk; + array->members[new_array->index].start_sector =3D start_sector; array->nr_devs++; =20 return 0; @@ -639,14 +666,15 @@ while (array) { struct grub_raid_array *p; - int i; + unsigned int i; =20 p =3D array; array =3D array->next; =20 - for (i =3D 0; i < GRUB_RAID_MAX_DEVICES; i++) - if (p->device[i]) - grub_disk_close (p->device[i]); + for (i =3D 0; i < p->allocated_devs; i++) + if (p->members[i].device) + grub_disk_close (p->members[i].device); + grub_free (p->members); =20 grub_free (p->uuid); grub_free (p->name); =3D=3D=3D modified file 'grub-core/disk/raid5_recover.c' --- grub-core/disk/raid5_recover.c 2009-07-31 04:38:20 +0000 +++ grub-core/disk/raid5_recover.c 2010-10-26 08:34:44 +0000 @@ -45,7 +45,7 @@ if (i =3D=3D disknr) continue; =20 - err =3D grub_disk_read (array->device[i], sector, 0, size, buf2); + err =3D grub_disk_read (array->members[i].device, sector, 0, size,= buf2); =20 if (err) { =3D=3D=3D modified file 'grub-core/disk/raid6_recover.c' --- grub-core/disk/raid6_recover.c 2010-01-03 22:05:07 +0000 +++ grub-core/disk/raid6_recover.c 2010-10-26 08:38:03 +0000 @@ -118,8 +118,9 @@ bad1 =3D i; else { - if ((array->device[pos]) && - (! grub_disk_read (array->device[pos], sector, 0, size, bu= f))) + if ((array->members[pos].device) && + (! grub_disk_read (array->members[pos].device, sector, + 0, size, buf))) { grub_raid_block_xor (pbuf, buf, size); grub_raid_block_mul (raid6_table2[i][i], buf, size); @@ -148,21 +149,21 @@ if (bad2 < 0) { /* One bad device */ - if ((array->device[p]) && - (! grub_disk_read (array->device[p], sector, 0, size, buf))) + if ((array->members[p].device) && + (! grub_disk_read (array->members[p].device, sector, 0, size, = buf))) { grub_raid_block_xor (buf, pbuf, size); goto quit; } =20 - if (! array->device[q]) + if (! array->members[q].device) { grub_error (GRUB_ERR_READ_ERROR, "not enough disk to restore")= ; goto quit; } =20 grub_errno =3D GRUB_ERR_NONE; - if (grub_disk_read (array->device[q], sector, 0, size, buf)) + if (grub_disk_read (array->members[q].device, sector, 0, size, buf= )) goto quit; =20 grub_raid_block_xor (buf, qbuf, size); @@ -174,18 +175,18 @@ /* Two bad devices */ grub_uint8_t c; =20 - if ((! array->device[p]) || (! array->device[q])) + if ((! array->members[p].device) || (! array->members[q].device)) { grub_error (GRUB_ERR_READ_ERROR, "not enough disk to restore")= ; goto quit; } =20 - if (grub_disk_read (array->device[p], sector, 0, size, buf)) + if (grub_disk_read (array->members[p].device, sector, 0, size, buf= )) goto quit; =20 grub_raid_block_xor (pbuf, buf, size); =20 - if (grub_disk_read (array->device[q], sector, 0, size, buf)) + if (grub_disk_read (array->members[q].device, sector, 0, size, buf= )) goto quit; =20 grub_raid_block_xor (qbuf, buf, size); =3D=3D=3D modified file 'include/grub/raid.h' --- include/grub/raid.h 2010-10-08 21:27:27 +0000 +++ include/grub/raid.h 2010-10-26 08:55:53 +0000 @@ -22,8 +22,6 @@ =20 #include =20 -#define GRUB_RAID_MAX_DEVICES 32 - #define GRUB_RAID_LAYOUT_LEFT_ASYMMETRIC 0 #define GRUB_RAID_LAYOUT_RIGHT_ASYMMETRIC 1 #define GRUB_RAID_LAYOUT_LEFT_SYMMETRIC 2 @@ -32,6 +30,13 @@ #define GRUB_RAID_LAYOUT_RIGHT_MASK 1 #define GRUB_RAID_LAYOUT_SYMMETRIC_MASK 2 =20 +struct grub_raid_member +{ + grub_disk_t device; /* Array of total_devs devices. */ + grub_disk_addr_t start_sector; + /* Start of each device, in 512 byte sectors. */ +}; + struct grub_raid_array { int number; /* The device number, taken from md_minor so = we @@ -43,16 +48,15 @@ grub_size_t chunk_size; /* The size of a chunk, in 512 byte sectors. = */ grub_uint64_t disk_size; /* Size of an individual disk, in 512 byte sectors. */ - int index; /* Index of current device. */ + unsigned int index; /* Index of current device. */ int uuid_len; /* The length of uuid. */ char *uuid; /* The UUID of the device. */ =20 /* The following field is setup by the caller. */ char *name; /* That will be "md". */ unsigned int nr_devs; /* The number of devices we've found so far. = */ - grub_disk_t device[GRUB_RAID_MAX_DEVICES]; /* Array of total_devs dev= ices. */ - grub_disk_addr_t start_sector[GRUB_RAID_MAX_DEVICES]; - /* Start of each device, in 512 byte sectors. */ + unsigned int allocated_devs; + struct grub_raid_member *members; struct grub_raid_array *next; =20 #ifdef GRUB_UTIL --------------010102000002000909020208-- --------------enig152259694F0EAEAECB73DAB8 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iF4EAREKAAYFAkzGmCEACgkQNak7dOguQglEkQD9E0bCgtfVX7QInaq//ewPaM6B KhBAfU9+8DZuIEnaCcoBAK+eSZail6MeXopuzVPpQAD3dBjWtVZmGLwP/gj54gxA =evLq -----END PGP SIGNATURE----- --------------enig152259694F0EAEAECB73DAB8--