qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] hw/gpio/pca9554: Avoid leak in pca9554_set_pin()
@ 2025-08-21 15:44 Peter Maydell
  2025-08-22  8:01 ` Philippe Mathieu-Daudé
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Peter Maydell @ 2025-08-21 15:44 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: qemu-ppc, Glenn Miles

In pca9554_set_pin() we have a string property which we parse in
order to set some non-string fields in the device state.  So we call
visit_type_str(), passing it the address of the local variable
state_str.

visit_type_str() will allocate a new copy of the string; we
never free this string, so the result is a memory leak, detected
by ASAN during a "make check" run:

Direct leak of 5 byte(s) in 1 object(s) allocated from:
    #0 0x5d605212ede3 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-arm+0x21f1de3) (
BuildId: 3d5373c89317f58bfcd191a33988c7347714be14)
    #1 0x7f7edea57b09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b68282
9a6913cf682d75)
    #2 0x7f7edea6d4d8 in g_strdup (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x784d8) (BuildId: 1eb6131419edb83b2178b68282
9a6913cf682d75)
    #3 0x5d6055289a91 in g_strdup_inline /usr/include/glib-2.0/glib/gstrfuncs.h:321:10
    #4 0x5d6055289a91 in qobject_input_type_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qapi/qo
bject-input-visitor.c:542:12
    #5 0x5d605528479c in visit_type_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qapi/qapi-visit
-core.c:349:10
    #6 0x5d60528bdd87 in pca9554_set_pin /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/gpio/pca9554.c:179:10
    #7 0x5d60549bcbbb in object_property_set /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:1450:5
    #8 0x5d60549d2055 in object_property_set_qobject /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/qom-qobject.c:28:10
    #9 0x5d60549bcdf1 in object_property_set_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:1458:15
    #10 0x5d605439d077 in gb200nvl_bmc_i2c_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/arm/aspeed.c:1267:5
    #11 0x5d60543a3bbc in aspeed_machine_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/arm/aspeed.c:493:9


Make the state_str g_autofree, so that we will always free
it, on both error-exit and success codepaths.

Cc: qemu-stable@nongnu.org
Fixes: de0c7d543bca ("misc: Add a pca9554 GPIO device model")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/gpio/pca9554.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/gpio/pca9554.c b/hw/gpio/pca9554.c
index de3f883aee9..eac0d23be34 100644
--- a/hw/gpio/pca9554.c
+++ b/hw/gpio/pca9554.c
@@ -174,7 +174,7 @@ static void pca9554_set_pin(Object *obj, Visitor *v, const char *name,
     PCA9554State *s = PCA9554(obj);
     int pin, rc, val;
     uint8_t state, mask;
-    char *state_str;
+    g_autofree char *state_str = NULL;
 
     if (!visit_type_str(v, name, &state_str, errp)) {
         return;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] hw/gpio/pca9554: Avoid leak in pca9554_set_pin()
  2025-08-21 15:44 [PATCH] hw/gpio/pca9554: Avoid leak in pca9554_set_pin() Peter Maydell
@ 2025-08-22  8:01 ` Philippe Mathieu-Daudé
  2025-08-22 14:16 ` Miles Glenn
  2025-08-28  8:49 ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-22  8:01 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel
  Cc: qemu-ppc, Glenn Miles, Markus Armbruster

On 21/8/25 17:44, Peter Maydell wrote:
> In pca9554_set_pin() we have a string property which we parse in
> order to set some non-string fields in the device state.  So we call
> visit_type_str(), passing it the address of the local variable
> state_str.
> 
> visit_type_str() will allocate a new copy of the string; we
> never free this string, so the result is a memory leak, detected
> by ASAN during a "make check" run:
> 
> Direct leak of 5 byte(s) in 1 object(s) allocated from:
>      #0 0x5d605212ede3 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-arm+0x21f1de3) (
> BuildId: 3d5373c89317f58bfcd191a33988c7347714be14)
>      #1 0x7f7edea57b09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b68282
> 9a6913cf682d75)
>      #2 0x7f7edea6d4d8 in g_strdup (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x784d8) (BuildId: 1eb6131419edb83b2178b68282
> 9a6913cf682d75)
>      #3 0x5d6055289a91 in g_strdup_inline /usr/include/glib-2.0/glib/gstrfuncs.h:321:10
>      #4 0x5d6055289a91 in qobject_input_type_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qapi/qo
> bject-input-visitor.c:542:12
>      #5 0x5d605528479c in visit_type_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qapi/qapi-visit
> -core.c:349:10
>      #6 0x5d60528bdd87 in pca9554_set_pin /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/gpio/pca9554.c:179:10
>      #7 0x5d60549bcbbb in object_property_set /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:1450:5
>      #8 0x5d60549d2055 in object_property_set_qobject /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/qom-qobject.c:28:10
>      #9 0x5d60549bcdf1 in object_property_set_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:1458:15
>      #10 0x5d605439d077 in gb200nvl_bmc_i2c_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/arm/aspeed.c:1267:5
>      #11 0x5d60543a3bbc in aspeed_machine_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/arm/aspeed.c:493:9
> 
> 
> Make the state_str g_autofree, so that we will always free
> it, on both error-exit and success codepaths.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: de0c7d543bca ("misc: Add a pca9554 GPIO device model")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/gpio/pca9554.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/gpio/pca9554.c b/hw/gpio/pca9554.c
> index de3f883aee9..eac0d23be34 100644
> --- a/hw/gpio/pca9554.c
> +++ b/hw/gpio/pca9554.c
> @@ -174,7 +174,7 @@ static void pca9554_set_pin(Object *obj, Visitor *v, const char *name,
>       PCA9554State *s = PCA9554(obj);
>       int pin, rc, val;
>       uint8_t state, mask;
> -    char *state_str;
> +    g_autofree char *state_str = NULL;
>   
>       if (!visit_type_str(v, name, &state_str, errp)) {
>           return;

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] hw/gpio/pca9554: Avoid leak in pca9554_set_pin()
  2025-08-21 15:44 [PATCH] hw/gpio/pca9554: Avoid leak in pca9554_set_pin() Peter Maydell
  2025-08-22  8:01 ` Philippe Mathieu-Daudé
