[Libguestfs] [nbdkit PATCH v2 2/8] cleanup: Add ACQUIRE_LOCK_FOR_CURRENT_SCOPE()
Add a new macro that lets us easily code a mutex cleanup no
matter how we leave a scope, which relies on the gcc/clang
extension of __attribute__((cleanup)). Note that there is
no semi-safe fallback possible to unlock a mutex once we rely
on cleanup semantics: eliding the attribute would deadlock,
and not defining the macro at all would fail to compile for
those old compilers. Our configure check was already warning
users about non-modern compilers (and no one was complaining),
so strengthen the probe into a full-time requirement that
refuses to configure without a decent compiler.
Another argument in favor of making cleanup support mandatory:
it is trivial to write an ill-behaved client that does a tight
loop in opening a connection, requesting an NBD_CMD_WRITE with
a 64M payload, then hanging up without actually sending the
data. But since connections.c uses CLEANUP_FREE for the 64M
buffer under client control, and our (previous) behavior for a
deficient compiler was to simply elide the cleanup call and
thus leak memory, the bad client can mount a DoS attack against
real clients by forcing nbdkit compiled without cleanup support
to run out of memory very quickly (nbdkit compiled with cleanup
support is immune).
Based on an idea in libguestfs, courtesy of Richard Jones.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
configure.ac | 7 +++----
src/cleanup.c | 8 +++++++-
src/internal.h | 9 +++++----
3 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index 86ce5f2..130ab78 100644
--- a/configure.ac
+++ b/configure.ac
@@ -113,17 +113,16 @@ main (int argc, char *argv[])
]])
],[
AC_MSG_RESULT([yes])
- AC_DEFINE([HAVE_ATTRIBUTE_CLEANUP],[1],[Define to 1 if
'__attribute__((cleanup(...)))' works with this compiler.])
],[
- AC_MSG_WARN(
+ AC_MSG_RESULT([no])
+ AC_MSG_ERROR(
['__attribute__((cleanup(...)))' does not work.
You may not be using a sufficiently recent version of GCC or CLANG, or
you may be using a C compiler which does not support this attribute,
or the configure test may be wrong.
-The code will still compile, but is likely to leak memory and other
-resources when it runs.])])
+This code requires the attribute to work for proper locking between threads.])])
dnl restore CFLAGS
CFLAGS="${acx_nbdkit_save_CFLAGS}"
diff --git a/src/cleanup.c b/src/cleanup.c
index fccf90c..3f7f8af 100644
--- a/src/cleanup.c
+++ b/src/cleanup.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2013 Red Hat Inc.
+ * Copyright (C) 2013-2017 Red Hat Inc.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -46,3 +46,9 @@ cleanup_free (void *ptr)
{
free (* (void **) ptr);
}
+
+void
+cleanup_unlock (pthread_mutex_t **ptr)
+{
+ pthread_mutex_unlock (*ptr);
+}
diff --git a/src/internal.h b/src/internal.h
index 5953185..a0bbef7 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -108,11 +108,12 @@ extern int quit_fd;
/* cleanup.c */
extern void cleanup_free (void *ptr);
-#ifdef HAVE_ATTRIBUTE_CLEANUP
#define CLEANUP_FREE __attribute__((cleanup (cleanup_free)))
-#else
-#define CLEANUP_FREE
-#endif
+extern void cleanup_unlock (pthread_mutex_t **ptr);
+#define CLEANUP_UNLOCK __attribute__((cleanup (cleanup_unlock)))
+#define ACQUIRE_LOCK_FOR_CURRENT_SCOPE(mutex) \
+ CLEANUP_UNLOCK pthread_mutex_t *_lock = mutex; \
+ pthread_mutex_lock (_lock)
/* connections.c */
struct connection;
--
2.13.6