Add macOS and FreeBSD support to the qemu-bridge-helper. Differences when compared to linux version. 1) Does no drop privileges at the start of the process and run as root throughout the lifetime of the process there by increasing the risk incase of security vulnerability. 2) Does not support --use-vnet flag.
Signed-off-by: Nikhil Balachandra <nikhilbalachandra.in...@gmail.com> --- Makefile | 8 +++- Makefile.objs | 5 ++ configure | 5 ++ qemu-bridge-helper.c | 131 +++++++++++++++++++++++++++++++++------------------ 4 files changed, 102 insertions(+), 47 deletions(-) This patch makes qemu-bridge-helper work in macOS and FreeBSD platforms. The patch also contains changes in configure script and Makefile for building qemu-bridge-helper in these platforms. After applying the patch, running `$ make` or `$ make qemu-bridge-helper` should build the binary (see macOS caveat below). While the compilation process is straight-forward in FreeBSD, there is one caveat while building for the macOS. Unlike FreeBSD, macOS does not ship with net/if_bridgevar.h header file. This Header file however could be obtained from the Darwin/XNU repository[1] and can be copied somewhere in the include path before building. Eventhough macOS does not ship with the if_bridgevar.h header file[2], I expect the API to remain stable as this header file is similar to what is found in other BSDs. If this patch is decided to be included in the qemu, can experienced qemu developers please tell me how to go about having this header file in the include path such that it does not require manually downloading and copying the file[3]? This is also my first patch to qemu and opensource C codebase project. Please be critical while reviewing the code. [1] - https://github.com/apple/darwin-xnu/blob/master/bsd/net/if_bridgevar.h [2] - Adding to the complexity, this header file is also marked as private/internal. [3] - Couple of ideas: This could be automatically downloaded during the build or can be directly copied into the qemu source tree if license permits such a usage. Thanks and Regards, Nikhil diff --git a/Makefile b/Makefile index 727ef118f3..c581d91c6b 100644 --- a/Makefile +++ b/Makefile @@ -347,7 +347,10 @@ $(call set-vpath, $(SRC_PATH)) LIBS+=-lz $(LIBS_TOOLS) +# Build Helpers (currently only qemu-bridge-helper) for Linux, FreeBSD and macOS. HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF) +HELPERS-$(CONFIG_FREEBSD) = qemu-bridge-helper$(EXESUF) +HELPERS-$(CONFIG_DARWIN) = qemu-bridge-helper$(EXESUF) ifdef BUILD_DOCS DOCS=qemu-doc.html qemu-doc.txt qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8 @@ -429,7 +432,8 @@ dummy := $(call unnest-vars,, \ ui-obj-m \ audio-obj-y \ audio-obj-m \ - trace-obj-y) + trace-obj-y \ + tap-obj-y) include $(SRC_PATH)/tests/Makefile.include @@ -535,7 +539,7 @@ qemu-img$(EXESUF): qemu-img.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-o qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS) qemu-io$(EXESUF): qemu-io.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS) -qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o $(COMMON_LDADDS) +qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o $(tap-obj-y) $(COMMON_LDADDS) qemu-keymap$(EXESUF): qemu-keymap.o ui/input-keymap.o $(COMMON_LDADDS) diff --git a/Makefile.objs b/Makefile.objs index c6c9b8fc21..1f9ced33bf 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -86,6 +86,11 @@ qom-obj-y = qom/ io-obj-y = io/ +####################################################################### +# tap-obj-y is code used by qemu-bridge-helper + +tap-obj-y = net/ + ###################################################################### # Target independent part of system emulation. The long term path is to # suppress *all* target specific code in case of system emulation, i.e. a diff --git a/configure b/configure index a2301dd0dc..34b6c398c7 100755 --- a/configure +++ b/configure @@ -728,6 +728,7 @@ GNU/kFreeBSD) ;; FreeBSD) bsd="yes" + freebsd="yes" make="${MAKE-gmake}" audio_drv_list="oss" audio_possible_drivers="oss sdl pa" @@ -5985,6 +5986,10 @@ if test "$linux" = "yes" ; then echo "CONFIG_LINUX=y" >> $config_host_mak fi +if test "$freebsd" = "yes" ; then + echo "CONFIG_FREEBSD=y" >> $config_host_mak +fi + if test "$darwin" = "yes" ; then echo "CONFIG_DARWIN=y" >> $config_host_mak fi diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c index 5396fbfbb6..2ed1b5503f 100644 --- a/qemu-bridge-helper.c +++ b/qemu-bridge-helper.c @@ -15,23 +15,47 @@ #include "qemu/osdep.h" - #include <sys/ioctl.h> #include <sys/socket.h> #include <sys/un.h> -#include <sys/prctl.h> - #include <net/if.h> +#if defined(__linux__) +#include <sys/prctl.h> #include <linux/sockios.h> +#include "net/tap-linux.h" +#elif defined(__FreeBSD__) || defined(__APPLE__) +#include <sys/sockio.h> +#endif -#ifndef SIOCBRADDIF +#if defined(__linux__) && !defined(SIOCBRADDIF) #include <linux/if_bridge.h> #endif +#if defined(__FreeBSD__) +#include <net/ethernet.h> +#include <net/if_bridgevar.h> +#endif + +/* Unlike other BSDs, macOS does not ship with if_bridgevar.h header and has + * marked header file as private (internal to Apple). Since this header file + * is similar to what found in other BSDs, it should have a fairly stable API + * to be used in this file. + */ +#if defined(__APPLE__) +#ifndef PRIVATE +#define PRIVATE 1 +#include "osx/if_bridgevar.h" +#undef PRIVATE +#else +#include "osx/if_bridgevar.h" +#endif +#endif + #include "qemu/queue.h" +#include "qapi/error.h" -#include "net/tap-linux.h" +#include "net/tap_int.h" #ifdef CONFIG_LIBCAP #include <cap-ng.h> @@ -143,6 +167,13 @@ static int parse_acl_file(const char *filename, ACLList *acl_list) return 0; } +static void prep_ifreq(struct ifreq *ifr, const char *ifname) +{ + memset(ifr, 0, sizeof(*ifr)); + snprintf(ifr->ifr_name, IFNAMSIZ, "%s", ifname); +} + +#if defined(__linux__) static bool has_vnet_hdr(int fd) { unsigned int features = 0; @@ -158,11 +189,48 @@ static bool has_vnet_hdr(int fd) return true; } -static void prep_ifreq(struct ifreq *ifr, const char *ifname) +static int add_iface_to_bridge(const char *iface, const char *bridge, int ctlfd) { - memset(ifr, 0, sizeof(*ifr)); - snprintf(ifr->ifr_name, IFNAMSIZ, "%s", ifname); + int ifindex; + int ret; + struct ifreq ifr; + prep_ifreq(&ifr, bridge); + ifindex = if_nametoindex(iface); +#ifndef SIOCBRADDIF + unsigned long ifargs[4]; + ifargs[0] = BRCTL_ADD_IF; + ifargs[1] = ifindex; + ifargs[2] = 0; + ifargs[3] = 0; + ifr.ifr_data = (void *)ifargs; + ret = ioctl(ctlfd, SIOCDEVPRIVATE, &ifr); +#else + ifr.ifr_ifindex = ifindex; + ret = ioctl(ctlfd, SIOCBRADDIF, &ifr); +#endif + return ret; } +#else +static bool has_vnet_hdr(int fd) +{ + return false; +} +static int add_iface_to_bridge(const char *iface, const char *bridge, int ctlfd) +{ + struct ifbreq req; + memset(&req, 0, sizeof(req)); + strlcpy(req.ifbr_ifsname, iface, sizeof(req.ifbr_ifsname)); + + struct ifdrv ifd; + memset(&ifd, 0, sizeof(ifd)); + + strlcpy(ifd.ifd_name, bridge, sizeof(ifd.ifd_name)); + ifd.ifd_cmd = BRDGADD; + ifd.ifd_len = sizeof(req); + ifd.ifd_data = &req; + return ioctl(ctlfd, SIOCSDRVSPEC, &ifd); +} +#endif static int send_fd(int c, int fd) { @@ -215,10 +283,6 @@ static int drop_privileges(void) int main(int argc, char **argv) { struct ifreq ifr; -#ifndef SIOCBRADDIF - unsigned long ifargs[4]; -#endif - int ifindex; int fd = -1, ctlfd = -1, unixfd = -1; int use_vnet = 0; int mtu; @@ -310,30 +374,18 @@ int main(int argc, char **argv) goto cleanup; } + /* open the tap device */ - fd = open("/dev/net/tun", O_RDWR); + memset(&iface, '\0', sizeof(char) * IFNAMSIZ); + int vnet_supported = has_vnet_hdr(fd); + Error *err = NULL; + fd = tap_open(&iface[0], sizeof(iface), &vnet_supported, use_vnet, 0, &err); if (fd == -1) { - fprintf(stderr, "failed to open /dev/net/tun: %s\n", strerror(errno)); + fprintf(stderr, "%s\n", error_get_pretty(err)); ret = EXIT_FAILURE; goto cleanup; } - - /* request a tap device, disable PI, and add vnet header support if - * requested and it's available. */ - prep_ifreq(&ifr, "tap%d"); - ifr.ifr_flags = IFF_TAP|IFF_NO_PI; - if (use_vnet && has_vnet_hdr(fd)) { - ifr.ifr_flags |= IFF_VNET_HDR; - } - - if (ioctl(fd, TUNSETIFF, &ifr) == -1) { - fprintf(stderr, "failed to create tun device: %s\n", strerror(errno)); - ret = EXIT_FAILURE; - goto cleanup; - } - - /* save tap device name */ - snprintf(iface, sizeof(iface), "%s", ifr.ifr_name); + error_free(err); /* get the mtu of the bridge */ prep_ifreq(&ifr, bridge); @@ -357,6 +409,7 @@ int main(int argc, char **argv) goto cleanup; } +#if defined(__linux__) /* Linux uses the lowest enslaved MAC address as the MAC address of * the bridge. Set MAC address to a high value so that it doesn't * affect the MAC address of the bridge. @@ -374,22 +427,10 @@ int main(int argc, char **argv) ret = EXIT_FAILURE; goto cleanup; } +#endif /* add the interface to the bridge */ - prep_ifreq(&ifr, bridge); - ifindex = if_nametoindex(iface); -#ifndef SIOCBRADDIF - ifargs[0] = BRCTL_ADD_IF; - ifargs[1] = ifindex; - ifargs[2] = 0; - ifargs[3] = 0; - ifr.ifr_data = (void *)ifargs; - ret = ioctl(ctlfd, SIOCDEVPRIVATE, &ifr); -#else - ifr.ifr_ifindex = ifindex; - ret = ioctl(ctlfd, SIOCBRADDIF, &ifr); -#endif - if (ret == -1) { + if (add_iface_to_bridge(iface, bridge, ctlfd) == -1) { fprintf(stderr, "failed to add interface `%s' to bridge `%s': %s\n", iface, bridge, strerror(errno)); ret = EXIT_FAILURE; -- 2.16.3