Re: [PATCH v2 resend] staging: bcm2835-audio: Fix memory corruption

2017-09-24 Thread Stefan Wahren
> Phil Elwell  hat am 24. September 2017 um 16:20 
> geschrieben:
> 
> 
> The previous commit (0adbfd46) fixed a memory leak but also freed a
> block in the success case, causing a stale pointer to be used with
> potentially fatal results. Only free the vchi_instance block in the
> case that vchi_connect fails; once connected, the instance is
> retained for subsequent connections.
> 
> Simplifying the code by removing a bunch of gotos and returning errors
> directly.
> 
> Signed-off-by: Phil Elwell 
> Fixes: 0adbfd4694c2 ("staging: bcm2835-audio: fix memory leak in 
> bcm2835_audio_open_connection()")

Tested-by: Stefan Wahren 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 resend] staging: bcm2835-audio: Fix memory corruption

2017-09-24 Thread Phil Elwell
The previous commit (0adbfd46) fixed a memory leak but also freed a
block in the success case, causing a stale pointer to be used with
potentially fatal results. Only free the vchi_instance block in the
case that vchi_connect fails; once connected, the instance is
retained for subsequent connections.

Simplifying the code by removing a bunch of gotos and returning errors
directly.

Signed-off-by: Phil Elwell 
Fixes: 0adbfd4694c2 ("staging: bcm2835-audio: fix memory leak in 
bcm2835_audio_open_connection()")
---
v2: Simplified following feedback from Dan Carpenter.
---
 .../vc04_services/bcm2835-audio/bcm2835-vchiq.c   | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c 
b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
index 5f3d8f2..4be864d 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
@@ -390,8 +390,7 @@ static int bcm2835_audio_open_connection(struct 
bcm2835_alsa_stream *alsa_stream
__func__, instance);
instance->alsa_stream = alsa_stream;
alsa_stream->instance = instance;
-   ret = 0; // xxx todo -1;
-   goto err_free_mem;
+   return 0;
}
 
/* Initialize and create a VCHI connection */
@@ -401,16 +400,15 @@ static int bcm2835_audio_open_connection(struct 
bcm2835_alsa_stream *alsa_stream
LOG_ERR("%s: failed to initialise VCHI instance 
(ret=%d)\n",
__func__, ret);
 
-   ret = -EIO;
-   goto err_free_mem;
+   return -EIO;
}
ret = vchi_connect(NULL, 0, vchi_instance);
if (ret) {
LOG_ERR("%s: failed to connect VCHI instance 
(ret=%d)\n",
__func__, ret);
 
-   ret = -EIO;
-   goto err_free_mem;
+   kfree(vchi_instance);
+   return -EIO;
}
initted = 1;
}
@@ -421,19 +419,16 @@ static int bcm2835_audio_open_connection(struct 
bcm2835_alsa_stream *alsa_stream
if (IS_ERR(instance)) {
LOG_ERR("%s: failed to initialize audio service\n", __func__);
 
-   ret = PTR_ERR(instance);
-   goto err_free_mem;
+   /* vchi_instance is retained for use the next time. */
+   return PTR_ERR(instance);
}
 
instance->alsa_stream = alsa_stream;
alsa_stream->instance = instance;
 
LOG_DBG(" success !\n");
-   ret = 0;
-err_free_mem:
-   kfree(vchi_instance);
 
-   return ret;
+   return 0;
 }
 
 int bcm2835_audio_open(struct bcm2835_alsa_stream *alsa_stream)
-- 
1.9.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel