Unlink the daemon socket in the shutdown callback, instead of right before creating a new one. This makes sure we are unlinking the right socket. --- src/launch-direct.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/launch-direct.c b/src/launch-direct.c index 1c805e3..b8e453d 100644 --- a/src/launch-direct.c +++ b/src/launch-direct.c @@ -296,7 +296,6 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) * for qemu to connect to. */ snprintf (data->guestfsd_sock, sizeof data->guestfsd_sock, "%s/guestfsd.sock", g->tmpdir); - unlink (data->guestfsd_sock); daemon_accept_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0); if (daemon_accept_sock == -1) { @@ -1510,6 +1509,11 @@ shutdown_direct (guestfs_h *g, void *datav, int check_for_errors) data->pid = data->recoverypid = 0; + if (data->guestfsd_sock[0] != '\0') { + unlink (data->guestfsd_sock); + data->guestfsd_sock[0] = '\0'; + } + free (data->qemu_help); data->qemu_help = NULL; free (data->qemu_version); -- 2.5.0

Unlink the sockets in the shutdown callback, instead of right before creating a new ones. This makes sure we are unlinking the right sockets. --- src/launch-libvirt.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index 185b44b..8a5d93e 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -397,7 +397,6 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) */ snprintf (data->guestfsd_path, sizeof data->guestfsd_path, "%s/guestfsd.sock", g->tmpdir); - unlink (data->guestfsd_path); set_socket_create_context (g); @@ -424,7 +423,6 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) /* For the serial console. */ snprintf (data->console_path, sizeof data->console_path, "%s/console.sock", g->tmpdir); - unlink (data->console_path); console_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0); if (console_sock == -1) { @@ -2027,6 +2025,16 @@ shutdown_libvirt (guestfs_h *g, void *datav, int check_for_errors) if (conn != NULL) virConnectClose (conn); + if (data->guestfsd_path[0] != '\0') { + unlink (data->guestfsd_path); + data->guestfsd_path[0] = '\0'; + } + + if (data->console_path[0] != '\0') { + unlink (data->console_path); + data->console_path[0] = '\0'; + } + data->conn = NULL; data->dom = NULL; -- 2.5.0

The API for UNIX sockets limits the path to a static size (usually 104 or 108 characters, NULL included), which is internally represented as UNIX_PATH_MAX. If the temporary directory set is long enough (e.g. when running tools as uninstalled, using "run") then these socket paths get trucated, if not even cause failures when binding the sockets. Introduce a new internal API to create paths for sockets, and a new helper path where store them in the above case. This new path is created only when needed, on a temporary directory under /tmp (hardcoded), and cleaned when the handle is destroyed. --- src/guestfs-internal.h | 8 ++++++++ src/handle.c | 4 +++- src/launch-direct.c | 4 +++- src/launch-libvirt.c | 10 ++++++---- src/launch.c | 30 ++++++++++++++++++++++++++++++ src/tmpdirs.c | 22 ++++++++++++++++++++++ 6 files changed, 72 insertions(+), 6 deletions(-) diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 5ecd322..5575aff 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -438,6 +438,11 @@ struct guestfs_h char *env_tmpdir; /* $TMPDIR (NULL if not set) */ char *int_tmpdir; /* $LIBGUESTFS_TMPDIR or guestfs_set_tmpdir or NULL */ char *int_cachedir; /* $LIBGUESTFS_CACHEDIR or guestfs_set_cachedir or NULL */ + /* Optional socket directory - this is created on demand only when + * creating a socket whose absolute path in tmpdir would not fit + * into sockaddr_un::sun_path. + */ + char *sockdir; /* Error handler, plus stack of old error handlers. */ guestfs_error_handler_cb error_cb; @@ -758,7 +763,9 @@ extern void guestfs_int_call_callbacks_array (guestfs_h *g, uint64_t event, cons /* tmpdirs.c */ extern int guestfs_int_set_env_tmpdir (guestfs_h *g, const char *tmpdir); extern int guestfs_int_lazy_make_tmpdir (guestfs_h *g); +extern int guestfs_int_lazy_make_sockdir (guestfs_h *g); extern void guestfs_int_remove_tmpdir (guestfs_h *g); +extern void guestfs_int_remove_sockdir (guestfs_h *g); extern void guestfs_int_recursive_remove_dir (guestfs_h *g, const char *dir); /* whole-file.c */ @@ -782,6 +789,7 @@ extern void guestfs_int_launch_send_progress (guestfs_h *g, int perdozen); extern char *guestfs_int_appliance_command_line (guestfs_h *g, const char *appliance_dev, int flags); #define APPLIANCE_COMMAND_LINE_IS_TCG 1 const char *guestfs_int_get_cpu_model (int kvm); +int guestfs_int_create_socketname (guestfs_h *g, const char *filename, char (*sockname)[UNIX_PATH_MAX]); extern void guestfs_int_register_backend (const char *name, const struct backend_ops *); extern int guestfs_int_set_backend (guestfs_h *g, const char *method); diff --git a/src/handle.c b/src/handle.c index 947818c..076e3fd 100644 --- a/src/handle.c +++ b/src/handle.c @@ -347,8 +347,9 @@ guestfs_close (guestfs_h *g) if (g->test_fp != NULL) fclose (g->test_fp); - /* Remove temporary directory. */ + /* Remove temporary directories. */ guestfs_int_remove_tmpdir (g); + guestfs_int_remove_sockdir (g); /* Mark the handle as dead and then free up all memory. */ g->state = NO_HANDLE; @@ -380,6 +381,7 @@ guestfs_close (guestfs_h *g) free (g->env_tmpdir); free (g->int_tmpdir); free (g->int_cachedir); + free (g->sockdir); free (g->last_error); free (g->identifier); free (g->program); diff --git a/src/launch-direct.c b/src/launch-direct.c index b8e453d..a81d4b3 100644 --- a/src/launch-direct.c +++ b/src/launch-direct.c @@ -295,7 +295,9 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) /* Using virtio-serial, we need to create a local Unix domain socket * for qemu to connect to. */ - snprintf (data->guestfsd_sock, sizeof data->guestfsd_sock, "%s/guestfsd.sock", g->tmpdir); + if (guestfs_int_create_socketname (g, "guestfsd.sock", + &data->guestfsd_sock) == -1) + goto cleanup0; daemon_accept_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0); if (daemon_accept_sock == -1) { diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index 8a5d93e..376bd80 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -395,8 +395,9 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) /* Using virtio-serial, we need to create a local Unix domain socket * for qemu to connect to. */ - snprintf (data->guestfsd_path, sizeof data->guestfsd_path, - "%s/guestfsd.sock", g->tmpdir); + if (guestfs_int_create_socketname (g, "guestfsd.sock", + &data->guestfsd_path) == -1) + goto cleanup; set_socket_create_context (g); @@ -421,8 +422,9 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) } /* For the serial console. */ - snprintf (data->console_path, sizeof data->console_path, - "%s/console.sock", g->tmpdir); + if (guestfs_int_create_socketname (g, "console.sock", + &data->console_path) == -1) + goto cleanup; console_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0); if (console_sock == -1) { diff --git a/src/launch.c b/src/launch.c index f59818f..1f1f93d 100644 --- a/src/launch.c +++ b/src/launch.c @@ -418,6 +418,36 @@ guestfs_int_get_cpu_model (int kvm) #endif } +/* Create the path for a socket with the selected filename in the tmpdir, + * falling back to creating a separate sockdir and using that as base. + */ +int +guestfs_int_create_socketname (guestfs_h *g, const char *filename, + char (*sockpath)[UNIX_PATH_MAX]) +{ + char *path = g->tmpdir; + + /* Check whether the new socket can fit into tmpdir. */ + if (strlen (path) + 1 + strlen (filename) > UNIX_PATH_MAX-1) { + if (guestfs_int_lazy_make_sockdir (g) == -1) + return -1; + + path = g->sockdir; + + /* Safety check for the new path in sockdir. */ + if (strlen (path) + 1 + strlen (filename) > UNIX_PATH_MAX-1) { + error (g, _("cannot get a path for the socket '%s'"), + filename); + return -1; + } + } + + snprintf (*sockpath, UNIX_PATH_MAX-1, "%s/%s", path, filename); + (*sockpath)[UNIX_PATH_MAX-1] = '\0'; + + return 0; +} + /* glibc documents, but does not actually implement, a 'getumask(3)' * call. This implements a thread-safe way to get the umask. Note * this is only called when g->verbose is true and after g->tmpdir diff --git a/src/tmpdirs.c b/src/tmpdirs.c index 9154d8b..e406e53 100644 --- a/src/tmpdirs.c +++ b/src/tmpdirs.c @@ -139,6 +139,21 @@ guestfs_int_lazy_make_tmpdir (guestfs_h *g) return 0; } +int +guestfs_int_lazy_make_sockdir (guestfs_h *g) +{ + if (!g->sockdir) { + g->sockdir = safe_strdup (g, "/tmp/guestfs-sockXXXXXX"); + if (mkdtemp (g->sockdir) == NULL) { + perrorf (g, _("%s: cannot create temporary directory"), g->sockdir); + free (g->sockdir); + g->sockdir = NULL; + return -1; + } + } + return 0; +} + /* Recursively remove a temporary directory. If removal fails, just * return (it's a temporary directory so it'll eventually be cleaned * up by a temp cleaner). This is done using "rm -rf" because that's @@ -161,3 +176,10 @@ guestfs_int_remove_tmpdir (guestfs_h *g) if (g->tmpdir) guestfs_int_recursive_remove_dir (g, g->tmpdir); } + +void +guestfs_int_remove_sockdir (guestfs_h *g) +{ + if (g->sockdir) + guestfs_int_recursive_remove_dir (g, g->sockdir); +} -- 2.5.0