[Qemu-devel] [PATCH] build: set up capabilities on qemu-bridge-helper

[Qemu-devel] [PATCH] build: set up capabilities on qemu-bridge-helper

[Avi Kivity]

Out-of-the-box, 'make install' sets up an unusable qemu-bridge-helper since
it doesn't have the required capabilities.

Fix by adding them.

Note: this may break installing as non-root.  This is actually the right
thing to do, since not setting up the capability would result in a broken
setup.  Perhaps we need a configure flag to disable helpers.

Signed-off-by: Avi Kivity <avi@cloudius-systems.com>
---
 Makefile  | 3 +++
 rules.mak | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/Makefile b/Makefile
index b15003f..af7748c 100644
--- a/Makefile
+++ b/Makefile
@@ -188,6 +188,7 @@ qemu-img$(EXESUF): qemu-img.o $(block-obj-y) libqemuutil.a libqemustub.a
 qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) libqemuutil.a libqemustub.a
 qemu-io$(EXESUF): qemu-io.o $(block-obj-y) libqemuutil.a libqemustub.a
 
+qemu-bridge-helper$(EXESUF).capabilities = cap_net_admin
 qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
 
 fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o libqemuutil.a libqemustub.a
@@ -345,6 +346,8 @@ endif
 ifneq ($(HELPERS-y),)
 	$(INSTALL_DIR) "$(DESTDIR)$(libexecdir)"
 	$(INSTALL_PROG) $(STRIP_OPT) $(HELPERS-y) "$(DESTDIR)$(libexecdir)"
+	$(SETCAP) $(foreach helper, $(HELPERS-y), \
+	    $($(helper).capabilities)+pe "$(DESTDIR)$(libexecdir)"/$(helper))
 endif
 ifneq ($(BLOBS),)
 	set -e; for x in $(BLOBS); do \
diff --git a/rules.mak b/rules.mak
index 49edb9b..9194c79 100644
--- a/rules.mak
+++ b/rules.mak
@@ -177,3 +177,5 @@ $(shell mkdir -p $(sort $(foreach var,$(nested-vars),$(dir $($(var))))))
 $(foreach var,$(nested-vars), $(eval \
   -include $(addsuffix *.d, $(sort $(dir $($(var)))))))
 endef
+
+SETCAP = setcap
\ No newline at end of file
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] build: set up capabilities on qemu-bridge-helper

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 841 bytes --]

On 11/12/2013 04:10 AM, Avi Kivity wrote:
> Out-of-the-box, 'make install' sets up an unusable qemu-bridge-helper since
> it doesn't have the required capabilities.
> 
> Fix by adding them.
> 
> Note: this may break installing as non-root.  This is actually the right
> thing to do, since not setting up the capability would result in a broken
> setup.  Perhaps we need a configure flag to disable helpers.
> 

> +++ b/rules.mak
> @@ -177,3 +177,5 @@ $(shell mkdir -p $(sort $(foreach var,$(nested-vars),$(dir $($(var))))))
>  $(foreach var,$(nested-vars), $(eval \
>    -include $(addsuffix *.d, $(sort $(dir $($(var)))))))
>  endef
> +
> +SETCAP = setcap
> \ No newline at end of file

Is this intentional?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH] build: set up capabilities on qemu-bridge-helper

[Avi Kivity]

