Patchwork [v8,6/9] module: add configurable module whitelist

login
register
mail settings
Submitter Fam Zheng
Date Sept. 13, 2013, 8:38 a.m.
Message ID <1379061534-19171-7-git-send-email-famz@redhat.com>
Download mbox | patch
Permalink /patch/274690/
State New
Headers show

Comments

Fam Zheng - Sept. 13, 2013, 8:38 a.m.
Accept configure option "--enable-modules=L", to restrict qemu to only
load whitelisted modules.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 configure             | 12 +++++++++++-
 rules.mak             |  7 ++++++-
 scripts/create_config |  7 +++++++
 util/module.c         | 16 ++++++++++++++++
 4 files changed, 40 insertions(+), 2 deletions(-)
Daniel P. Berrange - Sept. 13, 2013, 9:03 a.m.
On Fri, Sep 13, 2013 at 04:38:51PM +0800, Fam Zheng wrote:
> Accept configure option "--enable-modules=L", to restrict qemu to only
> load whitelisted modules.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  configure             | 12 +++++++++++-
>  rules.mak             |  7 ++++++-
>  scripts/create_config |  7 +++++++
>  util/module.c         | 16 ++++++++++++++++
>  4 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 3043059..01e3665 100755
> --- a/configure
> +++ b/configure
> @@ -652,7 +652,9 @@ for opt do
>    ;;
>    --disable-debug-info)
>    ;;
> -  --enable-modules) modules="yes"
> +  --enable-modules|--enable-modules=*)
> +      modules="yes"
> +      module_list=`echo "$optarg" | sed -e 's/,/ /g'`
>    ;;
>    --cpu=*)
>    ;;
> @@ -1060,6 +1062,8 @@ echo "  --sysconfdir=PATH        install config in PATH$confsuffix"
>  echo "  --localstatedir=PATH     install local state in PATH (set at runtime on win32)"
>  echo "  --with-confsuffix=SUFFIX suffix for QEMU data inside datadir and sysconfdir [$confsuffix]"
>  echo "  --enable-modules         enable modules support"
> +echo "  --enable-modules=L       enable modules and provide a whitelist"
> +echo "                           Available modules: curl iscsi gluster ssh rbd"
>  echo "  --enable-debug-tcg       enable TCG debugging"
>  echo "  --disable-debug-tcg      disable TCG debugging (default)"
>  echo "  --enable-debug-info       enable debugging information (default)"
> @@ -3590,6 +3594,9 @@ if test "$slirp" = "yes" ; then
>      echo "smbd              $smbd"
>  fi
>  echo "module support    $modules"
> +if test -n "$module_list"; then
> +    echo "module whitelist  $module_list"
> +fi
>  echo "host CPU          $cpu"
>  echo "host big endian   $bigendian"
>  echo "target list       $target_list"
> @@ -3711,6 +3718,9 @@ echo "ARCH=$ARCH" >> $config_host_mak
>  echo "CONFIG_FINGERPRINT=$(date +%s$$$RANDOM)" >> $config_host_mak
>  if test "$modules" = "yes"; then
>    echo "CONFIG_MODULES=y" >> $config_host_mak
> +  if test -n "$module_list"; then
> +      echo "CONFIG_MODULE_WHITELIST=$module_list" >> $config_host_mak
> +  fi
>  fi
>  case "$cpu" in
>    arm|i386|x86_64|x32|ppc|aarch64)
> diff --git a/rules.mak b/rules.mak
> index 0670366..e5529da 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -165,13 +165,18 @@ $(if $(nested-dirs),
>    $(call unnest-vars-1))
>  endef
>  
> +is-whitelisted = $(if $(CONFIG_MODULE_WHITELIST),$(strip \
> +    $(filter $(CONFIG_MODULE_WHITELIST),$(basename $(notdir $1)))),\
> +	yes)
>  define add-modules
>  $(foreach o,$(filter %.o,$($1)),
>  	$(eval $(patsubst %.o,%.mo,$o): $o) \
>  	$(eval $(patsubst %.o,%.mo,$o)-objs := $o))
>  $(foreach o,$(filter %.mo,$($1)),$(eval \
>      $o: $($o-objs)))
> -$(eval modules-m += $(patsubst %.o,%.mo,$($1)))
> +$(eval t := $(patsubst %.o,%.mo,$($1)))
> +$(foreach o,$t,$(if $(call is-whitelisted,$o),$(eval \
> +	modules-m += $o)))
>  endef
>  
>  define unnest-vars
> diff --git a/scripts/create_config b/scripts/create_config
> index ecc5d4d..ab430c7 100755
> --- a/scripts/create_config
> +++ b/scripts/create_config
> @@ -37,6 +37,13 @@ case $line in
>   CONFIG_MODULES=*)
>      echo "#define CONFIG_MODULES \"${line#*=}\""
>      ;;
> + CONFIG_MODULE_WHITELIST=*)
> +    echo "#define CONFIG_MODULE_WHITELIST\\"
> +    for mod in ${line#*=}; do
> +      echo "    \"${mod}\",\\"
> +    done
> +    echo "    NULL"
> +    ;;
>   CONFIG_AUDIO_DRIVERS=*)
>      drivers=${line#*=}
>      echo "#define CONFIG_AUDIO_DRIVERS \\"
> diff --git a/util/module.c b/util/module.c
> index 9135c14..cb882f0 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -124,7 +124,14 @@ void module_load(module_load_type type)
>      const char *path;
>      char *fname = NULL;
>      DIR *dp;
> +#ifdef CONFIG_MODULE_WHITELIST
> +    const char **mp;
> +    const char *module_whitelist[] = {
> +        CONFIG_MODULE_WHITELIST
> +    };
> +#else
>      struct dirent *ep = NULL;
> +#endif
>  
>      if (!g_module_supported()) {
>          return;
> @@ -149,10 +156,19 @@ void module_load(module_load_type type)
>          fprintf(stderr, "Failed to open dir %s\n", path);
>          return;
>      }
> +#ifdef CONFIG_MODULE_WHITELIST
> +    for (mp = &module_whitelist[0]; *mp; mp++) {
> +        fname = g_strdup_printf("%s%s" HOST_DSOSUF, path, *mp);
> +        module_load_file(fname);
> +        g_free(fname);
> +    }
> +#else
>      for (ep = readdir(dp); ep; ep = readdir(dp)) {
>          fname = g_strdup_printf("%s%s", path, ep->d_name);
>          module_load_file(fname);
>          g_free(fname);
>      }
>  #endif

Having two code paths here is silly. Even if the user does not specify
a whitelist to configure, we know exactly what modules the QEMU source
tree contains. We *never* have any need to use readdir here.

Daniel
Fam Zheng - Sept. 13, 2013, 9:57 a.m.
On Fri, 09/13 10:03, Daniel P. Berrange wrote:
> On Fri, Sep 13, 2013 at 04:38:51PM +0800, Fam Zheng wrote:
> > Accept configure option "--enable-modules=L", to restrict qemu to only
> > load whitelisted modules.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  configure             | 12 +++++++++++-
> >  rules.mak             |  7 ++++++-
> >  scripts/create_config |  7 +++++++
> >  util/module.c         | 16 ++++++++++++++++
> >  4 files changed, 40 insertions(+), 2 deletions(-)
> > 
> > diff --git a/configure b/configure
> > index 3043059..01e3665 100755
> > --- a/configure
> > +++ b/configure
> > @@ -652,7 +652,9 @@ for opt do
> >    ;;
> >    --disable-debug-info)
> >    ;;
> > -  --enable-modules) modules="yes"
> > +  --enable-modules|--enable-modules=*)
> > +      modules="yes"
> > +      module_list=`echo "$optarg" | sed -e 's/,/ /g'`
> >    ;;
> >    --cpu=*)
> >    ;;
> > @@ -1060,6 +1062,8 @@ echo "  --sysconfdir=PATH        install config in PATH$confsuffix"
> >  echo "  --localstatedir=PATH     install local state in PATH (set at runtime on win32)"
> >  echo "  --with-confsuffix=SUFFIX suffix for QEMU data inside datadir and sysconfdir [$confsuffix]"
> >  echo "  --enable-modules         enable modules support"
> > +echo "  --enable-modules=L       enable modules and provide a whitelist"
> > +echo "                           Available modules: curl iscsi gluster ssh rbd"
> >  echo "  --enable-debug-tcg       enable TCG debugging"
> >  echo "  --disable-debug-tcg      disable TCG debugging (default)"
> >  echo "  --enable-debug-info       enable debugging information (default)"
> > @@ -3590,6 +3594,9 @@ if test "$slirp" = "yes" ; then
> >      echo "smbd              $smbd"
> >  fi
> >  echo "module support    $modules"
> > +if test -n "$module_list"; then
> > +    echo "module whitelist  $module_list"
> > +fi
> >  echo "host CPU          $cpu"
> >  echo "host big endian   $bigendian"
> >  echo "target list       $target_list"
> > @@ -3711,6 +3718,9 @@ echo "ARCH=$ARCH" >> $config_host_mak
> >  echo "CONFIG_FINGERPRINT=$(date +%s$$$RANDOM)" >> $config_host_mak
> >  if test "$modules" = "yes"; then
> >    echo "CONFIG_MODULES=y" >> $config_host_mak
> > +  if test -n "$module_list"; then
> > +      echo "CONFIG_MODULE_WHITELIST=$module_list" >> $config_host_mak
> > +  fi
> >  fi
> >  case "$cpu" in
> >    arm|i386|x86_64|x32|ppc|aarch64)
> > diff --git a/rules.mak b/rules.mak
> > index 0670366..e5529da 100644
> > --- a/rules.mak
> > +++ b/rules.mak
> > @@ -165,13 +165,18 @@ $(if $(nested-dirs),
> >    $(call unnest-vars-1))
> >  endef
> >  
> > +is-whitelisted = $(if $(CONFIG_MODULE_WHITELIST),$(strip \
> > +    $(filter $(CONFIG_MODULE_WHITELIST),$(basename $(notdir $1)))),\
> > +	yes)
> >  define add-modules
> >  $(foreach o,$(filter %.o,$($1)),
> >  	$(eval $(patsubst %.o,%.mo,$o): $o) \
> >  	$(eval $(patsubst %.o,%.mo,$o)-objs := $o))
> >  $(foreach o,$(filter %.mo,$($1)),$(eval \
> >      $o: $($o-objs)))
> > -$(eval modules-m += $(patsubst %.o,%.mo,$($1)))
> > +$(eval t := $(patsubst %.o,%.mo,$($1)))
> > +$(foreach o,$t,$(if $(call is-whitelisted,$o),$(eval \
> > +	modules-m += $o)))
> >  endef
> >  
> >  define unnest-vars
> > diff --git a/scripts/create_config b/scripts/create_config
> > index ecc5d4d..ab430c7 100755
> > --- a/scripts/create_config
> > +++ b/scripts/create_config
> > @@ -37,6 +37,13 @@ case $line in
> >   CONFIG_MODULES=*)
> >      echo "#define CONFIG_MODULES \"${line#*=}\""
> >      ;;
> > + CONFIG_MODULE_WHITELIST=*)
> > +    echo "#define CONFIG_MODULE_WHITELIST\\"
> > +    for mod in ${line#*=}; do
> > +      echo "    \"${mod}\",\\"
> > +    done
> > +    echo "    NULL"
> > +    ;;
> >   CONFIG_AUDIO_DRIVERS=*)
> >      drivers=${line#*=}
> >      echo "#define CONFIG_AUDIO_DRIVERS \\"
> > diff --git a/util/module.c b/util/module.c
> > index 9135c14..cb882f0 100644
> > --- a/util/module.c
> > +++ b/util/module.c
> > @@ -124,7 +124,14 @@ void module_load(module_load_type type)
> >      const char *path;
> >      char *fname = NULL;
> >      DIR *dp;
> > +#ifdef CONFIG_MODULE_WHITELIST
> > +    const char **mp;
> > +    const char *module_whitelist[] = {
> > +        CONFIG_MODULE_WHITELIST
> > +    };
> > +#else
> >      struct dirent *ep = NULL;
> > +#endif
> >  
> >      if (!g_module_supported()) {
> >          return;
> > @@ -149,10 +156,19 @@ void module_load(module_load_type type)
> >          fprintf(stderr, "Failed to open dir %s\n", path);
> >          return;
> >      }
> > +#ifdef CONFIG_MODULE_WHITELIST
> > +    for (mp = &module_whitelist[0]; *mp; mp++) {
> > +        fname = g_strdup_printf("%s%s" HOST_DSOSUF, path, *mp);
> > +        module_load_file(fname);
> > +        g_free(fname);
> > +    }
> > +#else
> >      for (ep = readdir(dp); ep; ep = readdir(dp)) {
> >          fname = g_strdup_printf("%s%s", path, ep->d_name);
> >          module_load_file(fname);
> >          g_free(fname);
> >      }
> >  #endif
> 
> Having two code paths here is silly. Even if the user does not specify
> a whitelist to configure, we know exactly what modules the QEMU source
> tree contains. We *never* have any need to use readdir here.
> 
Yes, it's a good point. I'll send another revision to fix this.

Fam

Patch

diff --git a/configure b/configure
index 3043059..01e3665 100755
--- a/configure
+++ b/configure
@@ -652,7 +652,9 @@  for opt do
   ;;
   --disable-debug-info)
   ;;
