On Sun, Jul 19, 2009 at 12:40 PM, Pavel Roskin<pro...@gnu.org> wrote:
> Quoting Pavel Roskin <pro...@gnu.org>:
>
>> ==11029== Conditional jump or move depends on uninitialised value(s)
>> ==11029== at 0x403CAE: grub_disk_adjust_range (disk.c:373)
>> ==11029== by 0x403D6C: grub_disk_read (disk.c:392)
>> ==11029== by 0x41CB8C: grub_xfs_read_inode (xfs.c:222)
>
> P.S. It looks like struct grub_xfs_inode hardcodes size 256, so every use of
> sizeof (struct grub_xfs_inode) is suspicious. Likewise, struct
> grub_fshelp_node and struct grub_xfs_data should not be used blindly with
> sizeof.
Hi,
Oh nice catch, it's indeed a serious problem, this patch should fix it.
--
Bean
diff --git a/fs/xfs.c b/fs/xfs.c
index 68a4b4f..f4ffcfd 100644
--- a/fs/xfs.c
+++ b/fs/xfs.c
@@ -46,7 +46,8 @@ struct grub_xfs_sblock
grub_uint8_t unused4[20];
grub_uint8_t label[12];
grub_uint8_t log2_bsize;
- grub_uint8_t unused5[2];
+ grub_uint8_t log2_sect;
+ grub_uint8_t log2_inode;
grub_uint8_t log2_inop;
grub_uint8_t log2_agblk;
grub_uint8_t unused6[67];
@@ -131,21 +132,19 @@ struct grub_xfs_dirblock_tail
struct grub_fshelp_node
{
struct grub_xfs_data *data;
- struct grub_xfs_inode inode;
grub_uint64_t ino;
int inode_read;
+ struct grub_xfs_inode inode[0];
};
struct grub_xfs_data
{
struct grub_xfs_sblock sblock;
- struct grub_xfs_inode *inode;
grub_disk_t disk;
int pos;
int bsize;
int agsize;
- struct grub_fshelp_node diropen;
-
+ struct grub_fshelp_node *diropen;
};
static grub_dl_t my_mod;
@@ -194,7 +193,7 @@ grub_xfs_inode_block (struct grub_xfs_data *data,
long long ag = GRUB_XFS_INO_AG (data, ino);
long long block;
- block = (inoinag >> 4) + ag * data->agsize;
+ block = (inoinag >> data->sblock.log2_inop) + ag * data->agsize;
block <<= (data->sblock.log2_bsize - GRUB_DISK_SECTOR_BITS);
return block;
}
@@ -205,7 +204,8 @@ grub_xfs_inode_offset (struct grub_xfs_data *data,
grub_uint64_t ino)
{
int inoag = GRUB_XFS_INO_INOINAG (data, ino);
- return (inoag & ((1 << 4) - 1)) << 8;
+ return ((inoag & ((1 << data->sblock.log2_inop) - 1)) <<
+ data->sblock.log2_inode);
}
@@ -218,7 +218,7 @@ grub_xfs_read_inode (struct grub_xfs_data *data, grub_uint64_t ino,
/* Read the inode. */
if (grub_disk_read (data->disk, block, offset,
- sizeof (struct grub_xfs_inode), inode))
+ 1 << data->sblock.log2_inode, inode))
return grub_errno;
if (grub_strncmp ((char *) inode->magic, "IN", 2))
@@ -236,7 +236,7 @@ grub_xfs_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
grub_xfs_extent *exts;
grub_uint64_t ret = 0;
- if (node->inode.format == XFS_INODE_FORMAT_BTREE)
+ if (node->inode[0].format == XFS_INODE_FORMAT_BTREE)
{
grub_uint64_t *keys;
@@ -244,8 +244,8 @@ grub_xfs_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
if (leaf == 0)
return 0;
- nrec = grub_be_to_cpu16 (node->inode.data.btree.numrecs);
- keys = &node->inode.data.btree.keys[0];
+ nrec = grub_be_to_cpu16 (node->inode[0].data.btree.numrecs);
+ keys = &node->inode[0].data.btree.keys[0];
do
{
int i;
@@ -264,7 +264,7 @@ grub_xfs_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
}
if (grub_disk_read (node->data->disk,
- grub_be_to_cpu64 (keys[i - 1 + XFS_INODE_EXTENTS])
+ grub_be_to_cpu64 (keys[i - 1 + nrec])
<< (node->data->sblock.log2_bsize
- GRUB_DISK_SECTOR_BITS),
0, node->data->sblock.bsize, leaf))
@@ -282,16 +282,16 @@ grub_xfs_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
} while (leaf->level);
exts = (grub_xfs_extent *) keys;
}
- else if (node->inode.format == XFS_INODE_FORMAT_EXT)
+ else if (node->inode[0].format == XFS_INODE_FORMAT_EXT)
{
- nrec = grub_be_to_cpu32 (node->inode.nextents);
- exts = &node->inode.data.extents[0];
+ nrec = grub_be_to_cpu32 (node->inode[0].nextents);
+ exts = &node->inode[0].data.extents[0];
}
else
{
grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
"xfs does not support inode format %d yet",
- node->inode.format);
+ node->inode[0].format);
return 0;
}
@@ -330,7 +330,7 @@ grub_xfs_read_file (grub_fshelp_node_t node,
{
return grub_fshelp_read_file (node->data->disk, node, read_hook,
pos, len, buf, grub_xfs_read_block,
- grub_be_to_cpu64 (node->inode.size),
+ grub_be_to_cpu64 (node->inode[0].size),
node->data->sblock.log2_bsize
- GRUB_DISK_SECTOR_BITS);
}
@@ -339,12 +339,12 @@ grub_xfs_read_file (grub_fshelp_node_t node,
static char *
grub_xfs_read_symlink (grub_fshelp_node_t node)
{
- int size = grub_be_to_cpu64 (node->inode.size);
+ int size = grub_be_to_cpu64 (node->inode[0].size);
- switch (node->inode.format)
+ switch (node->inode[0].format)
{
case XFS_INODE_FORMAT_INO:
- return grub_strndup (node->inode.data.raw, size);
+ return grub_strndup (node->inode[0].data.raw, size);
case XFS_INODE_FORMAT_EXT:
{
@@ -400,7 +400,8 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
{
struct grub_fshelp_node *fdiro;
- fdiro = grub_malloc (sizeof (struct grub_fshelp_node));
+ fdiro = grub_malloc (sizeof (struct grub_fshelp_node) +
+ (1 << diro->data->sblock.log2_inode));
if (!fdiro)
return 0;
@@ -409,19 +410,21 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
fdiro->ino = ino;
fdiro->inode_read = 1;
fdiro->data = diro->data;
- grub_xfs_read_inode (diro->data, ino, &fdiro->inode);
+ grub_xfs_read_inode (diro->data, ino, &fdiro->inode[0]);
return hook (filename,
- grub_xfs_mode_to_filetype (fdiro->inode.mode),
+ grub_xfs_mode_to_filetype (fdiro->inode[0].mode),
fdiro);
}
- switch (diro->inode.format)
+ switch (diro->inode[0].format)
{
case XFS_INODE_FORMAT_INO:
{
- struct grub_xfs_dir_entry *de = &diro->inode.data.dir.direntry[0];
- int smallino = !diro->inode.data.dir.dirhead.smallino;
+ struct grub_xfs_dir_entry *de =
+ &diro->inode[0].data.dir.direntry[0];
+ int smallino =
+ !diro->inode[0].data.dir.dirhead.smallino;
int i;
grub_uint64_t parent;
@@ -429,14 +432,15 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
parent inode number is small too. */
if (smallino)
{
- parent = grub_be_to_cpu32 (diro->inode.data.dir.dirhead.parent.i4);
+ parent =
+ grub_be_to_cpu32 (diro->inode[0].data.dir.dirhead.parent.i4);
parent = grub_cpu_to_be64 (parent);
/* The header is a bit smaller than usual. */
de = (struct grub_xfs_dir_entry *) ((char *) de - 4);
}
else
{
- parent = diro->inode.data.dir.dirhead.parent.i8;
+ parent = diro->inode[0].data.dir.dirhead.parent.i8;
}
/* Synthesize the direntries for `.' and `..'. */
@@ -446,7 +450,7 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
if (call_hook (parent, ".."))
return 1;
- for (i = 0; i < diro->inode.data.dir.dirhead.count; i++)
+ for (i = 0; i < diro->inode[0].data.dir.dirhead.count; i++)
{
grub_uint64_t ino;
void *inopos = (((char *) de)
@@ -493,7 +497,7 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
/* Iterate over every block the directory has. */
for (blk = 0;
- blk < (grub_be_to_cpu64 (dir->inode.size)
+ blk < (grub_be_to_cpu64 (dir->inode[0].size)
>> dirblk_log2);
blk++)
{
@@ -566,7 +570,7 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir,
default:
grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
"xfs does not support inode format %d yet",
- diro->inode.format);
+ diro->inode[0].format);
}
return 0;
}
@@ -577,7 +581,7 @@ grub_xfs_mount (grub_disk_t disk)
{
struct grub_xfs_data *data = 0;
- data = grub_malloc (sizeof (struct grub_xfs_data));
+ data = grub_zalloc (sizeof (struct grub_xfs_data));
if (!data)
return 0;
@@ -592,17 +596,21 @@ grub_xfs_mount (grub_disk_t disk)
goto fail;
}
- data->diropen.data = data;
- data->diropen.ino = data->sblock.rootino;
- data->diropen.inode_read = 1;
+ data->diropen = grub_malloc (sizeof (struct grub_fshelp_node) +
+ (1 << data->sblock.log2_inode));
+ if (! data->diropen)
+ goto fail;
+
+ data->diropen->data = data;
+ data->diropen->ino = data->sblock.rootino;
+ data->diropen->inode_read = 1;
data->bsize = grub_be_to_cpu32 (data->sblock.bsize);
data->agsize = grub_be_to_cpu32 (data->sblock.agsize);
data->disk = disk;
- data->inode = &data->diropen.inode;
data->pos = 0;
- grub_xfs_read_inode (data, data->diropen.ino, data->inode);
+ grub_xfs_read_inode (data, data->diropen->ino, &data->diropen->inode[0]);
return data;
fail:
@@ -610,6 +618,7 @@ grub_xfs_mount (grub_disk_t disk)
if (grub_errno == GRUB_ERR_OUT_OF_RANGE)
grub_error (GRUB_ERR_BAD_FS, "not an xfs filesystem");
+ grub_free (data->diropen);
grub_free (data);
return 0;
@@ -640,28 +649,30 @@ grub_xfs_dir (grub_device_t device, const char *path,
}
grub_dl_ref (my_mod);
-
+
data = grub_xfs_mount (device->disk);
if (!data)
- goto fail;
+ goto fail_2;
- grub_fshelp_find_file (path, &data->diropen, &fdiro, grub_xfs_iterate_dir,
+ grub_fshelp_find_file (path, data->diropen, &fdiro, grub_xfs_iterate_dir,
grub_xfs_read_symlink, GRUB_FSHELP_DIR);
if (grub_errno)
goto fail;
-
+
grub_xfs_iterate_dir (fdiro, iterate);
fail:
- if (fdiro != &data->diropen)
+ if (fdiro != data->diropen)
grub_free (fdiro);
+
+ grub_free (data->diropen);
grub_free (data);
+ fail_2:
+
grub_dl_unref (my_mod);
return grub_errno;
-
- return 0;
}
@@ -676,36 +687,40 @@ grub_xfs_open (struct grub_file *file, const char *name)
data = grub_xfs_mount (file->device->disk);
if (!data)
- goto fail;
+ goto fail_2;
- grub_fshelp_find_file (name, &data->diropen, &fdiro, grub_xfs_iterate_dir,
+ grub_fshelp_find_file (name, data->diropen, &fdiro, grub_xfs_iterate_dir,
grub_xfs_read_symlink, GRUB_FSHELP_REG);
if (grub_errno)
goto fail;
if (!fdiro->inode_read)
{
- grub_xfs_read_inode (data, fdiro->ino, &fdiro->inode);
+ grub_xfs_read_inode (data, fdiro->ino, &fdiro->inode[0]);
if (grub_errno)
goto fail;
}
- grub_memcpy (data->inode,
- &fdiro->inode,
- sizeof (struct grub_xfs_inode));
- grub_free (fdiro);
+ if (fdiro != data->diropen)
+ {
+ grub_free (data->diropen);
+ data->diropen = fdiro;
+ }
- file->size = grub_be_to_cpu64 (data->inode->size);
+ file->size = grub_be_to_cpu64 (data->diropen->inode[0].size);
file->data = data;
file->offset = 0;
return 0;
fail:
- if (fdiro != &data->diropen)
+ if (fdiro != data->diropen)
grub_free (fdiro);
+
+ grub_free (data->diropen);
grub_free (data);
+ fail_2:
grub_dl_unref (my_mod);
return grub_errno;
@@ -718,15 +733,22 @@ grub_xfs_read (grub_file_t file, char *buf, grub_size_t len)
struct grub_xfs_data *data =
(struct grub_xfs_data *) file->data;
- return grub_xfs_read_file (&data->diropen, file->read_hook,
- file->offset, len, buf);
+ return grub_xfs_read_file (data->diropen, file->read_hook,
+ file->offset, len, buf);
}
static grub_err_t
grub_xfs_close (grub_file_t file)
{
- grub_free (file->data);
+ struct grub_xfs_data *data;
+
+ data = (struct grub_xfs_data *) file->data;
+ if (data)
+ {
+ grub_free (data->diropen);
+ grub_free (data);
+ }
grub_dl_unref (my_mod);
@@ -744,12 +766,15 @@ grub_xfs_label (grub_device_t device, char **label)
data = grub_xfs_mount (disk);
if (data)
- *label = grub_strndup ((char *) (data->sblock.label), 12);
+ {
+ *label = grub_strndup ((char *) (data->sblock.label), 12);
+ grub_free (data->diropen);
+ }
else
*label = 0;
grub_dl_unref (my_mod);
-
+
grub_free (data);
return grub_errno;
@@ -768,16 +793,22 @@ grub_xfs_uuid (grub_device_t device, char **uuid)
{
*uuid = grub_malloc (sizeof ("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"));
grub_sprintf (*uuid, "%04x%04x-%04x-%04x-%04x-%04x%04x%04x",
- grub_be_to_cpu16 (data->sblock.uuid[0]), grub_be_to_cpu16 (data->sblock.uuid[1]),
- grub_be_to_cpu16 (data->sblock.uuid[2]), grub_be_to_cpu16 (data->sblock.uuid[3]),
- grub_be_to_cpu16 (data->sblock.uuid[4]), grub_be_to_cpu16 (data->sblock.uuid[5]),
- grub_be_to_cpu16 (data->sblock.uuid[6]), grub_be_to_cpu16 (data->sblock.uuid[7]));
+ grub_be_to_cpu16 (data->sblock.uuid[0]),
+ grub_be_to_cpu16 (data->sblock.uuid[1]),
+ grub_be_to_cpu16 (data->sblock.uuid[2]),
+ grub_be_to_cpu16 (data->sblock.uuid[3]),
+ grub_be_to_cpu16 (data->sblock.uuid[4]),
+ grub_be_to_cpu16 (data->sblock.uuid[5]),
+ grub_be_to_cpu16 (data->sblock.uuid[6]),
+ grub_be_to_cpu16 (data->sblock.uuid[7]));
+
+ grub_free (data->diropen);
}
else
*uuid = NULL;
grub_dl_unref (my_mod);
-
+
grub_free (data);
return grub_errno;
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