[-- Attachment #1: Type: text/plain, Size: 932 bytes --]

On Nov 12, 2013 9:23 PM, "Eric Blake" <eblake@redhat.com> wrote:
>
> On 11/12/2013 04:10 AM, Avi Kivity wrote:
> > Out-of-the-box, 'make install' sets up an unusable qemu-bridge-helper
since
> > it doesn't have the required capabilities.
> >
> > Fix by adding them.
> >
> > Note: this may break installing as non-root.  This is actually the right
> > thing to do, since not setting up the capability would result in a
broken
> > setup.  Perhaps we need a configure flag to disable helpers.
> >
>
> > +++ b/rules.mak
> > @@ -177,3 +177,5 @@ $(shell mkdir -p $(sort $(foreach
var,$(nested-vars),$(dir $($(var))))))
> >  $(foreach var,$(nested-vars), $(eval \
> >    -include $(addsuffix *.d, $(sort $(dir $($(var)))))))
> >  endef
> > +
> > +SETCAP = setcap
> > \ No newline at end of file
>
> Is this intentional?

No.

>
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>

[-- Attachment #2: Type: text/html, Size: 1358 bytes --]

Re: [Qemu-devel] [PATCH] build: set up capabilities on qemu-bridge-helper

[Stefan Hajnoczi]

On Tue, Nov 12, 2013 at 01:10:24PM +0200, Avi Kivity wrote:
> Out-of-the-box, 'make install' sets up an unusable qemu-bridge-helper since
> it doesn't have the required capabilities.
> 
> Fix by adding them.

Up until now, downstreams had to make the bridge helper executable
setuid, add the cap_net_admin capability, or they did nothing and it was
broken ;-).  CCing downstream package maintainers in case they have any
comments on this patch.

> Note: this may break installing as non-root.  This is actually the right
> thing to do, since not setting up the capability would result in a broken
> setup.  Perhaps we need a configure flag to disable helpers.

Users who have been successfully installing QEMU would be upset if it
suddenly starts failing after this patch.  The bridge helper is a niche
feature that shouldn't cause a regression for the majority of users who
don't care about it.

If we're installing non-root then the bridge helper simply shouldn't be
installed.

> Signed-off-by: Avi Kivity <avi@cloudius-systems.com>
> ---
>  Makefile  | 3 +++
>  rules.mak | 2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index b15003f..af7748c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -188,6 +188,7 @@ qemu-img$(EXESUF): qemu-img.o $(block-obj-y) libqemuutil.a libqemustub.a
>  qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) libqemuutil.a libqemustub.a
>  qemu-io$(EXESUF): qemu-io.o $(block-obj-y) libqemuutil.a libqemustub.a
>  
> +qemu-bridge-helper$(EXESUF).capabilities = cap_net_admin
>  qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
>  
>  fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o libqemuutil.a libqemustub.a
> @@ -345,6 +346,8 @@ endif
>  ifneq ($(HELPERS-y),)
>  	$(INSTALL_DIR) "$(DESTDIR)$(libexecdir)"
>  	$(INSTALL_PROG) $(STRIP_OPT) $(HELPERS-y) "$(DESTDIR)$(libexecdir)"
> +	$(SETCAP) $(foreach helper, $(HELPERS-y), \
> +	    $($(helper).capabilities)+pe "$(DESTDIR)$(libexecdir)"/$(helper))
>  endif
>  ifneq ($(BLOBS),)
>  	set -e; for x in $(BLOBS); do \
> diff --git a/rules.mak b/rules.mak
> index 49edb9b..9194c79 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -177,3 +177,5 @@ $(shell mkdir -p $(sort $(foreach var,$(nested-vars),$(dir $($(var))))))
>  $(foreach var,$(nested-vars), $(eval \
>    -include $(addsuffix *.d, $(sort $(dir $($(var)))))))
>  endef
> +
> +SETCAP = setcap
> \ No newline at end of file
> -- 
> 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH] build: set up capabilities on qemu-bridge-helper

[Avi Kivity]

On 11/14/2013 03:29 PM, Stefan Hajnoczi wrote:
> On Tue, Nov 12, 2013 at 01:10:24PM +0200, Avi Kivity wrote:
>> Out-of-the-box, 'make install' sets up an unusable qemu-bridge-helper since
>> it doesn't have the required capabilities.
>>
>> Fix by adding them.
> Up until now, downstreams had to make the bridge helper executable
> setuid, add the cap_net_admin capability, or they did nothing and it was
> broken ;-).  CCing downstream package maintainers in case they have any
> comments on this patch.

And it was, indeed, broken.

>
>> Note: this may break installing as non-root.  This is actually the right
>> thing to do, since not setting up the capability would result in a broken
>> setup.  Perhaps we need a configure flag to disable helpers.
> Users who have been successfully installing QEMU would be upset if it
> suddenly starts failing after this patch.  The bridge helper is a niche
> feature that shouldn't cause a regression for the majority of users who
> don't care about it.
>
> If we're installing non-root then the bridge helper simply shouldn't be
> installed.

Or maybe installed without the capabilities?  This way if the user 
invokes qemu with sudo, it still works as before.