diff mbox

[v2,kvmtool] Make static libc and guest-init functionality optional.

Message ID 20150915172033.GI31157@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Will Deacon Sept. 15, 2015, 5:20 p.m. UTC 
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(-)

Comments

Dimitri John Ledkov Sept. 16, 2015, 8:08 a.m. UTC | #1 
Hello Will,

Looks good to me =)

On 15 September 2015 at 18:20, Will Deacon <will.deacon@arm.com> wrote:
> 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
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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