2017-08-11 09:58:38

by Phil Elwell

[permalink] [raw]
Subject: [PATCH] staging: bcm2835-audio: Fix memory corruption

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 <[email protected]>
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