[pulseaudio-discuss] [PATCH 3/3] pa_*_volume_change_push: Do not dereference freed memory when freeing the next events

[David Henningsson]

Ack, but would it not be more elegant to use PA_LLIST_FOREACH_SAFE?

On 2015-09-11 02:42, Felipe Sateler wrote:
> Found by coverity
> ---
>   src/pulsecore/sink.c   | 8 ++++++--
>   src/pulsecore/source.c | 8 ++++++--
>   2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
> index c2bf0e4..da008dd 100644
> --- a/src/pulsecore/sink.c
> +++ b/src/pulsecore/sink.c
> @@ -3543,6 +3543,7 @@ static void pa_sink_volume_change_free(pa_sink_volume_change *c) {
>   void pa_sink_volume_change_push(pa_sink *s) {
>       pa_sink_volume_change *c = NULL;
>       pa_sink_volume_change *nc = NULL;
> +    pa_sink_volume_change *pc = NULL;
>       uint32_t safety_margin = s->thread_info.volume_change_safety_margin;
>
>       const char *direction = NULL;
> @@ -3600,9 +3601,12 @@ void pa_sink_volume_change_push(pa_sink *s) {
>       pa_log_debug("Volume going %s to %d at %llu", direction, pa_cvolume_avg(&nc->hw_volume), (long long unsigned) nc->at);
>
>       /* We can ignore volume events that came earlier but should happen later than this. */
> -    PA_LLIST_FOREACH(c, nc->next) {
> +    c = nc->next;
> +    while(c) {
>           pa_log_debug("Volume change to %d at %llu was dropped", pa_cvolume_avg(&c->hw_volume), (long long unsigned) c->at);
> -        pa_sink_volume_change_free(c);
> +        pc = c;
> +        c = c->next;
> +        pa_sink_volume_change_free(pc);
>       }
>       nc->next = NULL;
>       s->thread_info.volume_changes_tail = nc;
> diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
> index 2dd7a28..0e304c6 100644
> --- a/src/pulsecore/source.c
> +++ b/src/pulsecore/source.c
> @@ -2654,6 +2654,7 @@ static void pa_source_volume_change_free(pa_source_volume_change *c) {
>   void pa_source_volume_change_push(pa_source *s) {
>       pa_source_volume_change *c = NULL;
>       pa_source_volume_change *nc = NULL;
> +    pa_source_volume_change *pc = NULL;
>       uint32_t safety_margin = s->thread_info.volume_change_safety_margin;
>
>       const char *direction = NULL;
> @@ -2711,9 +2712,12 @@ void pa_source_volume_change_push(pa_source *s) {
>       pa_log_debug("Volume going %s to %d at %llu", direction, pa_cvolume_avg(&nc->hw_volume), (long long unsigned) nc->at);
>
>       /* We can ignore volume events that came earlier but should happen later than this. */
> -    PA_LLIST_FOREACH(c, nc->next) {
> +    c = nc->next;
> +    while(c) {
>           pa_log_debug("Volume change to %d at %llu was dropped", pa_cvolume_avg(&c->hw_volume), (long long unsigned) c->at);
> -        pa_source_volume_change_free(c);
> +        pc = c;
> +        c = c->next;
> +        pa_source_volume_change_free(pc);
>       }
>       nc->next = NULL;
>       s->thread_info.volume_changes_tail = nc;
>

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic