Re: [PATCH v2 kvmtool] Make static libc and guest-init functionality optional.

[Will Deacon <will.deacon@arm.com>]

Hi Dmitri,

On Fri, Sep 11, 2015 at 03:40:00PM +0100, Dimitri John Ledkov wrote:
> If one typically only boots full disk-images, one wouldn't necessaraly
> want to statically link glibc, for the guest-init feature of the
> kvmtool. As statically linked glibc triggers haevy security
> maintainance.
> 
> Signed-off-by: Dimitri John Ledkov <dimitri.j.ledkov@intel.com>
> ---
>  Changes since v1:
>  - rename CONFIG_HAS_LIBC to CONFIG_GUEST_INIT for clarity
>  - use more ifdefs, instead of runtime check of _binary_guest_init_size==0

The idea looks good, but I think we can tidy some of this up at the same
time by moving all the guest_init code in builtin_setup.c.

How about the patch below?

Will

--->8

>From cdce942c1a3a04635065a7972ca4e21386664756 Mon Sep 17 00:00:00 2001
From: Dimitri John Ledkov <dimitri.j.ledkov@intel.com>
Date: Fri, 11 Sep 2015 15:40:00 +0100
Subject: [PATCH] Make static libc and guest-init functionality optional.

If one typically only boots full disk-images, one wouldn't necessaraly
want to statically link glibc, for the guest-init feature of the
kvmtool. As statically linked glibc triggers haevy security
maintainance.

Signed-off-by: Dimitri John Ledkov <dimitri.j.ledkov@intel.com>
[will: moved all the guest_init handling into builtin_setup.c]
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 Makefile                    | 12 +++++++-----
 builtin-run.c               | 29 +----------------------------
 builtin-setup.c             | 19 ++++++++++++++-----
 include/kvm/builtin-setup.h |  1 +
 4 files changed, 23 insertions(+), 38 deletions(-)

diff --git a/Makefile b/Makefile
index 7b17d529d13b..f1701aa7b8ec 100644
--- a/Makefile
+++ b/Makefile
@@ -34,8 +34,6 @@ bindir_SQ = $(subst ','\'',$(bindir))
 PROGRAM	:= lkvm
 PROGRAM_ALIAS := vm
 
-GUEST_INIT := guest/init
-
 OBJS	+= builtin-balloon.o
 OBJS	+= builtin-debug.o
 OBJS	+= builtin-help.o
@@ -279,8 +277,13 @@ ifeq ($(LTO),1)
 	endif
 endif
 
-ifneq ($(call try-build,$(SOURCE_STATIC),,-static),y)
-        $(error No static libc found. Please install glibc-static package.)
+ifeq ($(call try-build,$(SOURCE_STATIC),,-static),y)
+	CFLAGS		+= -DCONFIG_GUEST_INIT
+	GUEST_INIT	:= guest/init
+	GUEST_OBJS	= guest/guest_init.o
+else
+	$(warning No static libc found. Skipping guest init)
+	NOTFOUND        += static-libc
 endif
 
 ifeq (y,$(ARCH_WANT_LIBFDT))
@@ -356,7 +359,6 @@ c_flags	= -Wp,-MD,$(depfile) $(CFLAGS)
 # $(OTHEROBJS) are things that do not get substituted like this.
 #
 STATIC_OBJS = $(patsubst %.o,%.static.o,$(OBJS) $(OBJS_STATOPT))
-GUEST_OBJS = guest/guest_init.o
 
 $(PROGRAM)-static:  $(STATIC_OBJS) $(OTHEROBJS) $(GUEST_INIT)
 	$(E) "  LINK    " $@
diff --git a/builtin-run.c b/builtin-run.c
index 1ee75ad3f010..e0c87329e52b 100644
--- a/builtin-run.c
+++ b/builtin-run.c
@@ -59,9 +59,6 @@ static int  kvm_run_wrapper;
 
 bool do_debug_print = false;
 
