| Message ID | 1379061534-19171-7-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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 }
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(-)