@ 2025-08-22 14:16 ` Miles Glenn
  2025-08-28  8:49 ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 4+ messages in thread
From: Miles Glenn @ 2025-08-22 14:16 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: qemu-ppc

Reviewed-by: Glenn Miles <milesg@linux.ibm.com>

Thanks,

Glenn

On Thu, 2025-08-21 at 16:44 +0100, Peter Maydell wrote:
> In pca9554_set_pin() we have a string property which we parse in
> order to set some non-string fields in the device state.  So we call
> visit_type_str(), passing it the address of the local variable
> state_str.
> 
> visit_type_str() will allocate a new copy of the string; we
> never free this string, so the result is a memory leak, detected
> by ASAN during a "make check" run:
> 
> Direct leak of 5 byte(s) in 1 object(s) allocated from:
>     #0 0x5d605212ede3 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-arm+0x21f1de3) (
> BuildId: 3d5373c89317f58bfcd191a33988c7347714be14)
>     #1 0x7f7edea57b09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b68282
> 9a6913cf682d75)
>     #2 0x7f7edea6d4d8 in g_strdup (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x784d8) (BuildId: 1eb6131419edb83b2178b68282
> 9a6913cf682d75)
>     #3 0x5d6055289a91 in g_strdup_inline /usr/include/glib-2.0/glib/gstrfuncs.h:321:10
>     #4 0x5d6055289a91 in qobject_input_type_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qapi/qo
> bject-input-visitor.c:542:12
>     #5 0x5d605528479c in visit_type_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qapi/qapi-visit
> -core.c:349:10
>     #6 0x5d60528bdd87 in pca9554_set_pin /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/gpio/pca9554.c:179:10
>     #7 0x5d60549bcbbb in object_property_set /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:1450:5
>     #8 0x5d60549d2055 in object_property_set_qobject /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/qom-qobject.c:28:10
>     #9 0x5d60549bcdf1 in object_property_set_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:1458:15
>     #10 0x5d605439d077 in gb200nvl_bmc_i2c_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/arm/aspeed.c:1267:5
>     #11 0x5d60543a3bbc in aspeed_machine_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/arm/aspeed.c:493:9
> 
> 
> Make the state_str g_autofree, so that we will always free
> it, on both error-exit and success codepaths.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: de0c7d543bca ("misc: Add a pca9554 GPIO device model")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/gpio/pca9554.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/gpio/pca9554.c b/hw/gpio/pca9554.c
> index de3f883aee9..eac0d23be34 100644
> --- a/hw/gpio/pca9554.c
> +++ b/hw/gpio/pca9554.c
> @@ -174,7 +174,7 @@ static void pca9554_set_pin(Object *obj, Visitor *v, const char *name,
>      PCA9554State *s = PCA9554(obj);
>      int pin, rc, val;
>      uint8_t state, mask;
> -    char *state_str;
> +    g_autofree char *state_str = NULL;
>  
>      if (!visit_type_str(v, name, &state_str, errp)) {
>          return;



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] hw/gpio/pca9554: Avoid leak in pca9554_set_pin()
  2025-08-21 15:44 [PATCH] hw/gpio/pca9554: Avoid leak in pca9554_set_pin() Peter Maydell
  2025-08-22  8:01 ` Philippe Mathieu-Daudé
  2025-08-22 14:16 ` Miles Glenn
@ 2025-08-28  8:49 ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-28  8:49 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: qemu-ppc, Glenn Miles

On 21/8/25 17:44, Peter Maydell wrote:
> In pca9554_set_pin() we have a string property which we parse in
> order to set some non-string fields in the device state.  So we call
> visit_type_str(), passing it the address of the local variable
> state_str.
> 
> visit_type_str() will allocate a new copy of the string; we
> never free this string, so the result is a memory leak, detected
> by ASAN during a "make check" run:
> 
> Direct leak of 5 byte(s) in 1 object(s) allocated from:
>      #0 0x5d605212ede3 in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/qemu-system-arm+0x21f1de3) (
> BuildId: 3d5373c89317f58bfcd191a33988c7347714be14)
>      #1 0x7f7edea57b09 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x62b09) (BuildId: 1eb6131419edb83b2178b68282
> 9a6913cf682d75)
>      #2 0x7f7edea6d4d8 in g_strdup (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x784d8) (BuildId: 1eb6131419edb83b2178b68282
> 9a6913cf682d75)
>      #3 0x5d6055289a91 in g_strdup_inline /usr/include/glib-2.0/glib/gstrfuncs.h:321:10
>      #4 0x5d6055289a91 in qobject_input_type_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qapi/qo
> bject-input-visitor.c:542:12
>      #5 0x5d605528479c in visit_type_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qapi/qapi-visit
> -core.c:349:10
>      #6 0x5d60528bdd87 in pca9554_set_pin /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/gpio/pca9554.c:179:10
>      #7 0x5d60549bcbbb in object_property_set /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:1450:5
>      #8 0x5d60549d2055 in object_property_set_qobject /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/qom-qobject.c:28:10
>      #9 0x5d60549bcdf1 in object_property_set_str /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../qom/object.c:1458:15
>      #10 0x5d605439d077 in gb200nvl_bmc_i2c_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/arm/aspeed.c:1267:5
>      #11 0x5d60543a3bbc in aspeed_machine_init /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-asan/../../hw/arm/aspeed.c:493:9
> 
> 
> Make the state_str g_autofree, so that we will always free
> it, on both error-exit and success codepaths.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: de0c7d543bca ("misc: Add a pca9554 GPIO device model")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/gpio/pca9554.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Patch queued, thanks.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-08-28  8:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-21 15:44 [PATCH] hw/gpio/pca9554: Avoid leak in pca9554_set_pin() Peter Maydell
2025-08-22  8:01 ` Philippe Mathieu-Daudé
2025-08-22 14:16 ` Miles Glenn
2025-08-28  8:49 ` Philippe Mathieu-Daudé

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).