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 <f...@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
