* [PATCH] alsa-lib: Protect from freeing semaphore when already in use
@ 2016-11-25 10:13 sutar.mounesh
2016-11-28 19:15 ` Takashi Iwai
0 siblings, 1 reply; 2+ messages in thread
From: sutar.mounesh @ 2016-11-25 10:13 UTC (permalink / raw)
To: patch; +Cc: alsa-devel, Joshua Frkuska, mounesh_sutar
From: Joshua Frkuska <joshua_frkuska@mentor.com>
In the case of dshare, dsnoop, and dmix when a device is opened twice
and fails the second time, the semaphore is completely discarded. This
creates dangling semaphore data.
This patch removes the possibility for the semaphore to be destroyed during
a typical open failure by first checking if the shared memory can be destroyed
or not. If the shared memory cannot be released it means both it and the
semaphore are still in use and therefore the semaphore is just released.
Signed-off-by: Joshua Frkuska <joshua_frkuska@mentor.com>
---
src/pcm/pcm_dmix.c | 7 ++++---
src/pcm/pcm_dshare.c | 7 ++++---
src/pcm/pcm_dsnoop.c | 8 +++++---
3 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c
index 2bd5d39..b17f759 100644
--- a/src/pcm/pcm_dmix.c
+++ b/src/pcm/pcm_dmix.c
@@ -1128,9 +1128,10 @@ int snd_pcm_dmix_open(snd_pcm_t **pcmp, const char *name,
snd_pcm_close(spcm);
if (dmix->u.dmix.shmid_sum >= 0)
shm_sum_discard(dmix);
- if (dmix->shmid >= 0)
- snd_pcm_direct_shm_discard(dmix);
- if (snd_pcm_direct_semaphore_discard(dmix) < 0)
+ if ((dmix->shmid >= 0) && (snd_pcm_direct_shm_discard(dmix))) {
+ if (snd_pcm_direct_semaphore_discard(dmix))
+ snd_pcm_direct_semaphore_final(dmix, DIRECT_IPC_SEM_CLIENT);
+ } else
snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT);
_err_nosem:
if (dmix) {
diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c
index 5b32951..c080e7a 100644
--- a/src/pcm/pcm_dshare.c
+++ b/src/pcm/pcm_dshare.c
@@ -813,9 +813,10 @@ int snd_pcm_dshare_open(snd_pcm_t **pcmp, const char *name,
snd_pcm_direct_client_discard(dshare);
if (spcm)
snd_pcm_close(spcm);
- if (dshare->shmid >= 0)
- snd_pcm_direct_shm_discard(dshare);
- if (snd_pcm_direct_semaphore_discard(dshare) < 0)
+ if ((dshare->shmid >= 0) && (snd_pcm_direct_shm_discard(dshare))) {
+ if (snd_pcm_direct_semaphore_discard(dshare))
+ snd_pcm_direct_semaphore_final(dshare, DIRECT_IPC_SEM_CLIENT);
+ } else
snd_pcm_direct_semaphore_up(dshare, DIRECT_IPC_SEM_CLIENT);
_err_nosem:
if (dshare) {
diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c
index 055e4f4..74a3531 100644
--- a/src/pcm/pcm_dsnoop.c
+++ b/src/pcm/pcm_dsnoop.c
@@ -712,10 +712,12 @@ int snd_pcm_dsnoop_open(snd_pcm_t **pcmp, const char *name,
snd_pcm_direct_client_discard(dsnoop);
if (spcm)
snd_pcm_close(spcm);
- if (dsnoop->shmid >= 0)
- snd_pcm_direct_shm_discard(dsnoop);
- if (snd_pcm_direct_semaphore_discard(dsnoop) < 0)
+ if ((dsnoop->shmid >= 0) && (snd_pcm_direct_shm_discard(dsnoop))) {
+ if (snd_pcm_direct_semaphore_discard(dsnoop))
+ snd_pcm_direct_semaphore_final(dsnoop, DIRECT_IPC_SEM_CLIENT);
+ } else
snd_pcm_direct_semaphore_up(dsnoop, DIRECT_IPC_SEM_CLIENT);
+
_err_nosem:
if (dsnoop) {
free(dsnoop->bindings);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] alsa-lib: Protect from freeing semaphore when already in use
2016-11-25 10:13 [PATCH] alsa-lib: Protect from freeing semaphore when already in use sutar.mounesh
@ 2016-11-28 19:15 ` Takashi Iwai
0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2016-11-28 19:15 UTC (permalink / raw)
To: sutar.mounesh; +Cc: alsa-devel, Joshua Frkuska, mounesh_sutar
On Fri, 25 Nov 2016 11:13:40 +0100,
sutar.mounesh@gmail.com wrote:
>
> From: Joshua Frkuska <joshua_frkuska@mentor.com>
>
> In the case of dshare, dsnoop, and dmix when a device is opened twice
> and fails the second time, the semaphore is completely discarded. This
> creates dangling semaphore data.
>
> This patch removes the possibility for the semaphore to be destroyed during
> a typical open failure by first checking if the shared memory can be destroyed
> or not. If the shared memory cannot be released it means both it and the
> semaphore are still in use and therefore the semaphore is just released.
>
> Signed-off-by: Joshua Frkuska <joshua_frkuska@mentor.com>
Do you have some test code? It'd be great for checking the bug and
avoiding regressions.
In anyway, the changes look good to me, so I applied it now.
thanks,
Takashi
> ---
> src/pcm/pcm_dmix.c | 7 ++++---
> src/pcm/pcm_dshare.c | 7 ++++---
> src/pcm/pcm_dsnoop.c | 8 +++++---
> 3 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c
> index 2bd5d39..b17f759 100644
> --- a/src/pcm/pcm_dmix.c
> +++ b/src/pcm/pcm_dmix.c
> @@ -1128,9 +1128,10 @@ int snd_pcm_dmix_open(snd_pcm_t **pcmp, const char *name,
> snd_pcm_close(spcm);
> if (dmix->u.dmix.shmid_sum >= 0)
> shm_sum_discard(dmix);
> - if (dmix->shmid >= 0)
> - snd_pcm_direct_shm_discard(dmix);
> - if (snd_pcm_direct_semaphore_discard(dmix) < 0)
> + if ((dmix->shmid >= 0) && (snd_pcm_direct_shm_discard(dmix))) {
> + if (snd_pcm_direct_semaphore_discard(dmix))
> + snd_pcm_direct_semaphore_final(dmix, DIRECT_IPC_SEM_CLIENT);
> + } else
> snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT);
> _err_nosem:
> if (dmix) {
> diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c
> index 5b32951..c080e7a 100644
> --- a/src/pcm/pcm_dshare.c
> +++ b/src/pcm/pcm_dshare.c
> @@ -813,9 +813,10 @@ int snd_pcm_dshare_open(snd_pcm_t **pcmp, const char *name,
> snd_pcm_direct_client_discard(dshare);
> if (spcm)
> snd_pcm_close(spcm);
> - if (dshare->shmid >= 0)
> - snd_pcm_direct_shm_discard(dshare);
> - if (snd_pcm_direct_semaphore_discard(dshare) < 0)
> + if ((dshare->shmid >= 0) && (snd_pcm_direct_shm_discard(dshare))) {
> + if (snd_pcm_direct_semaphore_discard(dshare))
> + snd_pcm_direct_semaphore_final(dshare, DIRECT_IPC_SEM_CLIENT);
> + } else
> snd_pcm_direct_semaphore_up(dshare, DIRECT_IPC_SEM_CLIENT);
> _err_nosem:
> if (dshare) {
> diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c
> index 055e4f4..74a3531 100644
> --- a/src/pcm/pcm_dsnoop.c
> +++ b/src/pcm/pcm_dsnoop.c
> @@ -712,10 +712,12 @@ int snd_pcm_dsnoop_open(snd_pcm_t **pcmp, const char *name,
> snd_pcm_direct_client_discard(dsnoop);
> if (spcm)
> snd_pcm_close(spcm);
> - if (dsnoop->shmid >= 0)
> - snd_pcm_direct_shm_discard(dsnoop);
> - if (snd_pcm_direct_semaphore_discard(dsnoop) < 0)
> + if ((dsnoop->shmid >= 0) && (snd_pcm_direct_shm_discard(dsnoop))) {
> + if (snd_pcm_direct_semaphore_discard(dsnoop))
> + snd_pcm_direct_semaphore_final(dsnoop, DIRECT_IPC_SEM_CLIENT);
> + } else
> snd_pcm_direct_semaphore_up(dsnoop, DIRECT_IPC_SEM_CLIENT);
> +
> _err_nosem:
> if (dsnoop) {
> free(dsnoop->bindings);
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-11-28 19:15 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-25 10:13 [PATCH] alsa-lib: Protect from freeing semaphore when already in use sutar.mounesh
2016-11-28 19:15 ` Takashi Iwai
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).