* Re: [PATCH] segfault in grub-probe when spare or faulty disks are found
2010-09-27 20:32 [PATCH] segfault in grub-probe when spare or faulty disks are found Martin Schitter
2010-09-28 20:24 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2010-10-26 8:58 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-10-26 15:29 ` Martin Schitter
1 sibling, 1 reply; 7+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2010-10-26 8:58 UTC (permalink / raw)
To: grub-devel
[-- Attachment #1.1: Type: text/plain, Size: 682 bytes --]
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=593652
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: raid.diff --]
[-- Type: text/x-diff; name="raid.diff", Size: 11187 bytes --]
=== 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 = grub_le_to_cpu32 (real_sb->raid_disks);
array->disk_size = grub_le_to_cpu64 (real_sb->size);
array->chunk_size = 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) >=
grub_le_to_cpu32 (real_sb->max_dev))
- array->index = grub_le_to_cpu16
- (real_sb->dev_roles[grub_le_to_cpu32 (real_sb->dev_number)]);
- else
- array->index = 0xffff; /* disk will be later not used! */
+ return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+ "spares aren't implemented");
+
+ array->index = grub_le_to_cpu16
+ (real_sb->dev_roles[grub_le_to_cpu32 (real_sb->dev_number)]);
array->uuid_len = 16;
array->uuid = grub_malloc (16);
if (!array->uuid)
=== 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 != 5 && sb.level != 6 && sb.level != 10)
return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
"unsupported RAID level: %d", sb.level);
+ if (sb.this_disk.number == 0xffff || sb.this_disk.number == 0xfffe)
+ return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+ "spares aren't implemented");
array->name = NULL;
array->number = sb.md_minor;
=== 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;
for (i = 0; i < array->total_devs; i++)
- if (array->device[i])
+ if (array->members[i].device)
{
tmp = grub_malloc (sizeof (*tmp));
- tmp->disk = array->device[i];
+ tmp->disk = array->members[i].device;
tmp->next = list;
list = tmp;
}
@@ -255,13 +255,13 @@
k = disknr;
for (j = 0; j < far; j++)
{
- if (array->device[k])
+ if (array->members[k].device)
{
if (grub_errno == GRUB_ERR_READ_ERROR)
grub_errno = GRUB_ERR_NONE;
- err = grub_disk_read (array->device[k],
- array->start_sector[k] +
+ err = grub_disk_read (array->members[k].device,
+ array->members[k].start_sector +
read_sector + j * far_ofs + b,
0,
read_size << GRUB_DISK_SECTOR_BITS,
@@ -367,14 +367,14 @@
read_size = size;
e = 0;
- if (array->device[disknr])
+ if (array->members[disknr].device)
{
/* Reset read error. */
if (grub_errno == GRUB_ERR_READ_ERROR)
grub_errno = GRUB_ERR_NONE;
- err = grub_disk_read (array->device[disknr],
- array->start_sector[disknr] +
+ err = grub_disk_read (array->members[disknr].device,
+ array->members[disknr].start_sector +
read_sector + b, 0,
read_size << GRUB_DISK_SECTOR_BITS,
buf);
@@ -500,6 +500,21 @@
/* Do some checks before adding the device to the array. */
+ if (new_array->index >= array->allocated_devs)
+ {
+ void *tmp;
+ unsigned int newnum = 2 * (new_array->index + 1);
+ tmp = grub_realloc (array->members, newnum
+ * sizeof (array->members[0]));
+ if (!tmp)
+ return grub_errno;
+ array->members = tmp;
+ grub_memset (array->members + array->allocated_devs,
+ 0, (newnum - array->allocated_devs)
+ * sizeof (array->members[0]));
+ array->allocated_devs = newnum;
+ }
+
/* FIXME: Check whether the update time of the superblocks are
the same. */
@@ -510,7 +525,7 @@
grub_dprintf ("raid", "array->nr_devs > array->total_devs (%d)?!?",
array->total_devs);
- if (array->device[new_array->index] != NULL)
+ if (array->members[new_array->index].device != 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 = raid;
#endif
- grub_memset (&array->device, 0, sizeof (array->device));
- grub_memset (&array->start_sector, 0, sizeof (array->start_sector));
+ array->allocated_devs = 32;
+ if (new_array->index >= array->allocated_devs)
+ array->allocated_devs = 2 * (new_array->index + 1);
+
+ array->members = grub_zalloc (array->allocated_devs
+ * sizeof (array->members[0]));
+
+ if (!array->members)
+ {
+ grub_free (new_array->uuid);
+ return grub_errno;
+ }
if (! array->name)
{
@@ -582,6 +607,7 @@
array->name = grub_xasprintf ("md%d", array->number);
if (! array->name)
{
+ grub_free (array->members);
grub_free (array->uuid);
grub_free (array);
@@ -597,6 +623,7 @@
if (! new_name)
{
+ grub_free (array->members);
grub_free (array->uuid);
grub_free (array);
@@ -621,8 +648,8 @@
}
/* Add the device to the array. */
- array->device[new_array->index] = disk;
- array->start_sector[new_array->index] = start_sector;
+ array->members[new_array->index].device = disk;
+ array->members[new_array->index].start_sector = start_sector;
array->nr_devs++;
return 0;
@@ -639,14 +666,15 @@
while (array)
{
struct grub_raid_array *p;
- int i;
+ unsigned int i;
p = array;
array = array->next;
- for (i = 0; i < GRUB_RAID_MAX_DEVICES; i++)
- if (p->device[i])
- grub_disk_close (p->device[i]);
+ for (i = 0; i < p->allocated_devs; i++)
+ if (p->members[i].device)
+ grub_disk_close (p->members[i].device);
+ grub_free (p->members);
grub_free (p->uuid);
grub_free (p->name);
=== 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 == disknr)
continue;
- err = grub_disk_read (array->device[i], sector, 0, size, buf2);
+ err = grub_disk_read (array->members[i].device, sector, 0, size, buf2);
if (err)
{
=== 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 = i;
else
{
- if ((array->device[pos]) &&
- (! grub_disk_read (array->device[pos], sector, 0, size, buf)))
+ 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;
}
- if (! array->device[q])
+ if (! array->members[q].device)
{
grub_error (GRUB_ERR_READ_ERROR, "not enough disk to restore");
goto quit;
}
grub_errno = 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;
grub_raid_block_xor (buf, qbuf, size);
@@ -174,18 +175,18 @@
/* Two bad devices */
grub_uint8_t c;
- 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;
}
- 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;
grub_raid_block_xor (pbuf, buf, size);
- 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;
grub_raid_block_xor (qbuf, buf, size);
=== 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 @@
#include <grub/types.h>
-#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
+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. */
/* The following field is setup by the caller. */
char *name; /* That will be "md<number>". */
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 devices. */
- 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;
#ifdef GRUB_UTIL
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread