From: "Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>
To: grub-devel@gnu.org
Subject: Re: [PATCH] segfault in grub-probe when spare or faulty disks are found
Date: Tue, 26 Oct 2010 10:58:09 +0200 [thread overview]
Message-ID: <4CC69821.20605@gmail.com> (raw)
In-Reply-To: <4CA0FF76.90207@mur.at>
[-- 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 --]
next prev parent reply other threads:[~2010-10-26 9:14 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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-09-29 12:16 ` Martin Schitter
2010-10-25 11:07 ` Martin Schitter
2010-10-25 11:11 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-10-26 8:58 ` Vladimir 'φ-coder/phcoder' Serbinenko [this message]
2010-10-26 15:29 ` Martin Schitter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4CC69821.20605@gmail.com \
--to=phcoder@gmail.com \
--cc=grub-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.