On Fri, May 13, 2016 at 11:23:30PM +0200, Jiri Denemark wrote:
The list of CPU models is freed using virStringFreeList, which expects the list to by NULL-terminated. This bug could theoretically crash libvirtd in remoteDispatchConnectGetCPUModelNames, but luckily enough we never return more than REMOTE_CONNECT_CPU_MODELS_MAX models in the list.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu.c | 2 +- src/cpu/cpu_ppc64.c | 7 +++++++ src/cpu/cpu_x86.c | 7 +++++++ 3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 69055e2..2f2b658 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -716,7 +716,7 @@ cpuModelIsAllowed(const char *model, * cpuGetModels: * * @arch: CPU architecture - * @models: where to store the list of supported models + * @models: where to store the NULL-terminated list of supported models * * Fetches all CPU models supported by libvirt on @archName. * diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 1aed984..c784a55 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -897,6 +897,13 @@ ppc64DriverGetModels(char ***models) model = model->next; }
+ if (models) { + /* Make sure models is NULL-terminated */ + if (VIR_EXPAND_N(*models, nmodels, 1) < 0) + goto error; + nmodels--; + } +
This fixes the function only in case that there is no failure. If the VIR_EXPAND_N() or VIR_APPEND_ELEMENT() fails we will hit segfault in virStringFreeList(). We need to preallocate the whole array with nmodels + 1. Pavel
cleanup: ppc64MapFree(map);
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index c6aacc1..6ee7ff9 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -2230,6 +2230,13 @@ x86GetModels(char ***models) model = model->next; }
+ if (models) { + /* Make sure models is NULL-terminated */ + if (VIR_EXPAND_N(*models, nmodels, 1) < 0) + goto error; + nmodels--; + } + return nmodels;
error: -- 2.8.2
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list