-  --enable-modules) modules="yes"
+  --enable-modules|--enable-modules=*)
+      modules="yes"
+      module_list=`echo "$optarg" | sed -e 's/,/ /g'`
   ;;
   --cpu=*)
   ;;
@@ -1060,6 +1062,8 @@  echo "  --sysconfdir=PATH        install config in PATH$confsuffix"
 echo "  --localstatedir=PATH     install local state in PATH (set at runtime on win32)"
 echo "  --with-confsuffix=SUFFIX suffix for QEMU data inside datadir and sysconfdir [$confsuffix]"
 echo "  --enable-modules         enable modules support"
+echo "  --enable-modules=L       enable modules and provide a whitelist"
+echo "                           Available modules: curl iscsi gluster ssh rbd"
 echo "  --enable-debug-tcg       enable TCG debugging"
 echo "  --disable-debug-tcg      disable TCG debugging (default)"
 echo "  --enable-debug-info       enable debugging information (default)"
@@ -3590,6 +3594,9 @@  if test "$slirp" = "yes" ; then
     echo "smbd              $smbd"
 fi
 echo "module support    $modules"
+if test -n "$module_list"; then
+    echo "module whitelist  $module_list"
+fi
 echo "host CPU          $cpu"
 echo "host big endian   $bigendian"
 echo "target list       $target_list"
