8:33 a.m.
Resolve the given path within the chroot, so scrub can be invoked
outside the chroot on an already-resolved path.
Given that realpath is used, its availability is checked manually,
since scrub-file already depends on the "scrub" feature. Slightly ugly,
but on the other hand realpath is generally available nowadays, so the
check should not be failing.
Add few tests in scrub-file for this and other similar issues.
---
daemon/scrub.c | 24 +++++++++++++++++++++++-
generator/actions.ml | 13 ++++++++++++-
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/daemon/scrub.c b/daemon/scrub.c
index cd880b9..f500a08 100644
--- a/daemon/scrub.c
+++ b/daemon/scrub.c
@@ -54,12 +54,34 @@ do_scrub_device (const char *device)
int
do_scrub_file (const char *file)
{
+ CLEANUP_FREE char *rp = NULL;
CLEANUP_FREE char *buf = NULL;
CLEANUP_FREE char *err = NULL;
int r;
+ if (! optgroup_realpath_available ()) {
+ reply_with_error_errno (ENOTSUP,
+ "feature '%s' is not available in this\n"
+ "build of libguestfs. Read 'AVAILABILITY' in the guestfs(3) man page
for\n"
+ "how to check for the availability of features.",
+ "realpath");
+ return -1;
+ }
+
+ /* Resolve the path to the file. If it fails, then the file
+ * most probably does not exist or "file" is a symlink pointing
+ * outside the chroot.
+ */
+ CHROOT_IN;
+ rp = realpath (file, NULL);
+ CHROOT_OUT;
+ if (rp == NULL) {
+ reply_with_perror ("realpath: %s", file);
+ return -1;
+ }
+
/* Make the path relative to /sysroot. */
- buf = sysroot_path (file);
+ buf = sysroot_path (rp);
if (!buf) {
reply_with_perror ("malloc");
return -1;
diff --git a/generator/actions.ml b/generator/actions.ml
index 0826137..01f6ab5 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -5719,7 +5719,18 @@ manual page for more details." };
tests = [
InitScratchFS, Always, TestRun (
[["write"; "/scrub_file"; "content"];
- ["scrub_file"; "/scrub_file"]]), []
+ ["scrub_file"; "/scrub_file"]]), [];
+ InitScratchFS, Always, TestRun (
+ [["write"; "/scrub_file_2"; "content"];
+ ["ln_s"; "/scrub_file_2"; "/scrub_file_2_link"];
+ ["scrub_file"; "/scrub_file_2_link"]]), [];
+ InitScratchFS, Always, TestLastFail (
+ [["ln_s"; "/scrub_file_3_notexisting";
"/scrub_file_3_link"];
+ ["scrub_file"; "/scrub_file_3_link"]]), [];
+ InitScratchFS, Always, TestLastFail (
+ [["write"; "/scrub_file_4"; "content"];
+ ["ln_s"; "../sysroot/scrub_file_4";
"/scrub_file_4_link"];
+ ["scrub_file"; "/scrub_file_4_link"]]), [];
];
shortdesc = "scrub (securely wipe) a file";
longdesc = "\
--
1.9.0
9:56 a.m.
On Tue, May 20, 2014 at 03:33:31PM +0200, Pino Toscano wrote:
Resolve the given path within the chroot, so scrub can be invoked
outside the chroot on an already-resolved path.
Given that realpath is used, its availability is checked manually,
since scrub-file already depends on the "scrub" feature. Slightly ugly,
but on the other hand realpath is generally available nowadays, so the
check should not be failing.
Add few tests in scrub-file for this and other similar issues.
The realpath test was added (commit a86eb0e0d2c67e2) at a time when we
thought it would be a good idea to have the daemon run on Windows.
Since no one thinks that's a good idea any longer, I think you could
prepend this commit with one which removes tests for realpath /
HAVE_REALPATH and hard-codes optgroup_realpath_available() == 1.
On to this patch ...
Is it possible to make this change more generic so in future it could
be applied to other functions?
- Would it make sense to have a 'sysroot_realpath' library function to
replace 'sysroot_path' in certain functions?
- Would it even make sense to modify 'sysroot_path' so it just calls
realpath?
(I suspect not the second one because there are likely to be calls
like 'lstat' that want to see the symlink, not the final path.)
The concept behind the patch is good, I'd just like to see if we can
apply this in more places in future.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
11:16 a.m.
On Tuesday 20 May 2014 15:56:16 Richard W.M. Jones wrote:
On Tue, May 20, 2014 at 03:33:31PM +0200, Pino Toscano wrote:
> Resolve the given path within the chroot, so scrub can be invoked
> outside the chroot on an already-resolved path.
> Given that realpath is used, its availability is checked manually,
> since scrub-file already depends on the "scrub" feature. Slightly
> ugly, but on the other hand realpath is generally available
> nowadays, so the check should not be failing.
>
> Add few tests in scrub-file for this and other similar issues.
The realpath test was added (commit a86eb0e0d2c67e2) at a time when we
thought it would be a good idea to have the daemon run on Windows.
Since no one thinks that's a good idea any longer, I think you could
prepend this commit with one which removes tests for realpath /
HAVE_REALPATH and hard-codes optgroup_realpath_available() == 1.
I didn't know the story behind this, and went for the conservative
route.
Interesting enough, realpath has been unconditionally used already, so I
will pull the plug to the conditional usage in some other parts, then.
Is it possible to make this change more generic so in future it
could
be applied to other functions?
- Would it make sense to have a 'sysroot_realpath' library function to
replace 'sysroot_path' in certain functions?
Good idea. Do you have also an idea which daemon functions, other than
scrub-files and realpath itself, might need such handling?
--
Pino Toscano
12:54 p.m.
New subject: [PATCH 1/4] generator: add always-available optgroups
Support the possibility to have optional groups always enabled (e.g.
because they were present in the past, and they need to be kept for
users).
Add and use few helper optgroups-related functions to deal also with
them.
---
generator/daemon.ml | 24 ++++++++++++++++++++----
generator/optgroups.ml | 26 ++++++++++++++++++++++++++
2 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/generator/daemon.ml b/generator/daemon.ml
index 548982b..65d910a 100644
--- a/generator/daemon.ml
+++ b/generator/daemon.ml
@@ -713,11 +713,27 @@ and generate_daemon_optgroups_c () =
pr "#include \"optgroups.h\"\n";
pr "\n";
+ if optgroups_available <> [] then (
+ pr "static int\n";
+ pr "dummy_available (void)\n";
+ pr "{\n";
+ pr " return 1;\n";
+ pr "}\n";
+ pr "\n";
+
+ List.iter (
+ fun group ->
+ pr "#define optgroup_%s_available dummy_available\n" group;
+ ) optgroups_available;
+
+ pr "\n";
+ );
+
pr "struct optgroup optgroups[] = {\n";
List.iter (
- fun (group, _) ->
+ fun group ->
pr " { \"%s\", optgroup_%s_available },\n" group group
- ) optgroups;
+ ) optgroups_names_all;
pr " { NULL, NULL }\n";
pr "};\n"
@@ -729,9 +745,9 @@ and generate_daemon_optgroups_h () =
pr "\n";
List.iter (
- fun (group, _) ->
+ fun group ->
pr "extern int optgroup_%s_available (void);\n" group
- ) optgroups;
+ ) optgroups_names_all;
pr "\n";
diff --git a/generator/optgroups.ml b/generator/optgroups.ml
index 2a348e7..a08f9c3 100644
--- a/generator/optgroups.ml
+++ b/generator/optgroups.ml
@@ -21,6 +21,18 @@
open Types
open Actions
+(* The list of optional groups which need to be in the daemon as always
+ * available.
+ *
+ * NOTE: if a optional group listed here has functions using it, then its
+ * presence here will be ignored (thus being handled as usual). This way,
+ * there is no need to remove groups from this list, once they have been
+ * added (as it means at some point they had no more functions using them,
+ * but libguestfs ought to provide them to the user.
+ *)
+let internal_optgroups_available = [
+]
+
(* Create list of optional groups. *)
let optgroups =
let h = Hashtbl.create 13 in
@@ -42,3 +54,17 @@ let optgroups =
group, fns
) groups in
List.sort (fun x y -> compare (fst x) (fst y)) groups
+
+let optgroups_names =
+ fst (List.split optgroups)
+
+let optgroups_available =
+ let groups =
+ List.filter (
+ fun group ->
+ List.mem group optgroups_names <> true
+ ) internal_optgroups_available in
+ List.sort compare groups
+
+let optgroups_names_all =
+ List.sort compare (optgroups_names @ optgroups_available)
--
1.9.0
3:40 p.m.
New subject: [PATCH 1/4] generator: add always-available optgroups
On Tue, May 20, 2014 at 07:54:45PM +0200, Pino Toscano wrote:
Support the possibility to have optional groups always enabled (e.g.
because they were present in the past, and they need to be kept for
users).
Add and use few helper optgroups-related functions to deal also with
them.
What do you think about the attached addition to this patch?
(It is meant to be squashed into your patch.)
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
3:47 a.m.
New subject: [PATCH 1/4] generator: add always-available optgroups
On Tuesday 20 May 2014 21:40:21 Richard W.M. Jones wrote:
On Tue, May 20, 2014 at 07:54:45PM +0200, Pino Toscano wrote:
> Support the possibility to have optional groups always enabled (e.g.
> because they were present in the past, and they need to be kept for
> users).
> Add and use few helper optgroups-related functions to deal also with
> them.
What do you think about the attached addition to this patch?
let optgroups_names_all =
- List.sort compare (optgroups_names @ optgroups_available)
+ List.sort compare (optgroups_names @ optgroups_retired)
Wouldn't this give duplicate values if a feature appears in both
internal_optgroups_available/optgroups_retired and the auto-generated
optgroups_names?
--
Pino Toscano
4:28 a.m.
New subject: [PATCH 1/4] generator: add always-available optgroups
On Wed, May 21, 2014 at 10:47:04AM +0200, Pino Toscano wrote:
On Tuesday 20 May 2014 21:40:21 Richard W.M. Jones wrote:
> On Tue, May 20, 2014 at 07:54:45PM +0200, Pino Toscano wrote:
> > Support the possibility to have optional groups always enabled (e.g.
> > because they were present in the past, and they need to be kept for
> > users).
> > Add and use few helper optgroups-related functions to deal also with
> > them.
>
> What do you think about the attached addition to this patch?
let optgroups_names_all =
- List.sort compare (optgroups_names @ optgroups_available)
+ List.sort compare (optgroups_names @ optgroups_retired)
Wouldn't this give duplicate values if a feature appears in both
internal_optgroups_available/optgroups_retired and the auto-generated
optgroups_names?
If we only put retired optgroups into the optgroups_retired list then
it's not a problem.
Actually your comment does explain what the loop does that I removed
in my patch. I was wondering what it was for since it would have no
effect in the current code (where optgroups_retired are only retired
optgroups).
I think a better way to do this is to have the generator check -- and
die with an error -- if retired optgroups appear in the list of actions.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
12:54 p.m.
New subject: [PATCH 2/4] Make realpath mandatory
It has been used unconditionally already for quite some time, so having
just the "realpath" command conditional on the presence of it does not
make much sense.
Drop the configure/build system handling of it, and make the "realpath"
command no more optional (but keeping the "realpath" feature,
unconditionally available now).
---
configure.ac | 1 -
daemon/realpath.c | 14 --------------
generator/actions.ml | 1 -
generator/optgroups.ml | 1 +
4 files changed, 1 insertion(+), 16 deletions(-)
diff --git a/configure.ac b/configure.ac
index 552ed77..f3fd10f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -308,7 +308,6 @@ AC_CHECK_FUNCS([\
ntohs \
posix_fallocate \
posix_fadvise \
- realpath \
removexattr \
setitimer \
setxattr \
diff --git a/daemon/realpath.c b/daemon/realpath.c
index 99f5247..1e07291 100644
--- a/daemon/realpath.c
+++ b/daemon/realpath.c
@@ -33,14 +33,6 @@
#include "optgroups.h"
#include "actions.h"
-#ifdef HAVE_REALPATH
-
-int
-optgroup_realpath_available (void)
-{
- return 1;
-}
-
char *
do_realpath (const char *path)
{
@@ -57,12 +49,6 @@ do_realpath (const char *path)
return ret; /* caller frees */
}
-#else /* !HAVE_REALPATH */
-
-OPTGROUP_REALPATH_NOT_AVAILABLE
-
-#endif /* !HAVE_REALPATH */
-
static int find_path_element (int fd_cwd, int is_end, const char *name, char
**name_ret);
char *
diff --git a/generator/actions.ml b/generator/actions.ml
index 0826137..85503ce 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -6665,7 +6665,6 @@ matching lines." };
name = "realpath";
style = RString "rpath", [Pathname "path"], [];
proc_nr = Some 163;
- optional = Some "realpath";
tests = [
InitISOFS, Always, TestResultString (
[["realpath"; "/../directory"]], "/directory"), []
diff --git a/generator/optgroups.ml b/generator/optgroups.ml
index a08f9c3..eb5ff74 100644
--- a/generator/optgroups.ml
+++ b/generator/optgroups.ml
@@ -31,6 +31,7 @@ open Actions
* but libguestfs ought to provide them to the user.
*)
let internal_optgroups_available = [
+ "realpath";
]
(* Create list of optional groups. *)
--
1.9.0
3:41 p.m.
New subject: [PATCH 2/4] Make realpath mandatory
On Tue, May 20, 2014 at 07:54:46PM +0200, Pino Toscano wrote:
It has been used unconditionally already for quite some time, so
having
just the "realpath" command conditional on the presence of it does not
make much sense.
Drop the configure/build system handling of it, and make the "realpath"
command no more optional (but keeping the "realpath" feature,
unconditionally available now).
It would be good to mention commit a86eb0e0d2c67e2 in the
commit message.
Apart from that, ACK.
Rich.
configure.ac | 1 -
daemon/realpath.c | 14 --------------
generator/actions.ml | 1 -
generator/optgroups.ml | 1 +
4 files changed, 1 insertion(+), 16 deletions(-)
diff --git a/configure.ac b/configure.ac
index 552ed77..f3fd10f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -308,7 +308,6 @@ AC_CHECK_FUNCS([\
ntohs \
posix_fallocate \
posix_fadvise \
- realpath \
removexattr \
setitimer \
setxattr \
diff --git a/daemon/realpath.c b/daemon/realpath.c
index 99f5247..1e07291 100644
--- a/daemon/realpath.c
+++ b/daemon/realpath.c
@@ -33,14 +33,6 @@
#include "optgroups.h"
#include "actions.h"
-#ifdef HAVE_REALPATH
-
-int
-optgroup_realpath_available (void)
-{
- return 1;
-}
-
char *
do_realpath (const char *path)
{
@@ -57,12 +49,6 @@ do_realpath (const char *path)
return ret; /* caller frees */
}
-#else /* !HAVE_REALPATH */
-
-OPTGROUP_REALPATH_NOT_AVAILABLE
-
-#endif /* !HAVE_REALPATH */
-
static int find_path_element (int fd_cwd, int is_end, const char *name, char
**name_ret);
char *
diff --git a/generator/actions.ml b/generator/actions.ml
index 0826137..85503ce 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -6665,7 +6665,6 @@ matching lines." };
name = "realpath";
style = RString "rpath", [Pathname "path"], [];
proc_nr = Some 163;
- optional = Some "realpath";
tests = [
InitISOFS, Always, TestResultString (
[["realpath"; "/../directory"]], "/directory"),
[]
diff --git a/generator/optgroups.ml b/generator/optgroups.ml
index a08f9c3..eb5ff74 100644
--- a/generator/optgroups.ml
+++ b/generator/optgroups.ml
@@ -31,6 +31,7 @@ open Actions
* but libguestfs ought to provide them to the user.
*)
let internal_optgroups_available = [
+ "realpath";
]
(* Create list of optional groups. *)
--
1.9.0
_______________________________________________
Libguestfs mailing list
Libguestfs(a)redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
12:54 p.m.
New subject: [PATCH 3/4] daemon: add sysroot_realpath
Similar to sysroot_path, but first resolves (using realpath) the given
path within sysroot.
---
daemon/daemon.h | 1 +
daemon/guestfsd.c | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 6535658..94537b7 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -51,6 +51,7 @@ extern const char *sysroot;
extern size_t sysroot_len;
extern char *sysroot_path (const char *path);
+extern char *sysroot_realpath (const char *path);
extern int is_root_device (const char *device);
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index 6519ea6..622bda1 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -455,6 +455,27 @@ sysroot_path (const char *path)
return r;
}
+/* Resolve path within sysroot, calling sysroot_path on the resolved path.
+ *
+ * Caller must check for NULL and call reply_with_perror ("malloc/realpath")
+ * if it is. Caller must also free the string.
+ *
+ * See also the custom %R printf formatter which does shell quoting too.
+ */
+char *
+sysroot_realpath (const char *path)
+{
+ CLEANUP_FREE char *rp = NULL;
+
+ CHROOT_IN;
+ rp = realpath (path, NULL);
+ CHROOT_OUT;
+ if (rp == NULL)
+ return NULL;
+
+ return sysroot_path (rp);
+}
+
int
xwrite (int sock, const void *v_buf, size_t len)
{
--
1.9.0
3:41 p.m.
New subject: [PATCH 3/4] daemon: add sysroot_realpath
On Tue, May 20, 2014 at 07:54:47PM +0200, Pino Toscano wrote:
Similar to sysroot_path, but first resolves (using realpath) the
given
path within sysroot.
---
daemon/daemon.h | 1 +
daemon/guestfsd.c | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 6535658..94537b7 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -51,6 +51,7 @@ extern const char *sysroot;
extern size_t sysroot_len;
extern char *sysroot_path (const char *path);
+extern char *sysroot_realpath (const char *path);
extern int is_root_device (const char *device);
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index 6519ea6..622bda1 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -455,6 +455,27 @@ sysroot_path (const char *path)
return r;
}
+/* Resolve path within sysroot, calling sysroot_path on the resolved path.
+ *
+ * Caller must check for NULL and call reply_with_perror ("malloc/realpath")
+ * if it is. Caller must also free the string.
+ *
+ * See also the custom %R printf formatter which does shell quoting too.
+ */
+char *
+sysroot_realpath (const char *path)
+{
+ CLEANUP_FREE char *rp = NULL;
+
+ CHROOT_IN;
+ rp = realpath (path, NULL);
+ CHROOT_OUT;
+ if (rp == NULL)
+ return NULL;
+
+ return sysroot_path (rp);
+}
+
int
xwrite (int sock, const void *v_buf, size_t len)
{
--
1.9.0
ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
12:54 p.m.
New subject: [PATCH 4/4] daemon: scrub-file: resolve the path before calling scrub (RHBZ#1099490).
Resolve the given path within the chroot, so scrub can be invoked
outside the chroot on an already-resolved path.
Add few tests in scrub-file for this and other similar issues.
---
daemon/scrub.c | 7 +++++--
generator/actions.ml | 13 ++++++++++++-
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/daemon/scrub.c b/daemon/scrub.c
index cd880b9..c10b0a0 100644
--- a/daemon/scrub.c
+++ b/daemon/scrub.c
@@ -58,8 +58,11 @@ do_scrub_file (const char *file)
CLEANUP_FREE char *err = NULL;
int r;
- /* Make the path relative to /sysroot. */
- buf = sysroot_path (file);
+ /* Resolve the path to the file, and make the result relative to /sysroot.
+ * If it fails, then the file most probably does not exist or "file" is
+ * a symlink pointing outside the chroot.
+ */
+ buf = sysroot_realpath (file);
if (!buf) {
reply_with_perror ("malloc");
return -1;
diff --git a/generator/actions.ml b/generator/actions.ml
index 85503ce..ed65c6e 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -5719,7 +5719,18 @@ manual page for more details." };
tests = [
InitScratchFS, Always, TestRun (
[["write"; "/scrub_file"; "content"];
- ["scrub_file"; "/scrub_file"]]), []
+ ["scrub_file"; "/scrub_file"]]), [];
+ InitScratchFS, Always, TestRun (
+ [["write"; "/scrub_file_2"; "content"];
+ ["ln_s"; "/scrub_file_2"; "/scrub_file_2_link"];
+ ["scrub_file"; "/scrub_file_2_link"]]), [];
+ InitScratchFS, Always, TestLastFail (
+ [["ln_s"; "/scrub_file_3_notexisting";
"/scrub_file_3_link"];
+ ["scrub_file"; "/scrub_file_3_link"]]), [];
+ InitScratchFS, Always, TestLastFail (
+ [["write"; "/scrub_file_4"; "content"];
+ ["ln_s"; "../sysroot/scrub_file_4";
"/scrub_file_4_link"];
+ ["scrub_file"; "/scrub_file_4_link"]]), [];
];
shortdesc = "scrub (securely wipe) a file";
longdesc = "\
--
1.9.0
3:41 p.m.
New subject: [PATCH 4/4] daemon: scrub-file: resolve the path before calling scrub (RHBZ#1099490).
On Tue, May 20, 2014 at 07:54:48PM +0200, Pino Toscano wrote:
Resolve the given path within the chroot, so scrub can be invoked
outside the chroot on an already-resolved path.
Add few tests in scrub-file for this and other similar issues.
ACK.
Rich.
--
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/
2:03 p.m.
On Tue, May 20, 2014 at 06:16:37PM +0200, Pino Toscano wrote:
Good idea. Do you have also an idea which daemon functions, other
than
scrub-files and realpath itself, might need such handling?
Not without examining every call.
The only other bug that has been filed in this area (by me as it
happens) is https://bugzilla.redhat.com/show_bug.cgi?id=604041 which
covers a variety of APIs (grep for guestfs_.* in src/fuse.c).
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
8:15 a.m.
New subject: [PATCH 1/4] generator: add always-available optgroups
Support the possibility to have optional groups always enabled (e.g.
because they were present in the past, and they need to be kept for
users).
Add and use few helper optgroups-related functions to deal also with
them.
---
generator/daemon.ml | 24 ++++++++++++++++++++----
generator/optgroups.ml | 22 ++++++++++++++++++++++
2 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/generator/daemon.ml b/generator/daemon.ml
index 548982b..ca748d2 100644
--- a/generator/daemon.ml
+++ b/generator/daemon.ml
@@ -713,11 +713,27 @@ and generate_daemon_optgroups_c () =
pr "#include \"optgroups.h\"\n";
pr "\n";
+ if optgroups_retired <> [] then (
+ pr "static int\n";
+ pr "dummy_available (void)\n";
+ pr "{\n";
+ pr " return 1;\n";
+ pr "}\n";
+ pr "\n";
+
+ List.iter (
+ fun group ->
+ pr "#define optgroup_%s_available dummy_available\n" group;
+ ) optgroups_retired;
+
+ pr "\n";
+ );
+
pr "struct optgroup optgroups[] = {\n";
List.iter (
- fun (group, _) ->
+ fun group ->
pr " { \"%s\", optgroup_%s_available },\n" group group
- ) optgroups;
+ ) optgroups_names_all;
pr " { NULL, NULL }\n";
pr "};\n"
@@ -729,9 +745,9 @@ and generate_daemon_optgroups_h () =
pr "\n";
List.iter (
- fun (group, _) ->
+ fun group ->
pr "extern int optgroup_%s_available (void);\n" group
- ) optgroups;
+ ) optgroups_names;
pr "\n";
diff --git a/generator/optgroups.ml b/generator/optgroups.ml
index 2a348e7..1c7f45e 100644
--- a/generator/optgroups.ml
+++ b/generator/optgroups.ml
@@ -19,8 +19,16 @@
(* Please read generator/README first. *)
open Types
+open Utils
open Actions
+(* The list of optional groups which need to be in the daemon as always
+ * available. These are "retired" as they no longer appear in the
+ * list of functions.
+ *)
+let optgroups_retired = [
+]
+
(* Create list of optional groups. *)
let optgroups =
let h = Hashtbl.create 13 in
@@ -42,3 +50,17 @@ let optgroups =
group, fns
) groups in
List.sort (fun x y -> compare (fst x) (fst y)) groups
+
+let optgroups_names =
+ fst (List.split optgroups)
+
+let optgroups_names_all =
+ List.sort compare (optgroups_names @ optgroups_retired)
+
+let () =
+ let file = "generator/optgroups.ml" in
+ List.iter (
+ fun x ->
+ if List.mem x optgroups_names then
+ failwithf "%s: optgroups_retired list contains optgroup with functions
(%s)" file x
+ ) optgroups_retired
--
1.9.0
8:15 a.m.
New subject: [PATCH 2/4] Make realpath mandatory
commit a86eb0e0d2c67e2153139c681632edc72162b8ed made it an optional
feature, as on Windows it was not available; on the other hand, realpath
has been used unconditionally already for quite some time, so having
just the "realpath" command conditional on the presence of it does not
make much sense.
Drop the configure/build system handling of it, make the "realpath"
command no more optional, and keep the "realpath" feature as
unconditionally available now.
---
configure.ac | 1 -
daemon/realpath.c | 14 --------------
generator/actions.ml | 1 -
generator/optgroups.ml | 1 +
4 files changed, 1 insertion(+), 16 deletions(-)
diff --git a/configure.ac b/configure.ac
index 552ed77..f3fd10f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -308,7 +308,6 @@ AC_CHECK_FUNCS([\
ntohs \
posix_fallocate \
posix_fadvise \
- realpath \
removexattr \
setitimer \
setxattr \
diff --git a/daemon/realpath.c b/daemon/realpath.c
index 99f5247..1e07291 100644
--- a/daemon/realpath.c
+++ b/daemon/realpath.c
@@ -33,14 +33,6 @@
#include "optgroups.h"
#include "actions.h"
-#ifdef HAVE_REALPATH
-
-int
-optgroup_realpath_available (void)
-{
- return 1;
-}
-
char *
do_realpath (const char *path)
{
@@ -57,12 +49,6 @@ do_realpath (const char *path)
return ret; /* caller frees */
}
-#else /* !HAVE_REALPATH */
-
-OPTGROUP_REALPATH_NOT_AVAILABLE
-
-#endif /* !HAVE_REALPATH */
-
static int find_path_element (int fd_cwd, int is_end, const char *name, char
**name_ret);
char *
diff --git a/generator/actions.ml b/generator/actions.ml
index 0826137..85503ce 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -6665,7 +6665,6 @@ matching lines." };
name = "realpath";
style = RString "rpath", [Pathname "path"], [];
proc_nr = Some 163;
- optional = Some "realpath";
tests = [
InitISOFS, Always, TestResultString (
[["realpath"; "/../directory"]], "/directory"), []
diff --git a/generator/optgroups.ml b/generator/optgroups.ml
index 1c7f45e..d13d794 100644
--- a/generator/optgroups.ml
+++ b/generator/optgroups.ml
@@ -27,6 +27,7 @@ open Actions
* list of functions.
*)
let optgroups_retired = [
+ "realpath";
]
(* Create list of optional groups. *)
--
1.9.0
8:15 a.m.
New subject: [PATCH 3/4] daemon: add sysroot_realpath
Similar to sysroot_path, but first resolves (using realpath) the given
path within sysroot.
---
daemon/daemon.h | 1 +
daemon/guestfsd.c | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 6535658..94537b7 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -51,6 +51,7 @@ extern const char *sysroot;
extern size_t sysroot_len;
extern char *sysroot_path (const char *path);
+extern char *sysroot_realpath (const char *path);
extern int is_root_device (const char *device);
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index 6519ea6..622bda1 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -455,6 +455,27 @@ sysroot_path (const char *path)
return r;
}
+/* Resolve path within sysroot, calling sysroot_path on the resolved path.
+ *
+ * Caller must check for NULL and call reply_with_perror ("malloc/realpath")
+ * if it is. Caller must also free the string.
+ *
+ * See also the custom %R printf formatter which does shell quoting too.
+ */
+char *
+sysroot_realpath (const char *path)
+{
+ CLEANUP_FREE char *rp = NULL;
+
+ CHROOT_IN;
+ rp = realpath (path, NULL);
+ CHROOT_OUT;
+ if (rp == NULL)
+ return NULL;
+
+ return sysroot_path (rp);
+}
+
int
xwrite (int sock, const void *v_buf, size_t len)
{
--
1.9.0
8:15 a.m.
New subject: [PATCH 4/4] daemon: scrub-file: resolve the path before calling scrub (RHBZ#1099490).
Resolve the given path within the chroot, so scrub can be invoked
outside the chroot on an already-resolved path.
Add few tests in scrub-file for this and other similar issues.
---
daemon/scrub.c | 7 +++++--
generator/actions.ml | 13 ++++++++++++-
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/daemon/scrub.c b/daemon/scrub.c
index cd880b9..c10b0a0 100644
--- a/daemon/scrub.c
+++ b/daemon/scrub.c
@@ -58,8 +58,11 @@ do_scrub_file (const char *file)
CLEANUP_FREE char *err = NULL;
int r;
- /* Make the path relative to /sysroot. */
- buf = sysroot_path (file);
+ /* Resolve the path to the file, and make the result relative to /sysroot.
+ * If it fails, then the file most probably does not exist or "file" is
+ * a symlink pointing outside the chroot.
+ */
+ buf = sysroot_realpath (file);
if (!buf) {
reply_with_perror ("malloc");
return -1;
diff --git a/generator/actions.ml b/generator/actions.ml
index 85503ce..ed65c6e 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -5719,7 +5719,18 @@ manual page for more details." };
tests = [
InitScratchFS, Always, TestRun (
[["write"; "/scrub_file"; "content"];
- ["scrub_file"; "/scrub_file"]]), []
+ ["scrub_file"; "/scrub_file"]]), [];
+ InitScratchFS, Always, TestRun (
+ [["write"; "/scrub_file_2"; "content"];
+ ["ln_s"; "/scrub_file_2"; "/scrub_file_2_link"];
+ ["scrub_file"; "/scrub_file_2_link"]]), [];
+ InitScratchFS, Always, TestLastFail (
+ [["ln_s"; "/scrub_file_3_notexisting";
"/scrub_file_3_link"];
+ ["scrub_file"; "/scrub_file_3_link"]]), [];
+ InitScratchFS, Always, TestLastFail (
+ [["write"; "/scrub_file_4"; "content"];
+ ["ln_s"; "../sysroot/scrub_file_4";
"/scrub_file_4_link"];
+ ["scrub_file"; "/scrub_file_4_link"]]), [];
];
shortdesc = "scrub (securely wipe) a file";
longdesc = "\
--
1.9.0
10:12 a.m.
New subject: [PATCH 4/4] daemon: scrub-file: resolve the path before calling scrub (RHBZ#1099490).
On Wed, May 21, 2014 at 03:15:39PM +0200, Pino Toscano wrote:
Resolve the given path within the chroot, so scrub can be invoked
outside the chroot on an already-resolved path.
Add few tests in scrub-file for this and other similar issues.
---
ACK whole series.
Thanks,
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
3803
days inactive
3804
days old
18 comments
2 participants
participants (2)
-
Pino Toscano -
Richard W.M. Jones