[PATCH] list-filesystems: Do not segfault if guestfs_btrfs_subvolume_list returns an error (RHBZ#1064008).
If calling guestfs_list_filesystems with a disk image containing a corrupt btrfs volume, the library would segfault. There was a missing check for a NULL return from guestfs_btrfs_subvolume_list. This adds a check, returning the real error up through the stack and out of guestfs_list_filesystems. This is potentially a denial of service if processing disk images from untrusted sources, but is not exploitable. Thanks: Jeff Bastian for reporting the bug. --- src/listfs.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/src/listfs.c b/src/listfs.c index 9102c55..bbdb0a2 100644 --- a/src/listfs.c +++ b/src/listfs.c @@ -39,7 +39,7 @@ */ static void remove_from_list (char **list, const char *item); -static void check_with_vfs_type (guestfs_h *g, const char *dev, struct stringsbuf *sb); +static int check_with_vfs_type (guestfs_h *g, const char *dev, struct stringsbuf *sb); static int is_mbr_partition_type_42 (guestfs_h *g, const char *partition); char ** @@ -78,17 +78,21 @@ guestfs__list_filesystems (guestfs_h *g) /* Use vfs-type to check for filesystems on devices. */ for (i = 0; devices[i] != NULL; ++i) - check_with_vfs_type (g, devices[i], &ret); + if (check_with_vfs_type (g, devices[i], &ret) == -1) + goto error; /* Use vfs-type to check for filesystems on partitions. */ for (i = 0; partitions[i] != NULL; ++i) { - if (! is_mbr_partition_type_42 (g, partitions[i])) - check_with_vfs_type (g, partitions[i], &ret); + if (! is_mbr_partition_type_42 (g, partitions[i])) { + if (check_with_vfs_type (g, partitions[i], &ret) == -1) + goto error; + } } /* Use vfs-type to check for filesystems on md devices. */ for (i = 0; mds[i] != NULL; ++i) - check_with_vfs_type (g, mds[i], &ret); + if (check_with_vfs_type (g, mds[i], &ret) == -1) + goto error; if (guestfs_feature_available (g, (char **) lvm2)) { /* Use vfs-type to check for filesystems on LVs. */ @@ -96,7 +100,8 @@ guestfs__list_filesystems (guestfs_h *g) if (lvs == NULL) goto error; for (i = 0; lvs[i] != NULL; ++i) - check_with_vfs_type (g, lvs[i], &ret); + if (check_with_vfs_type (g, lvs[i], &ret) == -1) + goto error; } if (guestfs_feature_available (g, (char **) ldm)) { @@ -105,13 +110,15 @@ guestfs__list_filesystems (guestfs_h *g) if (ldmvols == NULL) goto error; for (i = 0; ldmvols[i] != NULL; ++i) - check_with_vfs_type (g, ldmvols[i], &ret); + if (check_with_vfs_type (g, ldmvols[i], &ret) == -1) + goto error; ldmparts = guestfs_list_ldm_partitions (g); if (ldmparts == NULL) goto error; for (i = 0; ldmparts[i] != NULL; ++i) - check_with_vfs_type (g, ldmparts[i], &ret); + if (check_with_vfs_type (g, ldmparts[i], &ret) == -1) + goto error; } /* Finish off the list and return it. */ @@ -143,7 +150,7 @@ remove_from_list (char **list, const char *item) * Apart from some types which we ignore, add the result to the * 'ret' string list. */ -static void +static int check_with_vfs_type (guestfs_h *g, const char *device, struct stringsbuf *sb) { const char *v; @@ -161,6 +168,9 @@ check_with_vfs_type (guestfs_h *g, const char *device, struct stringsbuf *sb) CLEANUP_FREE_BTRFSSUBVOLUME_LIST struct guestfs_btrfssubvolume_list *vols = guestfs_btrfs_subvolume_list (g, device); + if (vols == NULL) + return -1; + for (size_t i = 0; i < vols->len; i++) { struct guestfs_btrfssubvolume *this = &vols->val[i]; guestfs___add_sprintf (g, sb, @@ -178,17 +188,19 @@ check_with_vfs_type (guestfs_h *g, const char *device, struct stringsbuf *sb) */ size_t n = strlen (vfs_type); if (n >= 7 && STREQ (&vfs_type[n-7], "_member")) - return; + return 0; /* Ignore LUKS-encrypted partitions. These are also containers. */ if (STREQ (vfs_type, "crypto_LUKS")) - return; + return 0; v = vfs_type; } guestfs___add_string (g, sb, device); guestfs___add_string (g, sb, v); + + return 0; } /* We should ignore partitions that have MBR type byte 0x42, because -- 1.8.4.2
On Tuesday 11 February 2014 20:16:56 Richard W.M. Jones wrote:
If calling guestfs_list_filesystems with a disk image containing a corrupt btrfs volume, the library would segfault. There was a missing check for a NULL return from guestfs_btrfs_subvolume_list.
This adds a check, returning the real error up through the stack and out of guestfs_list_filesystems.
This is potentially a denial of service if processing disk images from untrusted sources, but is not exploitable.
Thanks: Jeff Bastian for reporting the bug. --- src/listfs.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-)
diff --git a/src/listfs.c b/src/listfs.c index 9102c55..bbdb0a2 100644 --- a/src/listfs.c +++ b/src/listfs.c @@ -39,7 +39,7 @@ */
static void remove_from_list (char **list, const char *item); -static void check_with_vfs_type (guestfs_h *g, const char *dev, struct stringsbuf *sb); +static int check_with_vfs_type (guestfs_h *g, const char *dev, struct stringsbuf *sb); static int is_mbr_partition_type_42 (guestfs_h *g, const char *partition);
char ** @@ -78,17 +78,21 @@ guestfs__list_filesystems (guestfs_h *g)
/* Use vfs-type to check for filesystems on devices. */ for (i = 0; devices[i] != NULL; ++i) - check_with_vfs_type (g, devices[i], &ret); + if (check_with_vfs_type (g, devices[i], &ret) == -1) + goto error;
/* Use vfs-type to check for filesystems on partitions. */ for (i = 0; partitions[i] != NULL; ++i) { - if (! is_mbr_partition_type_42 (g, partitions[i])) - check_with_vfs_type (g, partitions[i], &ret); + if (! is_mbr_partition_type_42 (g, partitions[i])) { + if (check_with_vfs_type (g, partitions[i], &ret) == -1) + goto error; + } }
/* Use vfs-type to check for filesystems on md devices. */ for (i = 0; mds[i] != NULL; ++i) - check_with_vfs_type (g, mds[i], &ret); + if (check_with_vfs_type (g, mds[i], &ret) == -1) + goto error;
if (guestfs_feature_available (g, (char **) lvm2)) { /* Use vfs-type to check for filesystems on LVs. */ @@ -96,7 +100,8 @@ guestfs__list_filesystems (guestfs_h *g) if (lvs == NULL) goto error;
for (i = 0; lvs[i] != NULL; ++i) - check_with_vfs_type (g, lvs[i], &ret); + if (check_with_vfs_type (g, lvs[i], &ret) == -1) + goto error; }
if (guestfs_feature_available (g, (char **) ldm)) { @@ -105,13 +110,15 @@ guestfs__list_filesystems (guestfs_h *g) if (ldmvols == NULL) goto error;
for (i = 0; ldmvols[i] != NULL; ++i) - check_with_vfs_type (g, ldmvols[i], &ret); + if (check_with_vfs_type (g, ldmvols[i], &ret) == -1) + goto error;
ldmparts = guestfs_list_ldm_partitions (g); if (ldmparts == NULL) goto error;
for (i = 0; ldmparts[i] != NULL; ++i) - check_with_vfs_type (g, ldmparts[i], &ret); + if (check_with_vfs_type (g, ldmparts[i], &ret) == -1) + goto error; }
/* Finish off the list and return it. */ @@ -143,7 +150,7 @@ remove_from_list (char **list, const char *item) * Apart from some types which we ignore, add the result to the * 'ret' string list. */ -static void +static int check_with_vfs_type (guestfs_h *g, const char *device, struct stringsbuf *sb) { const char *v; @@ -161,6 +168,9 @@ check_with_vfs_type (guestfs_h *g, const char *device, struct stringsbuf *sb) CLEANUP_FREE_BTRFSSUBVOLUME_LIST struct guestfs_btrfssubvolume_list *vols = guestfs_btrfs_subvolume_list (g, device);
+ if (vols == NULL) + return -1; + for (size_t i = 0; i < vols->len; i++) { struct guestfs_btrfssubvolume *this = &vols->val[i]; guestfs___add_sprintf (g, sb, @@ -178,17 +188,19 @@ check_with_vfs_type (guestfs_h *g, const char *device, struct stringsbuf *sb) */ size_t n = strlen (vfs_type); if (n >= 7 && STREQ (&vfs_type[n-7], "_member")) - return; + return 0;
/* Ignore LUKS-encrypted partitions. These are also containers. */ if (STREQ (vfs_type, "crypto_LUKS")) - return; + return 0;
v = vfs_type; }
guestfs___add_string (g, sb, device); guestfs___add_string (g, sb, v); + + return 0; }
/* We should ignore partitions that have MBR type byte 0x42, because
Looks good to me. -- Pino Toscano
On Wed, Feb 12, 2014 at 11:38:22AM +0100, Pino Toscano wrote:
Looks good to me.
Thanks for reviewing this. I'm going to push this to libguestfs 1.22 & 1.24 and do new releases of those branches as well .. coming up later today. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
participants (2)
-
Pino Toscano -
Richard W.M. Jones