@@ -3711,6 +3718,9 @@  echo "ARCH=$ARCH" >> $config_host_mak
 echo "CONFIG_FINGERPRINT=$(date +%s$$$RANDOM)" >> $config_host_mak
 if test "$modules" = "yes"; then
   echo "CONFIG_MODULES=y" >> $config_host_mak
+  if test -n "$module_list"; then
+      echo "CONFIG_MODULE_WHITELIST=$module_list" >> $config_host_mak
+  fi
 fi
 case "$cpu" in
   arm|i386|x86_64|x32|ppc|aarch64)
diff --git a/rules.mak b/rules.mak
index 0670366..e5529da 100644
--- a/rules.mak
+++ b/rules.mak
@@ -165,13 +165,18 @@  $(if $(nested-dirs),
   $(call unnest-vars-1))
 endef
 
+is-whitelisted = $(if $(CONFIG_MODULE_WHITELIST),$(strip \
+    $(filter $(CONFIG_MODULE_WHITELIST),$(basename $(notdir $1)))),\
+	yes)
 define add-modules
 $(foreach o,$(filter %.o,$($1)),
 	$(eval $(patsubst %.o,%.mo,$o): $o) \
 	$(eval $(patsubst %.o,%.mo,$o)-objs := $o))
 $(foreach o,$(filter %.mo,$($1)),$(eval \
     $o: $($o-objs)))
