|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v3] libxl: enforce prohibitions of internal callers
libxl_internal.h says:
* Functions using LIBXL__INIT_EGC may *not* generally be called from
* within libxl, because libxl__egc_cleanup may call back into the
* application. ...
and
* ... [Functions which take an ao_how] MAY NOT
* be called from inside libxl, because they can cause reentrancy
* callbacks.
However, this was not enforced. Particularly the latter restriction
is easy to overlook, especially since during the transition period to
the new event system we have bent this rule a couple of times, and the
bad pattern simply involves passing 0 or NULL for the ao_how.
So use the compiler to enforce this property, as follows:
- Mark all functions which take a libxl_asyncop_how, or which
use EGC_INIT or LIBXL__INIT_EGC, with a new annotation
LIBXL_EXTERNAL_CALLERS_ONLY in the public header.
- Change the documentation comment for asynch operations and egcs to
say that this should always be done.
- Arrange that if libxl.h is included via libxl_internal.h,
LIBXL_EXTERNAL_CALLERS_ONLY expands to __attribute__((warning(...))),
which generates a message like this:
libxl.c:1772: warning: call to 'libxl_device_disk_remove'
declared with attribute warning:
may not be called from within libxl
Otherwise, the annotation expands to nothing, so external
callers are unaffected.
- Forbid inclusion of both libxl.h and libxl_internal.h unless
libxl_internal.h came first, so that the above check doesn't have
any loopholes. Files which include libxl_internal.h should not
include libxl.h as well.
This is enforced explicitly using #error. However, in practice
with the current tree it just changes the error message when this
mistake is made; otherwise we would carry on to immediately
following #define which would cause the compiler to complain that
LIBXL_EXTERNAL_CALLERS_ONLY was redefined. Then the developer
might be tempted to add a #ifndef which would be wrong - it would
leave the affected translation unit unprotected by the new
enforcement regime. So let's be explicit.
- Fix the one source of files which violate the above principle, the
output from the idl compiler, by removing the redundant inclusion
of libxl.h from the output.
Also introduce a new script "check-libxl-api-rules" which contains
some ad-hoc regexps to spot and complain when libxl.h contains
functions which mention libxl_asyncop_how but not
LIBXL_EXTERNAL_CALLERS_ONLY. This isn't a full C parser but is likely
to get the common cases right and err on the side of complaining.
While we are here, the invocation of perl for the bsd queue.h seddery
to $(PERL).
Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
-
Changes in v3:
* Say $(PERL) rather than perl.
* Say $(PERL) in the _libxl_list.h rule too.
* Better commit message, mentions check-libxl-api-rules.
Changes in v2:
* Rebased to current xen-unstable tip.
* Added LIBXL_EXTERNAL_CALLERS_ONLY to more functions.
* -Wno-error is no longer needed as the tree now has no errors of this form.
* check-libxl-api-rules parses #... directives in the preprocessor output
so that it can report errors at their location in libxl.h.
* Indentation in check-libxl-api-rules fixed.
---
.gitignore | 1 +
.hgignore | 1 +
tools/libxl/Makefile | 16 +++++++--
tools/libxl/check-libxl-api-rules | 23 +++++++++++++
tools/libxl/gentypes.py | 1 -
tools/libxl/libxl.h | 64 ++++++++++++++++++++++++++-----------
tools/libxl/libxl_event.h | 21 ++++++++----
tools/libxl/libxl_internal.h | 14 ++++++--
8 files changed, 107 insertions(+), 34 deletions(-)
diff --git a/.gitignore b/.gitignore
index 0891d90..41c89c0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -187,6 +187,7 @@ tools/libxl/xl
tools/libxl/testenum
tools/libxl/testenum.c
tools/libxl/tmp.*
+tools/libxl/libxl.api-for-check
tools/libaio/src/*.ol
tools/libaio/src/*.os
tools/misc/cpuperf/cpuperf-perfcntr
diff --git a/.hgignore b/.hgignore
index 9baf57b..8ff83c7 100644
--- a/.hgignore
+++ b/.hgignore
@@ -185,6 +185,7 @@
^tools/libxl/testidl\.c$
^tools/libxl/tmp\..*$
^tools/libxl/.*\.new$
+^tools/libxl/libxl\.api-for-check
^tools/libvchan/vchan-node[12]$
^tools/libaio/src/.*\.ol$
^tools/libaio/src/.*\.os$
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 424a7ee..63a8157 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -74,7 +74,8 @@ LIBXL_OBJS += _libxl_types.o libxl_flask.o
_libxl_types_internal.o
$(LIBXL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest)
$(CFLAGS_libxenstore) $(CFLAGS_libblktapctl) -include $(XEN_ROOT)/tools/config.h
AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h _libxl_list.h _paths.h \
- _libxl_save_msgs_callout.h _libxl_save_msgs_helper.h
+ _libxl_save_msgs_callout.h _libxl_save_msgs_helper.h \
+ libxl.api-ok
AUTOSRCS= libxlu_cfg_y.c libxlu_cfg_l.c
AUTOSRCS += _libxl_save_msgs_callout.c _libxl_save_msgs_helper.c
LIBXLU_OBJS = libxlu_cfg_y.o libxlu_cfg_l.o libxlu_cfg.o \
@@ -113,13 +114,22 @@ $(LIBXL_OBJS) $(LIBXLU_OBJS) $(XL_OBJS)
$(SAVE_HELPER_OBJS): $(AUTOINCS)
genpath-target = $(call buildmakevars2file,_paths.h.tmp)
$(eval $(genpath-target))
+libxl.api-ok: check-libxl-api-rules libxl.api-for-check
+ $(PERL) $^
+
+%.api-for-check: %.h
+ $(CC) $(CPPFLAGS) $(CFLAGS) $(CFLAGS_$*.o) -c -E $< $(APPEND_CFLAGS) \
+ -DLIBXL_EXTERNAL_CALLERS_ONLY=LIBXL_EXTERNAL_CALLERS_ONLY \
+ >$@.new
+ $(call move-if-changed,$@.new,$@)
+
_paths.h: genpath
sed -e "s/\([^=]*\)=\(.*\)/#define \1 \2/g" $@.tmp >$@.2.tmp
rm -f $@.tmp
$(call move-if-changed,$@.2.tmp,$@)
_libxl_list.h: $(XEN_INCLUDE)/xen-external/bsd-sys-queue-h-seddery
$(XEN_INCLUDE)/xen-external/bsd-sys-queue.h
- perl $^ --prefix=libxl >$@.new
+ $(PERL) $^ --prefix=libxl >$@.new
$(call move-if-changed,$@.new,$@)
_libxl_save_msgs_helper.c _libxl_save_msgs_callout.c \
@@ -200,7 +210,7 @@ install: all
.PHONY: clean
clean:
$(RM) -f _*.h *.o *.so* *.a $(CLIENTS) $(DEPS)
- $(RM) -f _*.c *.pyc _paths.*.tmp
+ $(RM) -f _*.c *.pyc _paths.*.tmp *.api-for-check
$(RM) -f testidl.c.new testidl.c
# $(RM) -f $(AUTOSRCS) $(AUTOINCS)
diff --git a/tools/libxl/check-libxl-api-rules
b/tools/libxl/check-libxl-api-rules
new file mode 100755
index 0000000..18ff39c
--- /dev/null
+++ b/tools/libxl/check-libxl-api-rules
@@ -0,0 +1,23 @@
+#!/usr/bin/perl -w
+use strict;
+our $needed=0;
+our $speclineoffset=0;
+our $specfile;
+while (<>) {
+ if (m/^\# (\d+) \"(.*)\"$/) {
+ $speclineoffset = $1 - $. -1;
+ $specfile = $2;
+ }
+ my $file = defined($specfile) ? $specfile : $ARGV;
+ my $line = $speclineoffset + $.;
+ if (m/libxl_asyncop_how[^;]/) {
+ $needed=1;
+ }
+ if (m/LIBXL_EXTERNAL_CALLERS_ONLY/) {
+ $needed=0;
+ }
+ next unless $needed;
+ if (m/\;/) {
+ die "$file:$line:missing LIBXL_EXTERNAL_CALLERS_ONLY";
+ }
+}
diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index eb67524..1d13201 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -377,7 +377,6 @@ if __name__ == '__main__':
#include <stdlib.h>
#include <string.h>
-#include "libxl.h"
#include "libxl_internal.h"
#define LIBXL_DTOR_POISON 0xa5
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index e1a7296..5ec2d74 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -266,6 +266,13 @@
#endif
#endif
+/* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be
+ * called from within libxl itself. Callers outside libxl, who
+ * do not #include libxl_internal.h, are fine. */
+#ifndef LIBXL_EXTERNAL_CALLERS_ONLY
+#define LIBXL_EXTERNAL_CALLERS_ONLY /* disappears for callers outside libxl */
+#endif
+
typedef uint8_t libxl_mac[6];
#define LIBXL_MAC_FMT "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx"
#define LIBXL_MAC_FMTLEN ((2*6)+5) /* 6 hex bytes plus 5 colons */
@@ -496,11 +503,13 @@ int libxl_ctx_free(libxl_ctx *ctx /* 0 is OK */);
int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config,
uint32_t *domid,
const libxl_asyncop_how *ao_how,
- const libxl_asyncprogress_how *aop_console_how);
+ const libxl_asyncprogress_how *aop_console_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config,
uint32_t *domid, int restore_fd,
const libxl_asyncop_how *ao_how,
- const libxl_asyncprogress_how
*aop_console_how);
+ const libxl_asyncprogress_how *aop_console_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
/* A progress report will be made via ao_console_how, of type
* domain_create_console_available, when the domain's primary
* console is available and can be connected to.
@@ -511,7 +520,8 @@ void libxl_domain_config_dispose(libxl_domain_config
*d_config);
int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, int fd,
int flags, /* LIBXL_SUSPEND_* */
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
#define LIBXL_SUSPEND_DEBUG 1
#define LIBXL_SUSPEND_LIVE 2
@@ -523,12 +533,14 @@ int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid,
int suspend_cancel);
int libxl_domain_remus_start(libxl_ctx *ctx, libxl_domain_remus_info *info,
uint32_t domid, int send_fd, int recv_fd,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_domain_shutdown(libxl_ctx *ctx, uint32_t domid);
int libxl_domain_reboot(libxl_ctx *ctx, uint32_t domid);
int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid,
libxl_domain_create_info *info, const char *name_suffix, libxl_uuid new_uuid);
/* get max. number of cpus supported by hypervisor */
@@ -549,7 +561,8 @@ int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid);
int libxl_domain_core_dump(libxl_ctx *ctx, uint32_t domid,
const char *filename,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t
target_memkb);
int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, int32_t
target_memkb, int relative, int enforce);
@@ -677,13 +690,16 @@ void libxl_vcpuinfo_list_free(libxl_vcpuinfo *, int
nr_vcpus);
/* Disks */
int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid,
libxl_device_disk *disk,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
libxl_device_disk *disk,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_device_disk_destroy(libxl_ctx *ctx, uint32_t domid,
libxl_device_disk *disk,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int
*num);
int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
@@ -694,17 +710,21 @@ int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t
domid,
* be attached to the guest.
*/
int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
/* Network Interfaces */
int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid,
libxl_device_nic *nic,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_device_nic_destroy(libxl_ctx *ctx, uint32_t domid,
libxl_device_nic *nic,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx, uint32_t domid, int
*num);
int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
@@ -712,23 +732,29 @@ int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t
domid,
/* Keyboard */
int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t domid,
libxl_device_vkb *vkb,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_device_vkb_destroy(libxl_ctx *ctx, uint32_t domid,
libxl_device_vkb *vkb,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
/* Framebuffer */
int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t domid,
libxl_device_vfb *vfb,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_device_vfb_destroy(libxl_ctx *ctx, uint32_t domid,
libxl_device_vfb *vfb,
- const libxl_asyncop_how *ao_how);
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
/* PCI Passthrough */
int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid, libxl_device_pci
*pcidev);
diff --git a/tools/libxl/libxl_event.h b/tools/libxl/libxl_event.h
index 713d96d..3344bc8 100644
--- a/tools/libxl/libxl_event.h
+++ b/tools/libxl/libxl_event.h
@@ -37,7 +37,8 @@ typedef int libxl_event_predicate(const libxl_event*, void
*user);
int libxl_event_check(libxl_ctx *ctx, libxl_event **event_r,
uint64_t typemask,
- libxl_event_predicate *predicate, void *predicate_user);
+ libxl_event_predicate *predicate, void *predicate_user)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
/* Searches for an event, already-happened, which matches typemask
* and predicate. predicate==0 matches any event.
* libxl_event_check returns the event, which must then later be
@@ -48,7 +49,8 @@ int libxl_event_check(libxl_ctx *ctx, libxl_event **event_r,
int libxl_event_wait(libxl_ctx *ctx, libxl_event **event_r,
uint64_t typemask,
- libxl_event_predicate *predicate, void *predicate_user);
+ libxl_event_predicate *predicate, void *predicate_user)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
/* Like libxl_event_check but blocks if no suitable events are
* available, until some are. Uses libxl_osevent_beforepoll/
* _afterpoll so may be inefficient if very many domains are being
@@ -256,7 +258,8 @@ struct pollfd;
*/
int libxl_osevent_beforepoll(libxl_ctx *ctx, int *nfds_io,
struct pollfd *fds, int *timeout_upd,
- struct timeval now);
+ struct timeval now)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
/* nfds and fds[0..nfds] must be from the most recent call to
* _beforepoll, as modified by poll. (It is therefore not possible
@@ -271,7 +274,8 @@ int libxl_osevent_beforepoll(libxl_ctx *ctx, int *nfds_io,
* libxl_event_check.
*/
void libxl_osevent_afterpoll(libxl_ctx *ctx, int nfds, const struct pollfd
*fds,
- struct timeval now);
+ struct timeval now)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
typedef struct libxl_osevent_hooks {
@@ -357,14 +361,16 @@ void libxl_osevent_register_hooks(libxl_ctx *ctx,
*/
void libxl_osevent_occurred_fd(libxl_ctx *ctx, void *for_libxl,
- int fd, short events, short revents);
+ int fd, short events, short revents)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
/* Implicitly, on entry to this function the timeout has been
* deregistered. If _occurred_timeout is called, libxl will not
* call timeout_deregister; if it wants to requeue the timeout it
* will call timeout_register again.
*/
-void libxl_osevent_occurred_timeout(libxl_ctx *ctx, void *for_libxl);
+void libxl_osevent_occurred_timeout(libxl_ctx *ctx, void *for_libxl)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
/*======================================================================*/
@@ -506,7 +512,8 @@ void libxl_childproc_setmode(libxl_ctx *ctx, const
libxl_childproc_hooks *hooks,
* certainly need to use the self-pipe trick (or a working pselect or
* ppoll) to implement this.
*/
-int libxl_childproc_reaped(libxl_ctx *ctx, pid_t, int status);
+int libxl_childproc_reaped(libxl_ctx *ctx, pid_t, int status)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
/*
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 691b4f6..58004b3 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -52,6 +52,12 @@
#include <xen/io/xenbus.h>
+#ifdef LIBXL_H
+# error libxl.h should be included via libxl_internal.h, not separately
+#endif
+#define LIBXL_EXTERNAL_CALLERS_ONLY \
+ __attribute__((warning("may not be called from within libxl")))
+
#include "libxl.h"
#include "_paths.h"
#include "_libxl_save_msgs_callout.h"
@@ -1534,10 +1540,10 @@ int libxl__hotplug_settings(libxl__gc *gc,
xs_transaction_t t);
*
* Functions using LIBXL__INIT_EGC may *not* generally be called from
* within libxl, because libxl__egc_cleanup may call back into the
- * application. This should be documented near the function
- * prototype(s) for callers of LIBXL__INIT_EGC and EGC_INIT. You
- * should in any case not find it necessary to call egc-creators from
- * within libxl.
+ * application. This should be enforced by declaring all such
+ * functions in libxl.h or libxl_event.h with
+ * LIBXL_EXTERNAL_CALLERS_ONLY. You should in any case not find it
+ * necessary to call egc-creators from within libxl.
*
* The callbacks must all take place with the ctx unlocked because
* the application is entitled to reenter libxl from them. This
--
tg: (a7f0910..) t/xen/xl.external-callers-only (depends on:
t/xen/xl.bootloader.fix.no-blunder-on)
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |