* [PATCH v2] staging: bcm2835-audio: Fix memory corruption
@ 2017-08-11 10:20 Phil Elwell
2017-08-11 13:46 ` Dan Carpenter
2017-09-23 10:57 ` Stefan Wahren
0 siblings, 2 replies; 5+ messages in thread
From: Phil Elwell @ 2017-08-11 10:20 UTC (permalink / raw)
To: Eric Anholt, Stefan Wahren, Greg Kroah-Hartman, Florian Fainelli,
Aishwarya Pant, Dan Carpenter, linux-rpi-kernel, devel,
linux-kernel
Cc: 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 <phil@raspberrypi.org>
Fixes: 0adbfd4694c2 ("staging: bcm2835-audio: fix memory leak in bcm2835_audio_open_connection()")
---
[Resend with v2 in subject]
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] staging: bcm2835-audio: Fix memory corruption
2017-08-11 10:20 [PATCH v2] staging: bcm2835-audio: Fix memory corruption Phil Elwell
@ 2017-08-11 13:46 ` Dan Carpenter
2017-09-23 10:57 ` Stefan Wahren
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2017-08-11 13:46 UTC (permalink / raw)
To: Phil Elwell
Cc: Eric Anholt, Stefan Wahren, Greg Kroah-Hartman, Florian Fainelli,
Aishwarya Pant, linux-rpi-kernel, devel, linux-kernel
Looks Ok to me.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] staging: bcm2835-audio: Fix memory corruption
2017-08-11 10:20 [PATCH v2] staging: bcm2835-audio: Fix memory corruption Phil Elwell
2017-08-11 13:46 ` Dan Carpenter
@ 2017-09-23 10:57 ` Stefan Wahren
2017-09-23 16:15 ` Greg Kroah-Hartman
1 sibling, 1 reply; 5+ messages in thread
From: Stefan Wahren @ 2017-09-23 10:57 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Aishwarya Pant, Phil Elwell, Eric Anholt, linux-kernel, devel,
Dan Carpenter, linux-rpi-kernel
Hi Greg,
> Phil Elwell <phil@raspberrypi.org> hat am 11. August 2017 um 12: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 <phil@raspberrypi.org>
> Fixes: 0adbfd4694c2 ("staging: bcm2835-audio: fix memory leak in bcm2835_audio_open_connection()")
can you still apply this patch or do you need a resend?
> ---
> [Resend with v2 in subject]
> 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] staging: bcm2835-audio: Fix memory corruption
2017-09-23 10:57 ` Stefan Wahren
@ 2017-09-23 16:15 ` Greg Kroah-Hartman
2017-09-23 22:44 ` Stefan Wahren
0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2017-09-23 16:15 UTC (permalink / raw)
To: Stefan Wahren
Cc: Aishwarya Pant, Phil Elwell, Eric Anholt, linux-kernel, devel,
Dan Carpenter, linux-rpi-kernel
On Sat, Sep 23, 2017 at 12:57:33PM +0200, Stefan Wahren wrote:
> Hi Greg,
>
> > Phil Elwell <phil@raspberrypi.org> hat am 11. August 2017 um 12: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 <phil@raspberrypi.org>
> > Fixes: 0adbfd4694c2 ("staging: bcm2835-audio: fix memory leak in bcm2835_audio_open_connection()")
>
> can you still apply this patch or do you need a resend?
Hm, I don't see this anywhere in my tree, or in my todo mbox, so yes,
please resend it, thanks.
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] staging: bcm2835-audio: Fix memory corruption
2017-09-23 16:15 ` Greg Kroah-Hartman
@ 2017-09-23 22:44 ` Stefan Wahren
0 siblings, 0 replies; 5+ messages in thread
From: Stefan Wahren @ 2017-09-23 22:44 UTC (permalink / raw)
To: Phil Elwell
Cc: Aishwarya Pant, Eric Anholt, Dan Carpenter, linux-kernel, devel,
linux-rpi-kernel, Greg Kroah-Hartman
Hi Phil,
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> hat am 23. September 2017 um 18:15 geschrieben:
>
>
> On Sat, Sep 23, 2017 at 12:57:33PM +0200, Stefan Wahren wrote:
> > Hi Greg,
> >
> > > Phil Elwell <phil@raspberrypi.org> hat am 11. August 2017 um 12: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 <phil@raspberrypi.org>
> > > Fixes: 0adbfd4694c2 ("staging: bcm2835-audio: fix memory leak in bcm2835_audio_open_connection()")
> >
> > can you still apply this patch or do you need a resend?
>
> Hm, I don't see this anywhere in my tree, or in my todo mbox, so yes,
> please resend it, thanks.
could you please resend this patch?
Thanks
Stefan
>
> greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-09-23 22:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-11 10:20 [PATCH v2] staging: bcm2835-audio: Fix memory corruption Phil Elwell
2017-08-11 13:46 ` Dan Carpenter
2017-09-23 10:57 ` Stefan Wahren
2017-09-23 16:15 ` Greg Kroah-Hartman
2017-09-23 22:44 ` Stefan Wahren
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.