-$(eval modules-m += $(patsubst %.o,%.mo,$($1)))
+$(eval t := $(patsubst %.o,%.mo,$($1)))
+$(foreach o,$t,$(if $(call is-whitelisted,$o),$(eval \
+	modules-m += $o)))
 endef
 
 define unnest-vars
diff --git a/scripts/create_config b/scripts/create_config
index ecc5d4d..ab430c7 100755
--- a/scripts/create_config
+++ b/scripts/create_config
@@ -37,6 +37,13 @@  case $line in
  CONFIG_MODULES=*)
     echo "#define CONFIG_MODULES \"${line#*=}\""
     ;;
+ CONFIG_MODULE_WHITELIST=*)
+    echo "#define CONFIG_MODULE_WHITELIST\\"
+    for mod in ${line#*=}; do
+      echo "    \"${mod}\",\\"
+    done
+    echo "    NULL"
+    ;;
  CONFIG_AUDIO_DRIVERS=*)
     drivers=${line#*=}
     echo "#define CONFIG_AUDIO_DRIVERS \\"
diff --git a/util/module.c b/util/module.c
index 9135c14..cb882f0 100644
--- a/util/module.c
+++ b/util/module.c
@@ -124,7 +124,14 @@  void module_load(module_load_type type)
     const char *path;
     char *fname = NULL;
     DIR *dp;
+#ifdef CONFIG_MODULE_WHITELIST
+    const char **mp;
+    const char *module_whitelist[] = {
+        CONFIG_MODULE_WHITELIST
+    };
+#else
     struct dirent *ep = NULL;
+#endif
 
     if (!g_module_supported()) {
         return;
@@ -149,10 +156,19 @@  void module_load(module_load_type type)
         fprintf(stderr, "Failed to open dir %s\n", path);
         return;
     }
+#ifdef CONFIG_MODULE_WHITELIST
+    for (mp = &module_whitelist[0]; *mp; mp++) {
+        fname = g_strdup_printf("%s%s" HOST_DSOSUF, path, *mp);
+        module_load_file(fname);
+        g_free(fname);
+    }
+#else
     for (ep = readdir(dp); ep; ep = readdir(dp)) {
         fname = g_strdup_printf("%s%s", path, ep->d_name);
         module_load_file(fname);
         g_free(fname);
     }
 #endif
+
+#endif
 }