* [PATCH kvmtool] Make static libc and guest-init functionality optional. @ 2015-09-04 12:04 Dimitri John Ledkov 2015-09-11 12:47 ` Andre Przywara 2015-09-11 14:40 ` [PATCH v2 " Dimitri John Ledkov 0 siblings, 2 replies; 6+ messages in thread From: Dimitri John Ledkov @ 2015-09-04 12:04 UTC (permalink / raw) To: kvm 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> --- Makefile | 11 ++++++----- builtin-run.c | 7 +++++++ builtin-setup.c | 7 +++++++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 1534e6f..42a629a 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,12 @@ 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_HAS_LIBC + GUEST_INIT := guest/init + GUEST_OBJS = guest/guest_init.o +else + NOTFOUND += static-libc endif ifeq (y,$(ARCH_WANT_LIBFDT)) @@ -356,7 +358,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 1ee75ad..0f67471 100644 --- a/builtin-run.c +++ b/builtin-run.c @@ -59,8 +59,13 @@ static int kvm_run_wrapper; bool do_debug_print = false; +#ifdef CONFIG_HAS_LIBC extern char _binary_guest_init_start; extern char _binary_guest_init_size; +#else +static char _binary_guest_init_start=0; +static char _binary_guest_init_size=0; +#endif static const char * const run_usage[] = { "lkvm run [<options>] [<kernel image>]", @@ -354,6 +359,8 @@ static int kvm_setup_guest_init(struct kvm *kvm) char *data; /* Setup /virt/init */ + if (!_binary_guest_init_size) + die("Guest init not compiled"); 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); diff --git a/builtin-setup.c b/builtin-setup.c index 8b45c56..d77e5e0 100644 --- a/builtin-setup.c +++ b/builtin-setup.c @@ -16,8 +16,13 @@ #include <sys/mman.h> #include <fcntl.h> +#ifdef CONFIG_HAS_LIBC extern char _binary_guest_init_start; extern char _binary_guest_init_size; +#else +static char _binary_guest_init_start=0; +static char _binary_guest_init_size=0; +#endif static const char *instance_name; @@ -131,6 +136,8 @@ static int copy_init(const char *guestfs_name) int fd, ret; char *data; + if (!_binary_guest_init_size) + die("Guest init not compiled"); size = (size_t)&_binary_guest_init_size; data = (char *)&_binary_guest_init_start; snprintf(path, PATH_MAX, "%s%s/virt/init", kvm__get_dir(), guestfs_name); -- 2.1.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH kvmtool] Make static libc and guest-init functionality optional. 2015-09-04 12:04 [PATCH kvmtool] Make static libc and guest-init functionality optional Dimitri John Ledkov @ 2015-09-11 12:47 ` Andre Przywara 2015-09-11 13:44 ` Dimitri John Ledkov 2015-09-11 14:40 ` [PATCH v2 " Dimitri John Ledkov 1 sibling, 1 reply; 6+ messages in thread From: Andre Przywara @ 2015-09-11 12:47 UTC (permalink / raw) To: Dimitri John Ledkov; +Cc: kvm, Will Deacon Hi Dimitri, thanks for sharing this patch and sorry for the delay. (CC:ing Will) On 04/09/15 13:04, 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. I like the idea of making guest-init optional, and actually was bitten by this annoying static libc requirement once before. Some comments below: > > Signed-off-by: Dimitri John Ledkov <dimitri.j.ledkov@intel.com> > --- > Makefile | 11 ++++++----- > builtin-run.c | 7 +++++++ > builtin-setup.c | 7 +++++++ > 3 files changed, 20 insertions(+), 5 deletions(-) > > diff --git a/Makefile b/Makefile > index 1534e6f..42a629a 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,12 @@ 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_HAS_LIBC The name CONFIG_HAS_LIBC seems a bit misleading to me, so at least this symbol should read CONFIG_HAS_STATIC_LIBC. But I'd prefer to have it named after it's user instead: CONFIG_GUEST_INIT (or the like), since this is what it protects in the code. > + GUEST_INIT := guest/init > + GUEST_OBJS = guest/guest_init.o > +else > + NOTFOUND += static-libc > endif > > ifeq (y,$(ARCH_WANT_LIBFDT)) > @@ -356,7 +358,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 1ee75ad..0f67471 100644 > --- a/builtin-run.c > +++ b/builtin-run.c > @@ -59,8 +59,13 @@ static int kvm_run_wrapper; > > bool do_debug_print = false; > > +#ifdef CONFIG_HAS_LIBC > extern char _binary_guest_init_start; > extern char _binary_guest_init_size; > +#else > +static char _binary_guest_init_start=0; > +static char _binary_guest_init_size=0; > +#endif > > static const char * const run_usage[] = { > "lkvm run [<options>] [<kernel image>]", > @@ -354,6 +359,8 @@ static int kvm_setup_guest_init(struct kvm *kvm) > char *data; > > /* Setup /virt/init */ > + if (!_binary_guest_init_size) > + die("Guest init not compiled"); I wonder if comparing with 0 is safe in every case. I appreciate not spoiling the code with #ifdefs, but putting one around here seems cleaner to me (especially if you look at the error message). > 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); > diff --git a/builtin-setup.c b/builtin-setup.c > index 8b45c56..d77e5e0 100644 > --- a/builtin-setup.c > +++ b/builtin-setup.c > @@ -16,8 +16,13 @@ > #include <sys/mman.h> > #include <fcntl.h> > > +#ifdef CONFIG_HAS_LIBC > extern char _binary_guest_init_start; > extern char _binary_guest_init_size; > +#else > +static char _binary_guest_init_start=0; > +static char _binary_guest_init_size=0; > +#endif > > static const char *instance_name; > > @@ -131,6 +136,8 @@ static int copy_init(const char *guestfs_name) > int fd, ret; > char *data; > > + if (!_binary_guest_init_size) > + die("Guest init not compiled"); Same as above. Cheers, Andre. > size = (size_t)&_binary_guest_init_size; > data = (char *)&_binary_guest_init_start; > snprintf(path, PATH_MAX, "%s%s/virt/init", kvm__get_dir(), guestfs_name); > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH kvmtool] Make static libc and guest-init functionality optional. 2015-09-11 12:47 ` Andre Przywara @ 2015-09-11 13:44 ` Dimitri John Ledkov 0 siblings, 0 replies; 6+ messages in thread From: Dimitri John Ledkov @ 2015-09-11 13:44 UTC (permalink / raw) To: Andre Przywara; +Cc: kvm, Will Deacon On 11 September 2015 at 13:47, Andre Przywara <andre.przywara@arm.com> wrote: > Hi Dimitri, > > thanks for sharing this patch and sorry for the delay. No worries, I have a few more patches to send, polishing them for release. > > (CC:ing Will) > > On 04/09/15 13:04, 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. > > I like the idea of making guest-init optional, and actually was bitten > by this annoying static libc requirement once before. > Some comments below: > \o/ >> >> Signed-off-by: Dimitri John Ledkov <dimitri.j.ledkov@intel.com> >> --- >> Makefile | 11 ++++++----- >> builtin-run.c | 7 +++++++ >> builtin-setup.c | 7 +++++++ >> 3 files changed, 20 insertions(+), 5 deletions(-) >> >> diff --git a/Makefile b/Makefile >> index 1534e6f..42a629a 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,12 @@ 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_HAS_LIBC > > The name CONFIG_HAS_LIBC seems a bit misleading to me, so at least this > symbol should read CONFIG_HAS_STATIC_LIBC. But I'd prefer to have it > named after it's user instead: CONFIG_GUEST_INIT (or the like), since > this is what it protects in the code. > OK, sounds good. I am bad at naming things =) this looks good. >> + GUEST_INIT := guest/init >> + GUEST_OBJS = guest/guest_init.o >> +else >> + NOTFOUND += static-libc >> endif >> >> ifeq (y,$(ARCH_WANT_LIBFDT)) >> @@ -356,7 +358,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 1ee75ad..0f67471 100644 >> --- a/builtin-run.c >> +++ b/builtin-run.c >> @@ -59,8 +59,13 @@ static int kvm_run_wrapper; >> >> bool do_debug_print = false; >> >> +#ifdef CONFIG_HAS_LIBC >> extern char _binary_guest_init_start; >> extern char _binary_guest_init_size; >> +#else >> +static char _binary_guest_init_start=0; >> +static char _binary_guest_init_size=0; >> +#endif >> >> static const char * const run_usage[] = { >> "lkvm run [<options>] [<kernel image>]", >> @@ -354,6 +359,8 @@ static int kvm_setup_guest_init(struct kvm *kvm) >> char *data; >> >> /* Setup /virt/init */ >> + if (!_binary_guest_init_size) >> + die("Guest init not compiled"); > > I wonder if comparing with 0 is safe in every case. I appreciate not > spoiling the code with #ifdefs, but putting one around here seems > cleaner to me (especially if you look at the error message). Ok, I can put the #ifdef here as well. Note that the non-extern declaration will still be needed in the code above, as otherwise the build fails to link without static-libc. > >> 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); >> diff --git a/builtin-setup.c b/builtin-setup.c >> index 8b45c56..d77e5e0 100644 >> --- a/builtin-setup.c >> +++ b/builtin-setup.c >> @@ -16,8 +16,13 @@ >> #include <sys/mman.h> >> #include <fcntl.h> >> >> +#ifdef CONFIG_HAS_LIBC >> extern char _binary_guest_init_start; >> extern char _binary_guest_init_size; >> +#else >> +static char _binary_guest_init_start=0; >> +static char _binary_guest_init_size=0; >> +#endif >> >> static const char *instance_name; >> >> @@ -131,6 +136,8 @@ static int copy_init(const char *guestfs_name) >> int fd, ret; >> char *data; >> >> + if (!_binary_guest_init_size) >> + die("Guest init not compiled"); > > Same as above. Ack. > > Cheers, > Andre. > >> size = (size_t)&_binary_guest_init_size; >> data = (char *)&_binary_guest_init_start; >> snprintf(path, PATH_MAX, "%s%s/virt/init", kvm__get_dir(), guestfs_name); >> -- Regards, Dimitri. Pura Vida! https://clearlinux.org Open Source Technology Center Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 kvmtool] Make static libc and guest-init functionality optional. 2015-09-04 12:04 [PATCH kvmtool] Make static libc and guest-init functionality optional Dimitri John Ledkov 2015-09-11 12:47 ` Andre Przywara @ 2015-09-11 14:40 ` Dimitri John Ledkov 2015-09-15 17:20 ` Will Deacon 1 sibling, 1 reply; 6+ messages in thread From: Dimitri John Ledkov @ 2015-09-11 14:40 UTC (permalink / raw) To: kvm; +Cc: andre.przywara, will.deacon 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 Makefile | 11 ++++++----- builtin-run.c | 6 ++++++ builtin-setup.c | 6 ++++++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 1534e6f..bc6059c 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,12 @@ 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 + NOTFOUND += static-libc endif ifeq (y,$(ARCH_WANT_LIBFDT)) @@ -356,7 +358,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 1ee75ad..e27acd6 100644 --- a/builtin-run.c +++ b/builtin-run.c @@ -59,8 +59,10 @@ static int kvm_run_wrapper; bool do_debug_print = false; +#ifdef CONFIG_GUEST_INIT extern char _binary_guest_init_start; extern char _binary_guest_init_size; +#endif static const char * const run_usage[] = { "lkvm run [<options>] [<kernel image>]", @@ -347,6 +349,7 @@ void kvm_run_help(void) static int kvm_setup_guest_init(struct kvm *kvm) { +#ifdef CONFIG_GUEST_INIT const char *rootfs = kvm->cfg.custom_rootfs_name; char tmp[PATH_MAX]; size_t size; @@ -367,6 +370,9 @@ static int kvm_setup_guest_init(struct kvm *kvm) close(fd); return 0; +#else + die("Guest init not compiled"); +#endif } static int kvm_run_set_sandbox(struct kvm *kvm) diff --git a/builtin-setup.c b/builtin-setup.c index 8b45c56..ff796c3 100644 --- a/builtin-setup.c +++ b/builtin-setup.c @@ -16,8 +16,10 @@ #include <sys/mman.h> #include <fcntl.h> +#ifdef CONFIG_GUEST_INIT extern char _binary_guest_init_start; extern char _binary_guest_init_size; +#endif static const char *instance_name; @@ -126,6 +128,7 @@ static const char *guestfs_symlinks[] = { static int copy_init(const char *guestfs_name) { +#ifdef CONFIG_GUEST_INIT char path[PATH_MAX]; size_t size; int fd, ret; @@ -144,6 +147,9 @@ static int copy_init(const char *guestfs_name) close(fd); return 0; +#else + die("Guest init not compiled"); +#endif } static int copy_passwd(const char *guestfs_name) -- 2.1.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 kvmtool] Make static libc and guest-init functionality optional. 2015-09-11 14:40 ` [PATCH v2 " Dimitri John Ledkov @ 2015-09-15 17:20 ` Will Deacon 2015-09-16 8:08 ` Dimitri John Ledkov 0 siblings, 1 reply; 6+ messages in thread From: Will Deacon @ 2015-09-15 17:20 UTC (permalink / raw) To: Dimitri John Ledkov; +Cc: kvm@vger.kernel.org, Andre Przywara 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 kvmtool] Make static libc and guest-init functionality optional. 2015-09-15 17:20 ` Will Deacon @ 2015-09-16 8:08 ` Dimitri John Ledkov 0 siblings, 0 replies; 6+ messages in thread From: Dimitri John Ledkov @ 2015-09-16 8:08 UTC (permalink / raw) To: Will Deacon; +Cc: kvm@vger.kernel.org, Andre Przywara 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 -- Regards, Dimitri. 101 sleeps till Christmas https://clearlinux.org Open Source Technology Center Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-16 8:09 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-09-04 12:04 [PATCH kvmtool] Make static libc and guest-init functionality optional Dimitri John Ledkov 2015-09-11 12:47 ` Andre Przywara 2015-09-11 13:44 ` Dimitri John Ledkov 2015-09-11 14:40 ` [PATCH v2 " Dimitri John Ledkov 2015-09-15 17:20 ` Will Deacon 2015-09-16 8:08 ` Dimitri John Ledkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).