On Tue, Sep 20, 2011 at 12:11:32PM -0600, Eric Blake wrote:
> Bug introduced in commit 675464b. On an OOM, this would try to
> dereference a char* and free the contents as a pointer, which is
> doomed to failure.
>
> Adding a syntax check will prevent mistakes like this in the future.
>
> * cfg.mk (sc_prohibit_internal_functions): New syntax check.
> (exclude_file_name_regexp--sc_prohibit_internal_functions): Add
> exemptions.
> * daemon/remote.c (remoteRelayDomainEventIOError)
> (remoteRelayDomainEventIOErrorReason)
> (remoteRelayDomainEventGraphics, remoteRelayDomainEventBlockJob):
> Use correct free function.
> ---
> cfg.mk | 11 ++++++++++-
> daemon/remote.c | 28 ++++++++++++++--------------
> 2 files changed, 24 insertions(+), 15 deletions(-)
>
> diff --git a/cfg.mk b/cfg.mk
> index 95c5eff..9f4aa3e 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -212,7 +212,7 @@ useless_free_options = \
> # y virDomainWatchdogDefFree
> # n virDrvNodeGetCellsFreeMemory (returns int)
> # n virDrvNodeGetFreeMemory (returns long long)
> -# n virFree (dereferences param)
> +# n virFree - dereferences param
> # n virFreeError
> # n virHashFree (takes 2 args)
> # y virInterfaceDefFree
> @@ -306,6 +306,12 @@ sc_flags_usage:
> halt='flags should be unsigned' \
> $(_sc_search_regexp)
>
> +# Avoid functions that should only be called via macro counterparts.
> +sc_prohibit_internal_functions:
> + @prohibit='vir(Free|AllocN?|ReallocN|File(Close|Fclose|Fdopen)) *\(' \
> + halt='use VIR_ macros instead of internal functions' \
> + $(_sc_search_regexp)
> +
> # Avoid functions that can lead to double-close bugs.
> sc_prohibit_close:
> @prohibit='([^>.]|^)\<[fp]?close *\(' \
> @@ -706,6 +712,9 @@ exclude_file_name_regexp--sc_prohibit_fork_wrappers = \
>
> exclude_file_name_regexp--sc_prohibit_gethostname = ^src/util/util\.c$$
>
> +exclude_file_name_regexp--sc_prohibit_internal_functions = \
> + ^src/(util/(memory|util|virfile)\.[hc]|esx/esx_vi\.c)$$
> +
> exclude_file_name_regexp--sc_prohibit_newline_at_end_of_diagnostic = \
> ^src/rpc/gendispatch\.pl$$
>
> diff --git a/daemon/remote.c b/daemon/remote.c
> index 74e759a..245d41c 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -250,8 +250,8 @@ static int remoteRelayDomainEventIOError(virConnectPtr conn ATTRIBUTE_UNUSED,
> return 0;
> mem_error:
> virReportOOMError();
> - virFree(data.srcPath);
> - virFree(data.devAlias);
> + VIR_FREE(data.srcPath);
> + VIR_FREE(data.devAlias);
> return -1;
> }
>
> @@ -296,9 +296,9 @@ static int remoteRelayDomainEventIOErrorReason(virConnectPtr conn ATTRIBUTE_UNUS
>
> mem_error:
> virReportOOMError();
> - virFree(data.srcPath);
> - virFree(data.devAlias);
> - virFree(data.reason);
> + VIR_FREE(data.srcPath);
> + VIR_FREE(data.devAlias);
> + VIR_FREE(data.reason);
> return -1;
> }
>
> @@ -374,17 +374,17 @@ static int remoteRelayDomainEventGraphics(virConnectPtr conn ATTRIBUTE_UNUSED,
>
> mem_error:
> virReportOOMError();
> - virFree(data.authScheme);
> - virFree(data.local.node);
> - virFree(data.local.service);
> - virFree(data.remote.node);
> - virFree(data.remote.service);
> + VIR_FREE(data.authScheme);
> + VIR_FREE(data.local.node);
> + VIR_FREE(data.local.service);
> + VIR_FREE(data.remote.node);
> + VIR_FREE(data.remote.service);
> if (data.subject.subject_val != NULL) {
> for (i = 0 ; i < data.subject.subject_len ; i++) {
> - virFree(data.subject.subject_val[i].type);
> - virFree(data.subject.subject_val[i].name);
> + VIR_FREE(data.subject.subject_val[i].type);
> + VIR_FREE(data.subject.subject_val[i].name);
> }
> - virFree(data.subject.subject_val);
> + VIR_FREE(data.subject.subject_val);
> }
> return -1;
> }
> @@ -422,7 +422,7 @@ static int remoteRelayDomainEventBlockJob(virConnectPtr conn ATTRIBUTE_UNUSED,
>
> mem_error:
> virReportOOMError();
> - virFree(data.path);
> + VIR_FREE(data.path);
> return -1;
> }
>
Gahhh, I got it wrong, I wanted to use VIR_FREE and failed to notice
my mistake ...
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list