Pino Toscano
2016-Feb-09 13:21 UTC
[Libguestfs] [PATCH 1/2] tmpdirs: centralize permissions handling
Move to lazy_make_tmpdir the logic for making world-readable (but only for root) newly-created temporary directories, removing the non-fatal code doing that in guestfs_impl_launch. Followup of commit 772f649e595d202bdb67f05aeb62157c1104be89. --- src/launch.c | 7 ------- src/tmpdirs.c | 30 ++++++++++++------------------ 2 files changed, 12 insertions(+), 25 deletions(-) diff --git a/src/launch.c b/src/launch.c index 9273c58..958d4b3 100644 --- a/src/launch.c +++ b/src/launch.c @@ -60,13 +60,6 @@ guestfs_impl_launch (guestfs_h *g) if (guestfs_int_lazy_make_tmpdir (g) == -1) return -1; - /* Allow anyone to read the temporary directory. The socket in this - * directory won't be readable but anyone can see it exists if they - * want. (RHBZ#610880). - */ - if (chmod (g->tmpdir, 0755) == -1) - warning (g, "chmod: %s: %m (ignored)", g->tmpdir); - /* Some common debugging information. */ if (g->verbose) { CLEANUP_FREE_VERSION struct guestfs_version *v diff --git a/src/tmpdirs.c b/src/tmpdirs.c index 76bf1c5..0a36f2f 100644 --- a/src/tmpdirs.c +++ b/src/tmpdirs.c @@ -160,6 +160,17 @@ lazy_make_tmpdir (guestfs_h *g, char *(*getdir) (guestfs_h *g), char **dest) free (tmpdir); return -1; } + /* Allow qemu (which may be running as qemu.qemu) to read in this + * temporary directory; we are storing either sockets, or temporary + * disks which qemu needs to access to. (RHBZ#610880). + * We do this only for root, as for normal users qemu will be run + * under the same user. + */ + if (geteuid () == 0 && chmod (tmppath, 0755) == -1) { + perrorf (g, "chmod: %s", tmppath); + free (tmppath); + return -1; + } *dest = tmppath; } return 0; @@ -178,24 +189,7 @@ guestfs_int_lazy_make_tmpdir (guestfs_h *g) int guestfs_int_lazy_make_sockdir (guestfs_h *g) { - int ret; - uid_t euid = geteuid (); - - ret = lazy_make_tmpdir (g, guestfs_get_sockdir, &g->sockdir); - if (ret == -1) - return ret; - - if (euid == 0) { - /* Allow qemu (which may be running as qemu.qemu) to read the socket - * temporary directory. (RHBZ#610880). - */ - if (chmod (g->sockdir, 0755) == -1) { - perrorf (g, "chmod: %s", g->sockdir); - return -1; - } - } - - return ret; + return lazy_make_tmpdir (g, guestfs_get_sockdir, &g->sockdir); } /* Recursively remove a temporary directory. If removal fails, just -- 2.5.0
Pino Toscano
2016-Feb-09 13:21 UTC
[Libguestfs] [PATCH 2/2] tmpdirs: fix typo in variable name
On mkdtemp error, free tmppath and not tmpdir (which is CLEANUP_FREE). Fixes commit 673a7a959c15e9a389a13620f3a10cb12a9537d0. --- src/tmpdirs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tmpdirs.c b/src/tmpdirs.c index 0a36f2f..afa3dd4 100644 --- a/src/tmpdirs.c +++ b/src/tmpdirs.c @@ -157,7 +157,7 @@ lazy_make_tmpdir (guestfs_h *g, char *(*getdir) (guestfs_h *g), char **dest) char *tmppath = safe_asprintf (g, "%s/libguestfsXXXXXX", tmpdir); if (mkdtemp (tmppath) == NULL) { perrorf (g, _("%s: cannot create temporary directory"), tmppath); - free (tmpdir); + free (tmppath); return -1; } /* Allow qemu (which may be running as qemu.qemu) to read in this -- 2.5.0
Richard W.M. Jones
2016-Feb-09 13:26 UTC
Re: [Libguestfs] [PATCH 1/2] tmpdirs: centralize permissions handling
ACK series. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
Possibly Parallel Threads
- [PATCH] tmpdirs: Make the ‘su broken’ error message actionable.
- [PATCH] tmpdirs: Blame systemd because su is broken.
- [PATCH 2/4] src/tmpdirs.c: Add internal documentation.
- [PATCH 2/3] lib: extract lazy tmpdir creation helper
- [PATCH 1/3] launch: add internal helper for socket paths creation