-extern char _binary_guest_init_start;
-extern char _binary_guest_init_size;
-
 static const char * const run_usage[] = {
 	"lkvm run [<options>] [<kernel image>]",
 	NULL
@@ -345,30 +342,6 @@ void kvm_run_help(void)
 	usage_with_options(run_usage, options);
 }
 
-static int kvm_setup_guest_init(struct kvm *kvm)
-{
-	const char *rootfs = kvm->cfg.custom_rootfs_name;
-	char tmp[PATH_MAX];
-	size_t size;
-	int fd, ret;
-	char *data;
-
-	/* Setup /virt/init */
-	size = (size_t)&_binary_guest_init_size;
-	data = (char *)&_binary_guest_init_start;
-	snprintf(tmp, PATH_MAX, "%s%s/virt/init", kvm__get_dir(), rootfs);
-	remove(tmp);
-	fd = open(tmp, O_CREAT | O_WRONLY, 0755);
-	if (fd < 0)
-		die("Fail to setup %s", tmp);
-	ret = xwrite(fd, data, size);
-	if (ret < 0)
-		die("Fail to setup %s", tmp);
-	close(fd);
-
-	return 0;
-}
-
 static int kvm_run_set_sandbox(struct kvm *kvm)
 {
 	const char *guestfs_name = kvm->cfg.custom_rootfs_name;
@@ -631,7 +604,7 @@ static struct kvm *kvm_cmd_run_init(int argc, const char **argv)
 
 			if (!kvm->cfg.no_dhcp)
 				strcat(real_cmdline, "  ip=dhcp");
-			if (kvm_setup_guest_init(kvm))
+			if (kvm_setup_guest_init(kvm->cfg.custom_rootfs_name))
 				die("Failed to setup init for guest.");
 		}
 	} else if (!strstr(real_cmdline, "root=")) {
diff --git a/builtin-setup.c b/builtin-setup.c
index 8b45c5645ad4..40fef15dbbe4 100644
--- a/builtin-setup.c
+++ b/builtin-setup.c
@@ -16,9 +16,6 @@
 #include <sys/mman.h>
 #include <fcntl.h>
 
-extern char _binary_guest_init_start;
-extern char _binary_guest_init_size;
-
 static const char *instance_name;
 
 static const char * const setup_usage[] = {
@@ -124,7 +121,11 @@ static const char *guestfs_symlinks[] = {
 	"/etc/ld.so.conf",
 };
 
-static int copy_init(const char *guestfs_name)
+#ifdef CONFIG_GUEST_INIT
+extern char _binary_guest_init_start;
+extern char _binary_guest_init_size;
+
+int kvm_setup_guest_init(const char *guestfs_name)
 {
 	char path[PATH_MAX];
 	size_t size;
@@ -144,7 +145,15 @@ static int copy_init(const char *guestfs_name)
 	close(fd);
 
 	return 0;
+
+}
+#else
+int kvm_setup_guest_init(const char *guestfs_name)
+{
+	die("Guest init image not compiled in");
+	return 0;
 }
+#endif
 
 static int copy_passwd(const char *guestfs_name)
 {
@@ -222,7 +231,7 @@ static int do_setup(const char *guestfs_name)
 		make_guestfs_symlink(guestfs_name, guestfs_symlinks[i]);
 	}
 
-	ret = copy_init(guestfs_name);
+	ret = kvm_setup_guest_init(guestfs_name);
 	if (ret < 0)
 		return ret;
 
diff --git a/include/kvm/builtin-setup.h b/include/kvm/builtin-setup.h
index 4a8d7ee39425..239bbbdce09e 100644
--- a/include/kvm/builtin-setup.h
+++ b/include/kvm/builtin-setup.h
@@ -7,5 +7,6 @@ int kvm_cmd_setup(int argc, const char **argv, const char *prefix);
 void kvm_setup_help(void) NORETURN;
 int kvm_setup_create_new(const char *guestfs_name);
 void kvm_setup_resolv(const char *guestfs_name);
+int kvm_setup_guest_init(const char *guestfs_name);
 
 #endif
-- 
2.1.4