* [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).