Hi tech@,
The below patch removes calls to realpath(3) when looking up a qcow2
base image. Previous thread:
https://marc.info/?t=161562496400002&r=1&w=2
In short, the calls were failing inside vmctl, because of unveil. The
other thread has alternative solutions but I think this is simplest.
I included a regression test demonstrating the vmctl bug, in case
there's interest. I tested vmd manually as described in the other
thread.
I also added a check in case dirname(3) fails --- I don't think it
currently can, but better safe than sorry, I figure. (Noticed by Dave
in the other thread.)
- James
diff --git a/regress/usr.sbin/Makefile b/regress/usr.sbin/Makefile
index 60e2178d3c7..146f9c9f322 100644
--- a/regress/usr.sbin/Makefile
+++ b/regress/usr.sbin/Makefile
@@ -15,6 +15,7 @@ SUBDIR += rpki-client
SUBDIR += snmpd
SUBDIR += switchd
SUBDIR += syslogd
+SUBDIR += vmctl
.if ${MACHINE} == "amd64" || ${MACHINE} == "i386"
SUBDIR += vmd
diff --git a/regress/usr.sbin/vmctl/Makefile b/regress/usr.sbin/vmctl/Makefile
new file mode 100644
index 00000000000..8fa87f0f6f0
--- /dev/null
+++ b/regress/usr.sbin/vmctl/Makefile
@@ -0,0 +1,34 @@
+# $OpenBSD$
+
+REGRESS_TARGETS = run-regress-convert-with-base-path
+
+run-regress-convert-with-base-path:
+ # non-relative base path
+ rm -f *.qcow2
+ vmctl create -s 1m base.qcow2
+ vmctl create -b ${PWD}/base.qcow2 source.qcow2
+ vmctl create -i source.qcow2 dest.qcow2
+
+ # relative base path; two base images
+ rm -f *.qcow2
+ vmctl create -s 1m base0.qcow2
+ vmctl create -b base0.qcow2 base1.qcow2
+ vmctl create -b base1.qcow2 source.qcow2
+ vmctl create -i source.qcow2 dest.qcow2
+
+ # copy from a different directory
+ rm -rf dir *.qcow2
+ vmctl create -s 1m base.qcow2
+ vmctl create -b base.qcow2 source.qcow2
+ mkdir dir
+ cd dir; vmctl create -i ../source.qcow2 dest.qcow2
+
+ # base accessed through symlink
+ rm -rf dir sym *.qcow2
+ mkdir dir
+ cd dir; vmctl create -s 1m base.qcow2
+ cd dir; vmctl create -b base.qcow2 source.qcow2
+ ln -s dir sym
+ vmctl create -i sym/source.qcow2 dest.qcow2
+
+.include <bsd.regress.mk>
diff --git a/usr.sbin/vmd/vioqcow2.c b/usr.sbin/vmd/vioqcow2.c
index 34d0f116cc4..be8609f1644 100644
--- a/usr.sbin/vmd/vioqcow2.c
+++ b/usr.sbin/vmd/vioqcow2.c
@@ -145,8 +145,8 @@ virtio_qcow2_init(struct virtio_backing *file, off_t *szp,
int *fd, size_t nfd)
ssize_t
virtio_qcow2_get_base(int fd, char *path, size_t npath, const char *dpath)
{
+ char pathbuf[PATH_MAX];
char dpathbuf[PATH_MAX];
- char expanded[PATH_MAX];
struct qcheader header;
uint64_t backingoff;
uint32_t backingsz;
@@ -180,27 +180,23 @@ virtio_qcow2_get_base(int fd, char *path, size_t npath,
const char *dpath)
* rather than relative to the directory vmd happens to be running in,
* since this is the only userful interpretation.
*/
- if (path[0] == '/') {
- if (realpath(path, expanded) == NULL ||
- strlcpy(path, expanded, npath) >= npath) {
- log_warnx("unable to resolve %s", path);
+ if (path[0] != '/') {
+ if (strlcpy(pathbuf, path, sizeof(pathbuf)) >=
+ sizeof(pathbuf)) {
+ log_warnx("path too long: %s", path);
return -1;
}
- } else {
if (strlcpy(dpathbuf, dpath, sizeof(dpathbuf)) >=
sizeof(dpathbuf)) {
log_warnx("path too long: %s", dpath);
return -1;
}
- s = dirname(dpathbuf);
- if (snprintf(expanded, sizeof(expanded),
- "%s/%s", s, path) >= (int)sizeof(expanded)) {
- log_warnx("path too long: %s/%s", s, path);
+ if ((s = dirname(dpathbuf)) == NULL) {
+ log_warn("dirname");
return -1;
}
- if (npath < PATH_MAX ||
- realpath(expanded, path) == NULL) {
- log_warnx("unable to resolve %s", path);
+ if (snprintf(path, npath, "%s/%s", s, pathbuf) >= (int)npath) {
+ log_warnx("path too long: %s/%s", s, path);
return -1;
}
}
