|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 14/21] libxl: Do not pass NULL as gc_opt; introduce NOGC
In 25182:6c3345d7e9d9 the practice of passing NULL to gc-using memory
allocation functions was introduced. However, the arrangements there
were not correct as committed, because the error handling and logging
depends on getting a ctx from the gc - so an allocation error would in
fact result in libxl dereferencing NULL.
Instead, provide a special dummy gc in the ctx, called `nogc_gc'. It
is marked out specially by having alloc_maxsize==-1, which is
otherwise invalid.
Functions which need to actually look into the gc use the new test
function gc_is_real (whose purpose is mainly clarity of the code) to
check whether the gc is the dummy one, and do nothing if it is. And
we provide a helper macro NOGC which uses the in-scope real gc to find
the ctx and hence the dummy gc (and which replaces the previous
#define NOGC NULL).
Change all callers which pass 0 or NULL to an allocation function to
use NOGC or &ctx->nogc_gc, as applicable in the context.
We add a comment near the definition of LIBXL_INIT_GC pointing out
that it isn't any more the only place a libxl__gc struct is
initialised, for the benefit of anyone changing the contents of gc's
in the future.
Also, actually document that libxl__ptr_add is legal with ptr==NULL,
and change a couple of calls not to check for NULL argument.
Reported-by: Bamvor Jian Zhang <bjzhang@xxxxxxxx>
Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Bamvor Jian Zhang <bjzhang@xxxxxxxx>
Acked-by: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
---
tools/libxl/libxl.c | 3 +++
tools/libxl/libxl_aoutils.c | 3 ++-
tools/libxl/libxl_create.c | 2 +-
tools/libxl/libxl_event.c | 5 +++--
tools/libxl/libxl_exec.c | 2 +-
tools/libxl/libxl_fork.c | 2 +-
tools/libxl/libxl_internal.c | 11 +++++++++--
tools/libxl/libxl_internal.h | 37 +++++++++++++++++++++++--------------
tools/libxl/libxl_utils.c | 6 ++----
tools/libxl/libxl_xshelp.c | 7 ++-----
10 files changed, 47 insertions(+), 31 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index ce990ab..d3b017b 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -41,6 +41,9 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
/* First initialise pointers etc. (cannot fail) */
+ ctx->nogc_gc.alloc_maxsize = -1;
+ ctx->nogc_gc.owner = ctx;
+
LIBXL_TAILQ_INIT(&ctx->occurred);
ctx->osevent_hooks = 0;
diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
index 7f8d6d3..99972a2 100644
--- a/tools/libxl/libxl_aoutils.c
+++ b/tools/libxl/libxl_aoutils.c
@@ -77,6 +77,7 @@ static void datacopier_check_state(libxl__egc *egc,
libxl__datacopier_state *dc)
void libxl__datacopier_prefixdata(libxl__egc *egc, libxl__datacopier_state *dc,
const void *data, size_t len)
{
+ EGC_GC;
libxl__datacopier_buf *buf;
/*
* It is safe for this to be called immediately after _start, as
@@ -88,7 +89,7 @@ void libxl__datacopier_prefixdata(libxl__egc *egc,
libxl__datacopier_state *dc,
assert(len < dc->maxsz - dc->used);
- buf = libxl__zalloc(0, sizeof(*buf) - sizeof(buf->buf) + len);
+ buf = libxl__zalloc(NOGC, sizeof(*buf) - sizeof(buf->buf) + len);
buf->used = len;
memcpy(buf->buf, data, len);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 00705d8..ab000bc 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -108,7 +108,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
}
if (b_info->blkdev_start == NULL)
- b_info->blkdev_start = libxl__strdup(0, "xvda");
+ b_info->blkdev_start = libxl__strdup(NOGC, "xvda");
if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
if (!b_info->u.hvm.bios)
diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 565d2c2..eb23a93 100644
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -772,7 +772,7 @@ static int beforepoll_internal(libxl__gc *gc, libxl__poller
*poller,
if (poller->fd_rindices_allocd < maxfd) {
assert(ARRAY_SIZE_OK(poller->fd_rindices, maxfd));
poller->fd_rindices =
- libxl__realloc(0, poller->fd_rindices,
+ libxl__realloc(NOGC, poller->fd_rindices,
maxfd * sizeof(*poller->fd_rindices));
memset(poller->fd_rindices + poller->fd_rindices_allocd,
0,
@@ -1099,9 +1099,10 @@ void libxl_event_free(libxl_ctx *ctx, libxl_event *event)
libxl_event *libxl__event_new(libxl__egc *egc,
libxl_event_type type, uint32_t domid)
{
+ EGC_GC;
libxl_event *ev;
- ev = libxl__zalloc(0,sizeof(*ev));
+ ev = libxl__zalloc(NOGC,sizeof(*ev));
ev->type = type;
ev->domid = domid;
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index 082bf44..cfa379c 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -280,7 +280,7 @@ int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state
*ss)
int status, rc;
libxl__spawn_init(ss);
- ss->ssd = libxl__zalloc(0, sizeof(*ss->ssd));
+ ss->ssd = libxl__zalloc(NOGC, sizeof(*ss->ssd));
libxl__ev_child_init(&ss->ssd->mid);
rc = libxl__ev_time_register_rel(gc, &ss->timeout,
diff --git a/tools/libxl/libxl_fork.c b/tools/libxl/libxl_fork.c
index 9ff99e0..044ddad 100644
--- a/tools/libxl/libxl_fork.c
+++ b/tools/libxl/libxl_fork.c
@@ -92,7 +92,7 @@ libxl__carefd *libxl__carefd_record(libxl_ctx *ctx, int fd)
libxl__carefd *cf = 0;
libxl_fd_set_cloexec(ctx, fd, 1);
- cf = libxl__zalloc(NULL, sizeof(*cf));
+ cf = libxl__zalloc(&ctx->nogc_gc, sizeof(*cf));
cf->fd = fd;
LIBXL_LIST_INSERT_HEAD(&carefds, cf, entry);
return cf;
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 8139520..fbff7d0 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -30,11 +30,16 @@ void libxl__alloc_failed(libxl_ctx *ctx, const char *func,
#undef L
}
+static int gc_is_real(const libxl__gc *gc)
+{
+ return gc->alloc_maxsize >= 0;
+}
+
void libxl__ptr_add(libxl__gc *gc, void *ptr)
{
int i;
- if (!gc)
+ if (!gc_is_real(gc))
return;
if (!ptr)
@@ -66,6 +71,8 @@ void libxl__free_all(libxl__gc *gc)
void *ptr;
int i;
+ assert(gc_is_real(gc));
+
for (i = 0; i < gc->alloc_maxsize; i++) {
ptr = gc->alloc_ptrs[i];
gc->alloc_ptrs[i] = NULL;
@@ -104,7 +111,7 @@ void *libxl__realloc(libxl__gc *gc, void *ptr, size_t
new_size)
if (ptr == NULL) {
libxl__ptr_add(gc, new_ptr);
- } else if (new_ptr != ptr && gc != NULL) {
+ } else if (new_ptr != ptr && gc_is_real(gc)) {
for (i = 0; i < gc->alloc_maxsize; i++) {
if (gc->alloc_ptrs[i] == ptr) {
gc->alloc_ptrs[i] = new_ptr;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index c9b4189..aa150b5 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -277,10 +277,18 @@ struct libxl__poller {
int wakeup_pipe[2]; /* 0 means no fd allocated */
};
+struct libxl__gc {
+ /* mini-GC */
+ int alloc_maxsize; /* -1 means this is the dummy non-gc gc */
+ void **alloc_ptrs;
+ libxl_ctx *owner;
+};
+
struct libxl__ctx {
xentoollog_logger *lg;
xc_interface *xch;
struct xs_handle *xsh;
+ libxl__gc nogc_gc;
const libxl_event_hooks *event_hooks;
void *event_hooks_user;
@@ -356,13 +364,6 @@ typedef struct {
#define PRINTF_ATTRIBUTE(x, y) __attribute__((format(printf, x, y)))
-struct libxl__gc {
- /* mini-GC */
- int alloc_maxsize;
- void **alloc_ptrs;
- libxl_ctx *owner;
-};
-
struct libxl__egc {
/* For event-generating functions only.
* The egc and its gc may be accessed only on the creating thread. */
@@ -420,6 +421,7 @@ struct libxl__ao {
(gc).alloc_ptrs = 0; \
(gc).owner = (ctx); \
} while(0)
+ /* NB, also, a gc struct ctx->nogc_gc is initialised in libxl_ctx_alloc */
static inline libxl_ctx *libxl__gc_owner(libxl__gc *gc)
{
@@ -438,13 +440,20 @@ static inline libxl_ctx *libxl__gc_owner(libxl__gc *gc)
* All pointers returned by these functions are registered for garbage
* collection on exit from the outermost libxl callframe.
*
- * However, where the argument is stated to be "gc_opt", NULL may be
- * passed instead, in which case no garbage collection will occur; the
- * pointer must later be freed with free(). This is for memory
- * allocations of types (b) and (c).
+ * However, where the argument is stated to be "gc_opt", &ctx->nogc_gc
+ * may be passed instead, in which case no garbage collection will
+ * occur; the pointer must later be freed with free(). (Passing NULL
+ * for gc_opt is not permitted.) This is for memory allocations of
+ * types (b) and (c). The convenience macro NOGC should be used where
+ * possible.
+ *
+ * NOGC (and ctx->nogc_gc) may ONLY be used with functions which
+ * explicitly declare that it's OK. Use with nonconsenting functions
+ * may result in leaks of those functions' internal allocations on the
+ * psuedo-gc.
*/
-/* register @ptr in @gc for free on exit from outermost libxl callframe. */
-_hidden void libxl__ptr_add(libxl__gc *gc_opt, void *ptr);
+/* register ptr in gc for free on exit from outermost libxl callframe. */
+_hidden void libxl__ptr_add(libxl__gc *gc_opt, void *ptr /* may be NULL */);
/* if this is the outermost libxl callframe then free all pointers in @gc */
_hidden void libxl__free_all(libxl__gc *gc);
/* allocate and zero @bytes. (similar to a gc'd malloc(3)+memzero()) */
@@ -2110,7 +2119,7 @@ _hidden const char
*libxl__device_model_savefile(libxl__gc *gc, uint32_t domid);
#define GC_INIT(ctx) libxl__gc gc[1]; LIBXL_INIT_GC(gc[0],ctx)
#define GC_FREE libxl__free_all(gc)
#define CTX libxl__gc_owner(gc)
-#define NOGC NULL
+#define NOGC (&CTX->nogc_gc) /* pass only to consenting functions */
/* Allocation macros all of which use the gc. */
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 67ef82c..08c7dac 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -58,8 +58,7 @@ char *libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid)
char *libxl__domid_to_name(libxl__gc *gc, uint32_t domid)
{
char *s = libxl_domid_to_name(libxl__gc_owner(gc), domid);
- if ( s )
- libxl__ptr_add(gc, s);
+ libxl__ptr_add(gc, s);
return s;
}
@@ -107,8 +106,7 @@ char *libxl_cpupoolid_to_name(libxl_ctx *ctx, uint32_t
poolid)
char *libxl__cpupoolid_to_name(libxl__gc *gc, uint32_t poolid)
{
char *s = libxl_cpupoolid_to_name(libxl__gc_owner(gc), poolid);
- if ( s )
- libxl__ptr_add(gc, s);
+ libxl__ptr_add(gc, s);
return s;
}
diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
index 993f527..7fdf164 100644
--- a/tools/libxl/libxl_xshelp.c
+++ b/tools/libxl/libxl_xshelp.c
@@ -86,11 +86,8 @@ char * libxl__xs_read(libxl__gc *gc, xs_transaction_t t,
const char *path)
char *ptr;
ptr = xs_read(ctx->xsh, t, path, NULL);
- if (ptr != NULL) {
- libxl__ptr_add(gc, ptr);
- return ptr;
- }
- return 0;
+ libxl__ptr_add(gc, ptr);
+ return ptr;
}
char *libxl__xs_get_dompath(libxl__gc *gc, uint32_t domid)
--
1.7.2.5
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |