Hi Rich,
I noticed that there were some unchecked malloc and calloc return values.
That could result in NULL deref upon failed allocation. Based on what
you said about there not being much point in trying to recover from OOM
errors, I made the small textual change to convert e.g., malloc(n) to
xmalloc(g,n) where xmalloc now ends up calling g->abort_cb upon malloc
failure. Then any following dereference of the result is guaranteed to
be valid.
Since I did the same with the calloc->xcalloc transformation,
I needed a corresponding guestfs_safe_calloc function wrapper.
>From 9f39ef2f45b4d5fe93b69d5a42fab69ba0b3e633 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Thu, 2 Jul 2009 15:39:08 +0200
Subject: [PATCH] use xmalloc and xcalloc in generated code
* src/generator.ml (xmalloc): Define to guestfs_safe_malloc.
(xcalloc): Define to guestfs_safe_calloc.
[most generated code]: Fail immediately upon failure of otherwise-
unchecked malloc and calloc calls.
* src/guestfs.c: Include <stddef.h>.
(xalloc_oversized): Define.
* src/guestfs.h (guestfs_safe_calloc): Declare.
---
src/generator.ml | 44 +++++++++++++++++++++++---------------------
src/guestfs.c | 39 ++++++++++++++++++++++++++++++++++++++-
src/guestfs.h | 3 ++-
3 files changed, 63 insertions(+), 23 deletions(-)
diff --git a/src/generator.ml b/src/generator.ml
index abe7e89..da93b70 100755
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -121,7 +121,7 @@ type flags =
| NotInDocs (* do not add this function to documentation *)
let protocol_limit_warning =
- "Because of the message protocol, there is a transfer limit
+ "Because of the message protocol, there is a transfer limit
of somewhere between 2MB and 4MB. To transfer large files you should use
FTP."
@@ -6384,7 +6384,7 @@ Sys::Guestfs - Perl bindings for libguestfs
=head1 SYNOPSIS
use Sys::Guestfs;
-
+
my $h = Sys::Guestfs->new ();
$h->add_drive ('guest.img');
$h->launch ();
@@ -8132,6 +8132,8 @@ and generate_bindtests () =
#include \"guestfs_protocol.h\"
#define error guestfs_error
+#define xmalloc guestfs_safe_malloc
+#define xcalloc guestfs_safe_calloc
static void
print_strings (char * const* const argv)
@@ -8207,70 +8209,70 @@ print_strings (char * const* const argv)
pr " char **strs;\n";
pr " int n, i;\n";
pr " sscanf (val, \"%%d\", &n);\n";
- pr " strs = malloc ((n+1) * sizeof (char *));\n";
+ pr " strs = xmalloc (g, (n+1) * sizeof (char *));\n";
pr " for (i = 0; i < n; ++i) {\n";
- pr " strs[i] = malloc (16);\n";
+ pr " strs[i] = xmalloc (g, 16);\n";
pr " snprintf (strs[i], 16, \"%%d\", i);\n";
pr " }\n";
pr " strs[n] = NULL;\n";
pr " return strs;\n"
| RIntBool _ ->
pr " struct guestfs_int_bool *r;\n";
- pr " r = malloc (sizeof *r);\n";
+ pr " r = xmalloc (g, sizeof *r);\n";
pr " sscanf (val, \"%%\" SCNi32, &r->i);\n";
pr " r->b = 0;\n";
pr " return r;\n"
| RPVList _ ->
pr " struct guestfs_lvm_pv_list *r;\n";
pr " int i;\n";
- pr " r = malloc (sizeof *r);\n";
+ pr " r = xmalloc (g, sizeof *r);\n";
pr " sscanf (val, \"%%d\", &r->len);\n";
- pr " r->val = calloc (r->len, sizeof *r->val);\n";
+ pr " r->val = xcalloc (g, r->len, sizeof *r->val);\n";
pr " for (i = 0; i < r->len; ++i) {\n";
- pr " r->val[i].pv_name = malloc (16);\n";
+ pr " r->val[i].pv_name = xmalloc (g, 16);\n";
pr " snprintf (r->val[i].pv_name, 16, \"%%d\", i);\n";
pr " }\n";
pr " return r;\n"
| RVGList _ ->
pr " struct guestfs_lvm_vg_list *r;\n";
pr " int i;\n";
- pr " r = malloc (sizeof *r);\n";
+ pr " r = xmalloc (g, sizeof *r);\n";
pr " sscanf (val, \"%%d\", &r->len);\n";
- pr " r->val = calloc (r->len, sizeof *r->val);\n";
+ pr " r->val = xcalloc (g, r->len, sizeof *r->val);\n";
pr " for (i = 0; i < r->len; ++i) {\n";
- pr " r->val[i].vg_name = malloc (16);\n";
+ pr " r->val[i].vg_name = xmalloc (g, 16);\n";
pr " snprintf (r->val[i].vg_name, 16, \"%%d\", i);\n";
pr " }\n";
pr " return r;\n"
| RLVList _ ->
pr " struct guestfs_lvm_lv_list *r;\n";
pr " int i;\n";
- pr " r = malloc (sizeof *r);\n";
+ pr " r = xmalloc (g, sizeof *r);\n";
pr " sscanf (val, \"%%d\", &r->len);\n";
- pr " r->val = calloc (r->len, sizeof *r->val);\n";
+ pr " r->val = xcalloc (g, r->len, sizeof *r->val);\n";
pr " for (i = 0; i < r->len; ++i) {\n";
- pr " r->val[i].lv_name = malloc (16);\n";
+ pr " r->val[i].lv_name = xmalloc (g, 16);\n";
pr " snprintf (r->val[i].lv_name, 16, \"%%d\", i);\n";
pr " }\n";
pr " return r;\n"
| RStat _ ->
pr " struct guestfs_stat *r;\n";
- pr " r = calloc (1, sizeof (*r));\n";
+ pr " r = xcalloc (g, 1, sizeof (*r));\n";
pr " sscanf (val, \"%%\" SCNi64, &r->dev);\n";
pr " return r;\n"
| RStatVFS _ ->
pr " struct guestfs_statvfs *r;\n";
- pr " r = calloc (1, sizeof (*r));\n";
+ pr " r = xcalloc (g, 1, sizeof (*r));\n";
pr " sscanf (val, \"%%\" SCNi64, &r->bsize);\n";
pr " return r;\n"
| RHashtable _ ->
pr " char **strs;\n";
pr " int n, i;\n";
pr " sscanf (val, \"%%d\", &n);\n";
- pr " strs = malloc ((n*2+1) * sizeof (*strs));\n";
+ pr " strs = xmalloc (g, (n*2+1) * sizeof (*strs));\n";
pr " for (i = 0; i < n; ++i) {\n";
- pr " strs[i*2] = malloc (16);\n";
- pr " strs[i*2+1] = malloc (16);\n";
+ pr " strs[i*2] = xmalloc (g, 16);\n";
+ pr " strs[i*2+1] = xmalloc (g, 16);\n";
pr " snprintf (strs[i*2], 16, \"%%d\", i);\n";
pr " snprintf (strs[i*2+1], 16, \"%%d\", i);\n";
pr " }\n";
@@ -8279,9 +8281,9 @@ print_strings (char * const* const argv)
| RDirentList _ ->
pr " struct guestfs_dirent_list *r;\n";
pr " int i;\n";
- pr " r = malloc (sizeof *r);\n";
+ pr " r = xmalloc (g, sizeof *r);\n";
pr " sscanf (val, \"%%d\", &r->len);\n";
- pr " r->val = calloc (r->len, sizeof *r->val);\n";
+ pr " r->val = xcalloc (g, r->len, sizeof *r->val);\n";
pr " for (i = 0; i < r->len; ++i)\n";
pr " r->val[i].ino = i;\n";
pr " return r;\n"
diff --git a/src/guestfs.c b/src/guestfs.c
index c3bce0b..350d848 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -1,5 +1,5 @@
/* libguestfs
- * Copyright (C) 2009 Red Hat Inc.
+ * Copyright (C) 2009 Red Hat Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -24,6 +24,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>
+#include <stddef.h>
#include <unistd.h>
#include <ctype.h>
#include <string.h>
@@ -453,6 +454,42 @@ guestfs_safe_malloc (guestfs_h *g, size_t nbytes)
return ptr;
}
+/* Return 1 if an array of N objects, each of size S, cannot exist due
+ to size arithmetic overflow. S must be positive and N must be
+ nonnegative. This is a macro, not an inline function, so that it
+ works correctly even when SIZE_MAX < N.
+
+ By gnulib convention, SIZE_MAX represents overflow in size
+ calculations, so the conservative dividend to use here is
+ SIZE_MAX - 1, since SIZE_MAX might represent an overflowed value.
+ However, malloc (SIZE_MAX) fails on all known hosts where
+ sizeof (ptrdiff_t) <= sizeof (size_t), so do not bother to test for
+ exactly-SIZE_MAX allocations on such hosts; this avoids a test and
+ branch when S is known to be 1. */
+# define xalloc_oversized(n, s) \
+ ((size_t) (sizeof (ptrdiff_t) <= sizeof (size_t) ? -1 : -2) / (s) < (n))
+
+/* Technically we should add an autoconf test for this, testing for the desired
+ functionality, like what's done in gnulib, but for now, this is fine. */
+#define HAVE_GNU_CALLOC (__GLIBC__ >= 2)
+
+/* Allocate zeroed memory for N elements of S bytes, with error
+ checking. S must be nonzero. */
+void *
+guestfs_safe_calloc (guestfs_h *g, size_t n, size_t s)
+{
+ /* From gnulib's calloc function in xmalloc.c. */
+ void *p;
+ /* Test for overflow, since some calloc implementations don't have
+ proper overflow checks. But omit overflow and size-zero tests if
+ HAVE_GNU_CALLOC, since GNU calloc catches overflow and never
+ returns NULL if successful. */
+ if ((! HAVE_GNU_CALLOC && xalloc_oversized (n, s))
+ || (! (p = calloc (n, s)) && (HAVE_GNU_CALLOC || n != 0)))
+ g->abort_cb ();
+ return p;
+}
+
void *
guestfs_safe_realloc (guestfs_h *g, void *ptr, int nbytes)
{
diff --git a/src/guestfs.h b/src/guestfs.h
index 201d60c..264986f 100644
--- a/src/guestfs.h
+++ b/src/guestfs.h
@@ -1,5 +1,5 @@
/* libguestfs
- * Copyright (C) 2009 Red Hat Inc.
+ * Copyright (C) 2009 Red Hat Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -77,6 +77,7 @@ extern void guestfs_error (guestfs_h *g, const char *fs, ...)
extern void guestfs_perrorf (guestfs_h *g, const char *fs, ...)
__attribute__((format (printf,2,3)));
extern void *guestfs_safe_malloc (guestfs_h *g, size_t nbytes);
+extern void *guestfs_safe_calloc (guestfs_h *g, size_t n, size_t s);
extern void *guestfs_safe_realloc (guestfs_h *g, void *ptr, int nbytes);
extern char *guestfs_safe_strdup (guestfs_h *g, const char *str);
extern void *guestfs_safe_memdup (guestfs_h *g, void *ptr, size_t size);
--
1.6.3.3.507.gc6b5a
