* [Qemu-devel] [PATCH 0/3] ppc: fix memory leaks @ 2017-06-06 15:22 Greg Kurz 2017-06-06 15:22 ` [Qemu-devel] [PATCH 1/3] target/ppc: pass const pointer to kvmppc_is_mem_backend_page_size_ok() Greg Kurz ` (3 more replies) 0 siblings, 4 replies; 17+ messages in thread From: Greg Kurz @ 2017-06-06 15:22 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Thomas Huth, qemu-ppc, qemu-stable, David Gibson Coverity just reported a memory leak introduced by this commit (QEMU 2.9): commit df58713396f8b2deb923e39c00b10744c5c63909 Author: Thomas Huth <thuth@redhat.com> Date: Wed Feb 15 10:21:44 2017 +0100 hw/ppc/spapr: Check for valid page size when hot plugging memory It boils down to the fact that object_property_get_str() returns a string allocated with g_strdup(), which must be deallocated with g_free() at some point. -- Greg --- Greg Kurz (3): target/ppc: pass const pointer to kvmppc_is_mem_backend_page_size_ok() target/ppc: fix memory leak in kvmppc_is_mem_backend_page_size_ok() spapr: fix memory leak in spapr_memory_pre_plug() hw/ppc/spapr.c | 5 ++++- target/ppc/kvm.c | 5 +++-- target/ppc/kvm_ppc.h | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 1/3] target/ppc: pass const pointer to kvmppc_is_mem_backend_page_size_ok() 2017-06-06 15:22 [Qemu-devel] [PATCH 0/3] ppc: fix memory leaks Greg Kurz @ 2017-06-06 15:22 ` Greg Kurz 2017-06-06 15:33 ` Philippe Mathieu-Daudé ` (3 more replies) 2017-06-06 15:22 ` [Qemu-devel] [PATCH 2/3] target/ppc: fix memory leak in kvmppc_is_mem_backend_page_size_ok() Greg Kurz ` (2 subsequent siblings) 3 siblings, 4 replies; 17+ messages in thread From: Greg Kurz @ 2017-06-06 15:22 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Thomas Huth, qemu-ppc, qemu-stable, David Gibson Signed-off-by: Greg Kurz <groug@kaod.org> --- target/ppc/kvm.c | 4 ++-- target/ppc/kvm_ppc.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 51249ce79e55..88817620766c 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -478,7 +478,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu) } } -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path) +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) { Object *mem_obj = object_resolve_path(obj_path, NULL); char *mempath = object_property_get_str(mem_obj, "mem-path", NULL); @@ -499,7 +499,7 @@ static inline void kvm_fixup_page_sizes(PowerPCCPU *cpu) { } -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path) +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) { return true; } diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h index f48243d13ffc..fd72d6b05a98 100644 --- a/target/ppc/kvm_ppc.h +++ b/target/ppc/kvm_ppc.h @@ -64,7 +64,7 @@ int kvmppc_enable_hwrng(void); int kvmppc_put_books_sregs(PowerPCCPU *cpu); PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void); -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path); +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path); #else ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] target/ppc: pass const pointer to kvmppc_is_mem_backend_page_size_ok() 2017-06-06 15:22 ` [Qemu-devel] [PATCH 1/3] target/ppc: pass const pointer to kvmppc_is_mem_backend_page_size_ok() Greg Kurz @ 2017-06-06 15:33 ` Philippe Mathieu-Daudé 2017-06-06 15:34 ` Thomas Huth ` (2 subsequent siblings) 3 siblings, 0 replies; 17+ messages in thread From: Philippe Mathieu-Daudé @ 2017-06-06 15:33 UTC (permalink / raw) To: Greg Kurz, qemu-devel Cc: Peter Maydell, Thomas Huth, qemu-ppc, qemu-stable, David Gibson On 06/06/2017 12:22 PM, Greg Kurz wrote: > Signed-off-by: Greg Kurz <groug@kaod.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > target/ppc/kvm.c | 4 ++-- > target/ppc/kvm_ppc.h | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 51249ce79e55..88817620766c 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -478,7 +478,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu) > } > } > > -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path) > +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) > { > Object *mem_obj = object_resolve_path(obj_path, NULL); > char *mempath = object_property_get_str(mem_obj, "mem-path", NULL); > @@ -499,7 +499,7 @@ static inline void kvm_fixup_page_sizes(PowerPCCPU *cpu) > { > } > > -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path) > +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) > { > return true; > } > diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h > index f48243d13ffc..fd72d6b05a98 100644 > --- a/target/ppc/kvm_ppc.h > +++ b/target/ppc/kvm_ppc.h > @@ -64,7 +64,7 @@ int kvmppc_enable_hwrng(void); > int kvmppc_put_books_sregs(PowerPCCPU *cpu); > PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void); > > -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path); > +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path); > > #else > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] target/ppc: pass const pointer to kvmppc_is_mem_backend_page_size_ok() 2017-06-06 15:22 ` [Qemu-devel] [PATCH 1/3] target/ppc: pass const pointer to kvmppc_is_mem_backend_page_size_ok() Greg Kurz 2017-06-06 15:33 ` Philippe Mathieu-Daudé @ 2017-06-06 15:34 ` Thomas Huth 2017-06-06 15:46 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz 2017-06-06 16:12 ` [Qemu-devel] [PATCH v2] target/ppc: pass const string " Greg Kurz 3 siblings, 0 replies; 17+ messages in thread From: Thomas Huth @ 2017-06-06 15:34 UTC (permalink / raw) To: Greg Kurz, qemu-devel Cc: Peter Maydell, qemu-ppc, qemu-stable, David Gibson, Philippe Mathieu-Daudé A short patch description for saying why this is appropriate would be nice :-) On 06.06.2017 17:22, Greg Kurz wrote: > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > target/ppc/kvm.c | 4 ++-- > target/ppc/kvm_ppc.h | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 51249ce79e55..88817620766c 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -478,7 +478,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu) > } > } > > -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path) > +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) > { > Object *mem_obj = object_resolve_path(obj_path, NULL); > char *mempath = object_property_get_str(mem_obj, "mem-path", NULL); > @@ -499,7 +499,7 @@ static inline void kvm_fixup_page_sizes(PowerPCCPU *cpu) > { > } > > -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path) > +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) > { > return true; > } > diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h > index f48243d13ffc..fd72d6b05a98 100644 > --- a/target/ppc/kvm_ppc.h > +++ b/target/ppc/kvm_ppc.h > @@ -64,7 +64,7 @@ int kvmppc_enable_hwrng(void); > int kvmppc_put_books_sregs(PowerPCCPU *cpu); > PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void); > > -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path); > +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path); > > #else I think you could also add the "const" to the "static inline bool kvmppc_is_mem_backend_page_size_ok()" later in that header. Thomas ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/3] target/ppc: pass const pointer to kvmppc_is_mem_backend_page_size_ok() 2017-06-06 15:22 ` [Qemu-devel] [PATCH 1/3] target/ppc: pass const pointer to kvmppc_is_mem_backend_page_size_ok() Greg Kurz 2017-06-06 15:33 ` Philippe Mathieu-Daudé 2017-06-06 15:34 ` Thomas Huth @ 2017-06-06 15:46 ` Greg Kurz 2017-06-06 16:12 ` [Qemu-devel] [PATCH v2] target/ppc: pass const string " Greg Kurz 3 siblings, 0 replies; 17+ messages in thread From: Greg Kurz @ 2017-06-06 15:46 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Thomas Huth, qemu-ppc, qemu-stable, David Gibson [-- Attachment #1: Type: text/plain, Size: 1575 bytes --] On Tue, 06 Jun 2017 17:22:48 +0200 Greg Kurz <groug@kaod.org> wrote: > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > target/ppc/kvm.c | 4 ++-- > target/ppc/kvm_ppc.h | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 51249ce79e55..88817620766c 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -478,7 +478,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu) > } > } > > -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path) > +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) > { > Object *mem_obj = object_resolve_path(obj_path, NULL); > char *mempath = object_property_get_str(mem_obj, "mem-path", NULL); > @@ -499,7 +499,7 @@ static inline void kvm_fixup_page_sizes(PowerPCCPU *cpu) > { > } > > -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path) > +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) > { > return true; > } > diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h > index f48243d13ffc..fd72d6b05a98 100644 > --- a/target/ppc/kvm_ppc.h > +++ b/target/ppc/kvm_ppc.h > @@ -64,7 +64,7 @@ int kvmppc_enable_hwrng(void); > int kvmppc_put_books_sregs(PowerPCCPU *cpu); > PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void); > > -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path); > +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path); > > #else > Dumb me forgot to patch the inline stub... Will send a v2 for this patch only. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v2] target/ppc: pass const string to kvmppc_is_mem_backend_page_size_ok() 2017-06-06 15:22 ` [Qemu-devel] [PATCH 1/3] target/ppc: pass const pointer to kvmppc_is_mem_backend_page_size_ok() Greg Kurz ` (2 preceding siblings ...) 2017-06-06 15:46 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz @ 2017-06-06 16:12 ` Greg Kurz 2017-06-06 16:34 ` Thomas Huth 3 siblings, 1 reply; 17+ messages in thread From: Greg Kurz @ 2017-06-06 16:12 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Thomas Huth, qemu-ppc, qemu-stable, David Gibson This function has three implementations. Two are stubs that do nothing and the third one only passes the obj_path argument to: Object *object_resolve_path(const char *path, bool *ambiguous); Signed-off-by: Greg Kurz <groug@kaod.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- v2: - also patch inline stub - renamed patch and added changelog --- target/ppc/kvm.c | 4 ++-- target/ppc/kvm_ppc.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 51249ce79e55..88817620766c 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -478,7 +478,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu) } } -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path) +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) { Object *mem_obj = object_resolve_path(obj_path, NULL); char *mempath = object_property_get_str(mem_obj, "mem-path", NULL); @@ -499,7 +499,7 @@ static inline void kvm_fixup_page_sizes(PowerPCCPU *cpu) { } -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path) +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) { return true; } diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h index f48243d13ffc..eab7c8fdb325 100644 --- a/target/ppc/kvm_ppc.h +++ b/target/ppc/kvm_ppc.h @@ -64,7 +64,7 @@ int kvmppc_enable_hwrng(void); int kvmppc_put_books_sregs(PowerPCCPU *cpu); PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void); -bool kvmppc_is_mem_backend_page_size_ok(char *obj_path); +bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path); #else @@ -211,7 +211,7 @@ static inline uint64_t kvmppc_rma_size(uint64_t current_size, return ram_size; } -static inline bool kvmppc_is_mem_backend_page_size_ok(char *obj_path) +static inline bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) { return true; } ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target/ppc: pass const string to kvmppc_is_mem_backend_page_size_ok() 2017-06-06 16:12 ` [Qemu-devel] [PATCH v2] target/ppc: pass const string " Greg Kurz @ 2017-06-06 16:34 ` Thomas Huth 0 siblings, 0 replies; 17+ messages in thread From: Thomas Huth @ 2017-06-06 16:34 UTC (permalink / raw) To: Greg Kurz, qemu-devel; +Cc: Peter Maydell, qemu-ppc, qemu-stable, David Gibson On 06.06.2017 18:12, Greg Kurz wrote: > This function has three implementations. Two are stubs that do nothing > and the third one only passes the obj_path argument to: > > Object *object_resolve_path(const char *path, bool *ambiguous); > > Signed-off-by: Greg Kurz <groug@kaod.org> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > v2: - also patch inline stub > - renamed patch and added changelog > --- > target/ppc/kvm.c | 4 ++-- > target/ppc/kvm_ppc.h | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Thomas Huth <thuth@redhat.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 2/3] target/ppc: fix memory leak in kvmppc_is_mem_backend_page_size_ok() 2017-06-06 15:22 [Qemu-devel] [PATCH 0/3] ppc: fix memory leaks Greg Kurz 2017-06-06 15:22 ` [Qemu-devel] [PATCH 1/3] target/ppc: pass const pointer to kvmppc_is_mem_backend_page_size_ok() Greg Kurz @ 2017-06-06 15:22 ` Greg Kurz 2017-06-06 15:28 ` Peter Maydell 2017-06-06 15:41 ` Thomas Huth 2017-06-06 15:22 ` [Qemu-devel] [PATCH 3/3] spapr: fix memory leak in spapr_memory_pre_plug() Greg Kurz 2017-06-06 23:45 ` [Qemu-devel] [PATCH 0/3] ppc: fix memory leaks David Gibson 3 siblings, 2 replies; 17+ messages in thread From: Greg Kurz @ 2017-06-06 15:22 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Thomas Huth, qemu-ppc, qemu-stable, David Gibson The string returned by object_property_get_str() is dynamically allocated. Signed-off-by: Greg Kurz <groug@kaod.org> --- target/ppc/kvm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 88817620766c..f2f7c531bc7b 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -486,6 +486,7 @@ bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) if (mempath) { pagesize = qemu_mempath_getpagesize(mempath); + g_free(mempath); } else { pagesize = getpagesize(); } ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] target/ppc: fix memory leak in kvmppc_is_mem_backend_page_size_ok() 2017-06-06 15:22 ` [Qemu-devel] [PATCH 2/3] target/ppc: fix memory leak in kvmppc_is_mem_backend_page_size_ok() Greg Kurz @ 2017-06-06 15:28 ` Peter Maydell 2017-06-06 15:41 ` Greg Kurz 2017-06-06 15:41 ` Thomas Huth 1 sibling, 1 reply; 17+ messages in thread From: Peter Maydell @ 2017-06-06 15:28 UTC (permalink / raw) To: Greg Kurz Cc: QEMU Developers, Thomas Huth, qemu-ppc@nongnu.org, qemu-stable, David Gibson On 6 June 2017 at 16:22, Greg Kurz <groug@kaod.org> wrote: > The string returned by object_property_get_str() is dynamically allocated. > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > target/ppc/kvm.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 88817620766c..f2f7c531bc7b 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -486,6 +486,7 @@ bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) > > if (mempath) { > pagesize = qemu_mempath_getpagesize(mempath); > + g_free(mempath); > } else { > pagesize = getpagesize(); > } Huh, I wasn't expecting this function to free its argument. If we take that API then don't we also need to change the two other implementations of it in the tree? Having the single caller do the g_free() seems simpler... thanks -- PMM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] target/ppc: fix memory leak in kvmppc_is_mem_backend_page_size_ok() 2017-06-06 15:28 ` Peter Maydell @ 2017-06-06 15:41 ` Greg Kurz 2017-06-06 15:43 ` Peter Maydell 0 siblings, 1 reply; 17+ messages in thread From: Greg Kurz @ 2017-06-06 15:41 UTC (permalink / raw) To: Peter Maydell Cc: QEMU Developers, Thomas Huth, qemu-ppc@nongnu.org, qemu-stable, David Gibson [-- Attachment #1: Type: text/plain, Size: 1553 bytes --] On Tue, 6 Jun 2017 16:28:26 +0100 Peter Maydell <peter.maydell@linaro.org> wrote: > On 6 June 2017 at 16:22, Greg Kurz <groug@kaod.org> wrote: > > The string returned by object_property_get_str() is dynamically allocated. > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > target/ppc/kvm.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > > index 88817620766c..f2f7c531bc7b 100644 > > --- a/target/ppc/kvm.c > > +++ b/target/ppc/kvm.c > > @@ -486,6 +486,7 @@ bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) > > > > if (mempath) { > > pagesize = qemu_mempath_getpagesize(mempath); > > + g_free(mempath); > > } else { > > pagesize = getpagesize(); > > } > > Huh, I wasn't expecting this function to free its argument. Hmm... mempath isn't an argument, it is computed locally: bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) { Object *mem_obj = object_resolve_path(obj_path, NULL); char *mempath = object_property_get_str(mem_obj, "mem-path", NULL); long pagesize; if (mempath) { pagesize = qemu_mempath_getpagesize(mempath); g_free(mempath); } else { pagesize = getpagesize(); } return pagesize >= max_cpu_page_size; } > If we take that API then don't we also need to change the > two other implementations of it in the tree? Having the > single caller do the g_free() seems simpler... > > thanks > -- PMM [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] target/ppc: fix memory leak in kvmppc_is_mem_backend_page_size_ok() 2017-06-06 15:41 ` Greg Kurz @ 2017-06-06 15:43 ` Peter Maydell 0 siblings, 0 replies; 17+ messages in thread From: Peter Maydell @ 2017-06-06 15:43 UTC (permalink / raw) To: Greg Kurz Cc: QEMU Developers, Thomas Huth, qemu-ppc@nongnu.org, qemu-stable, David Gibson On 6 June 2017 at 16:41, Greg Kurz <groug@kaod.org> wrote: > On Tue, 6 Jun 2017 16:28:26 +0100 > Peter Maydell <peter.maydell@linaro.org> wrote: > >> On 6 June 2017 at 16:22, Greg Kurz <groug@kaod.org> wrote: >> > The string returned by object_property_get_str() is dynamically allocated. >> > >> > Signed-off-by: Greg Kurz <groug@kaod.org> >> > --- >> > target/ppc/kvm.c | 1 + >> > 1 file changed, 1 insertion(+) >> > >> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c >> > index 88817620766c..f2f7c531bc7b 100644 >> > --- a/target/ppc/kvm.c >> > +++ b/target/ppc/kvm.c >> > @@ -486,6 +486,7 @@ bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) >> > >> > if (mempath) { >> > pagesize = qemu_mempath_getpagesize(mempath); >> > + g_free(mempath); >> > } else { >> > pagesize = getpagesize(); >> > } >> >> Huh, I wasn't expecting this function to free its argument. > > Hmm... mempath isn't an argument, it is computed locally: Oops; sorry, I misread the patch. thanks -- PMM ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] target/ppc: fix memory leak in kvmppc_is_mem_backend_page_size_ok() 2017-06-06 15:22 ` [Qemu-devel] [PATCH 2/3] target/ppc: fix memory leak in kvmppc_is_mem_backend_page_size_ok() Greg Kurz 2017-06-06 15:28 ` Peter Maydell @ 2017-06-06 15:41 ` Thomas Huth 1 sibling, 0 replies; 17+ messages in thread From: Thomas Huth @ 2017-06-06 15:41 UTC (permalink / raw) To: Greg Kurz, qemu-devel; +Cc: Peter Maydell, qemu-ppc, qemu-stable, David Gibson On 06.06.2017 17:22, Greg Kurz wrote: > The string returned by object_property_get_str() is dynamically allocated. > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > target/ppc/kvm.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 88817620766c..f2f7c531bc7b 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -486,6 +486,7 @@ bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) > > if (mempath) { > pagesize = qemu_mempath_getpagesize(mempath); > + g_free(mempath); > } else { > pagesize = getpagesize(); > } > Reviewed-by: Thomas Huth <thuth@redhat.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH 3/3] spapr: fix memory leak in spapr_memory_pre_plug() 2017-06-06 15:22 [Qemu-devel] [PATCH 0/3] ppc: fix memory leaks Greg Kurz 2017-06-06 15:22 ` [Qemu-devel] [PATCH 1/3] target/ppc: pass const pointer to kvmppc_is_mem_backend_page_size_ok() Greg Kurz 2017-06-06 15:22 ` [Qemu-devel] [PATCH 2/3] target/ppc: fix memory leak in kvmppc_is_mem_backend_page_size_ok() Greg Kurz @ 2017-06-06 15:22 ` Greg Kurz 2017-06-06 15:43 ` Thomas Huth 2017-06-06 23:45 ` [Qemu-devel] [PATCH 0/3] ppc: fix memory leaks David Gibson 3 siblings, 1 reply; 17+ messages in thread From: Greg Kurz @ 2017-06-06 15:22 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Thomas Huth, qemu-ppc, qemu-stable, David Gibson The string returned by object_property_get_str() is dynamically allocated. (Spotted by Coverity, CID 1375942) Signed-off-by: Greg Kurz <groug@kaod.org> --- hw/ppc/spapr.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 86e622834f63..f834a6a7dfac 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2617,8 +2617,11 @@ static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, if (mem_dev && !kvmppc_is_mem_backend_page_size_ok(mem_dev)) { error_setg(errp, "Memory backend has bad page size. " "Use 'memory-backend-file' with correct mem-path."); - return; + goto out; } + +out: + g_free(mem_dev); } struct sPAPRDIMMState { ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] spapr: fix memory leak in spapr_memory_pre_plug() 2017-06-06 15:22 ` [Qemu-devel] [PATCH 3/3] spapr: fix memory leak in spapr_memory_pre_plug() Greg Kurz @ 2017-06-06 15:43 ` Thomas Huth 2017-06-06 15:55 ` Greg Kurz 0 siblings, 1 reply; 17+ messages in thread From: Thomas Huth @ 2017-06-06 15:43 UTC (permalink / raw) To: Greg Kurz, qemu-devel; +Cc: Peter Maydell, qemu-ppc, qemu-stable, David Gibson On 06.06.2017 17:22, Greg Kurz wrote: > The string returned by object_property_get_str() is dynamically allocated. > > (Spotted by Coverity, CID 1375942) > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > hw/ppc/spapr.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 86e622834f63..f834a6a7dfac 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -2617,8 +2617,11 @@ static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > if (mem_dev && !kvmppc_is_mem_backend_page_size_ok(mem_dev)) { > error_setg(errp, "Memory backend has bad page size. " > "Use 'memory-backend-file' with correct mem-path."); > - return; > + goto out; > } > + > +out: > + g_free(mem_dev); > } You don't need the goto and the "out" label here. Thomas ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] spapr: fix memory leak in spapr_memory_pre_plug() 2017-06-06 15:43 ` Thomas Huth @ 2017-06-06 15:55 ` Greg Kurz 0 siblings, 0 replies; 17+ messages in thread From: Greg Kurz @ 2017-06-06 15:55 UTC (permalink / raw) To: Thomas Huth Cc: qemu-devel, Peter Maydell, qemu-ppc, qemu-stable, David Gibson [-- Attachment #1: Type: text/plain, Size: 1267 bytes --] On Tue, 6 Jun 2017 17:43:13 +0200 Thomas Huth <thuth@redhat.com> wrote: > On 06.06.2017 17:22, Greg Kurz wrote: > > The string returned by object_property_get_str() is dynamically allocated. > > > > (Spotted by Coverity, CID 1375942) > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > hw/ppc/spapr.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 86e622834f63..f834a6a7dfac 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -2617,8 +2617,11 @@ static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > > if (mem_dev && !kvmppc_is_mem_backend_page_size_ok(mem_dev)) { > > error_setg(errp, "Memory backend has bad page size. " > > "Use 'memory-backend-file' with correct mem-path."); > > - return; > > + goto out; > > } > > + > > +out: > > + g_free(mem_dev); > > } > > You don't need the goto and the "out" label here. > > Thomas I did it on purpose, so that someone may add some code to this function and doesn't need to care for the freeing... but of course, we can simply: - return; } + g_free(mem_dev); } [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] ppc: fix memory leaks 2017-06-06 15:22 [Qemu-devel] [PATCH 0/3] ppc: fix memory leaks Greg Kurz ` (2 preceding siblings ...) 2017-06-06 15:22 ` [Qemu-devel] [PATCH 3/3] spapr: fix memory leak in spapr_memory_pre_plug() Greg Kurz @ 2017-06-06 23:45 ` David Gibson 2017-06-07 5:45 ` Greg Kurz 3 siblings, 1 reply; 17+ messages in thread From: David Gibson @ 2017-06-06 23:45 UTC (permalink / raw) To: Greg Kurz; +Cc: qemu-devel, Peter Maydell, Thomas Huth, qemu-ppc, qemu-stable [-- Attachment #1: Type: text/plain, Size: 797 bytes --] On Tue, Jun 06, 2017 at 05:22:42PM +0200, Greg Kurz wrote: > Coverity just reported a memory leak introduced by this commit (QEMU 2.9): > > commit df58713396f8b2deb923e39c00b10744c5c63909 > Author: Thomas Huth <thuth@redhat.com> > Date: Wed Feb 15 10:21:44 2017 +0100 > > hw/ppc/spapr: Check for valid page size when hot plugging memory > > It boils down to the fact that object_property_get_str() returns a string > allocated with g_strdup(), which must be deallocated with g_free() at some > point. Applied to ppc-for-2.10. Do we need to queue this for 2.9 stable as well? -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH 0/3] ppc: fix memory leaks 2017-06-06 23:45 ` [Qemu-devel] [PATCH 0/3] ppc: fix memory leaks David Gibson @ 2017-06-07 5:45 ` Greg Kurz 0 siblings, 0 replies; 17+ messages in thread From: Greg Kurz @ 2017-06-07 5:45 UTC (permalink / raw) To: David Gibson Cc: qemu-devel, Peter Maydell, Thomas Huth, qemu-ppc, qemu-stable [-- Attachment #1: Type: text/plain, Size: 733 bytes --] On Wed, 7 Jun 2017 09:45:06 +1000 David Gibson <david@gibson.dropbear.id.au> wrote: > On Tue, Jun 06, 2017 at 05:22:42PM +0200, Greg Kurz wrote: > > Coverity just reported a memory leak introduced by this commit (QEMU 2.9): > > > > commit df58713396f8b2deb923e39c00b10744c5c63909 > > Author: Thomas Huth <thuth@redhat.com> > > Date: Wed Feb 15 10:21:44 2017 +0100 > > > > hw/ppc/spapr: Check for valid page size when hot plugging memory > > > > It boils down to the fact that object_property_get_str() returns a string > > allocated with g_strdup(), which must be deallocated with g_free() at some > > point. > > Applied to ppc-for-2.10. Do we need to queue this for 2.9 stable as > well? > Yes. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-06-07 5:45 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-06 15:22 [Qemu-devel] [PATCH 0/3] ppc: fix memory leaks Greg Kurz 2017-06-06 15:22 ` [Qemu-devel] [PATCH 1/3] target/ppc: pass const pointer to kvmppc_is_mem_backend_page_size_ok() Greg Kurz 2017-06-06 15:33 ` Philippe Mathieu-Daudé 2017-06-06 15:34 ` Thomas Huth 2017-06-06 15:46 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz 2017-06-06 16:12 ` [Qemu-devel] [PATCH v2] target/ppc: pass const string " Greg Kurz 2017-06-06 16:34 ` Thomas Huth 2017-06-06 15:22 ` [Qemu-devel] [PATCH 2/3] target/ppc: fix memory leak in kvmppc_is_mem_backend_page_size_ok() Greg Kurz 2017-06-06 15:28 ` Peter Maydell 2017-06-06 15:41 ` Greg Kurz 2017-06-06 15:43 ` Peter Maydell 2017-06-06 15:41 ` Thomas Huth 2017-06-06 15:22 ` [Qemu-devel] [PATCH 3/3] spapr: fix memory leak in spapr_memory_pre_plug() Greg Kurz 2017-06-06 15:43 ` Thomas Huth 2017-06-06 15:55 ` Greg Kurz 2017-06-06 23:45 ` [Qemu-devel] [PATCH 0/3] ppc: fix memory leaks David Gibson 2017-06-07 5:45 ` Greg Kurz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).