mirror of
				https://gitlab.freedesktop.org/pipewire/pipewire.git
				synced 2025-11-03 09:01:54 -05:00 
			
		
		
		
	Revert "loop: remove destroy list"
This reverts commit c474846c42.
In addition, `s->loop` is also checked before dispatching a source.
The destroy list is needed in the presence of threads. The
issue is that a source may be destroyed between `epoll_wait()`
returning and thread loop lock being acquired. If this
source is active, then a use-after-free will be triggered
when the thread loop acquires the lock and starts dispatching
the sources.
  thread 1                       thread 2
 ----------                     ----------
                                loop_iterate
                                  spa_loop_control_hook_before
                                    // release lock
 pw_thread_loop_lock
                                  spa_system_pollfd_wait
                                    // assume it returns with source A
 pw_loop_destroy_source(..., A)
  // frees storage of A
 pw_thread_loop_unlock
                                  spa_loop_control_hook_after
                                    // acquire the lock
                                  for (...) {
                                    struct spa_source *s = ep[i].data;
                                    s->rmask = ep[i].events;
                                      // use-after-free if `s` refers to
                                      // the previously freed `A`
Fixes #2147
			
			
This commit is contained in:
		
							parent
							
								
									bb1cd23a1a
								
							
						
					
					
						commit
						16f63a3c8f
					
				
					 1 changed files with 17 additions and 2 deletions
				
			
		| 
						 | 
					@ -75,6 +75,7 @@ struct impl {
 | 
				
			||||||
        struct spa_system *system;
 | 
					        struct spa_system *system;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	struct spa_list source_list;
 | 
						struct spa_list source_list;
 | 
				
			||||||
 | 
						struct spa_list destroy_list;
 | 
				
			||||||
	struct spa_hook_list hooks_list;
 | 
						struct spa_hook_list hooks_list;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	int poll_fd;
 | 
						int poll_fd;
 | 
				
			||||||
| 
						 | 
					@ -325,6 +326,14 @@ static void loop_leave(void *object)
 | 
				
			||||||
		impl->thread = 0;
 | 
							impl->thread = 0;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					static inline void process_destroy(struct impl *impl)
 | 
				
			||||||
 | 
					{
 | 
				
			||||||
 | 
						struct source_impl *source, *tmp;
 | 
				
			||||||
 | 
						spa_list_for_each_safe(source, tmp, &impl->destroy_list, link)
 | 
				
			||||||
 | 
							free(source);
 | 
				
			||||||
 | 
						spa_list_init(&impl->destroy_list);
 | 
				
			||||||
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static int loop_iterate(void *object, int timeout)
 | 
					static int loop_iterate(void *object, int timeout)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
	struct impl *impl = object;
 | 
						struct impl *impl = object;
 | 
				
			||||||
| 
						 | 
					@ -354,11 +363,14 @@ static int loop_iterate(void *object, int timeout)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	for (i = 0; i < nfds; i++) {
 | 
						for (i = 0; i < nfds; i++) {
 | 
				
			||||||
		struct spa_source *s = ep[i].data;
 | 
							struct spa_source *s = ep[i].data;
 | 
				
			||||||
		if (SPA_LIKELY(s && s->rmask)) {
 | 
							if (SPA_LIKELY(s && s->rmask && s->loop)) {
 | 
				
			||||||
			s->priv = NULL;
 | 
								s->priv = NULL;
 | 
				
			||||||
			s->func(s);
 | 
								s->func(s);
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
						if (SPA_UNLIKELY(!spa_list_is_empty(&impl->destroy_list)))
 | 
				
			||||||
 | 
							process_destroy(impl);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	return nfds;
 | 
						return nfds;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -712,7 +724,7 @@ static void loop_destroy_source(void *object, struct spa_source *source)
 | 
				
			||||||
		spa_system_close(impl->impl->system, source->fd);
 | 
							spa_system_close(impl->impl->system, source->fd);
 | 
				
			||||||
		source->fd = -1;
 | 
							source->fd = -1;
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	free(source);
 | 
						spa_list_insert(&impl->impl->destroy_list, &impl->link);
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static const struct spa_loop_methods impl_loop = {
 | 
					static const struct spa_loop_methods impl_loop = {
 | 
				
			||||||
| 
						 | 
					@ -783,6 +795,8 @@ static int impl_clear(struct spa_handle *handle)
 | 
				
			||||||
	spa_list_consume(source, &impl->source_list, link)
 | 
						spa_list_consume(source, &impl->source_list, link)
 | 
				
			||||||
		loop_destroy_source(impl, &source->source);
 | 
							loop_destroy_source(impl, &source->source);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						process_destroy(impl);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	spa_system_close(impl->system, impl->ack_fd);
 | 
						spa_system_close(impl->system, impl->ack_fd);
 | 
				
			||||||
	spa_system_close(impl->system, impl->poll_fd);
 | 
						spa_system_close(impl->system, impl->poll_fd);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -844,6 +858,7 @@ impl_init(const struct spa_handle_factory *factory,
 | 
				
			||||||
	impl->poll_fd = res;
 | 
						impl->poll_fd = res;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	spa_list_init(&impl->source_list);
 | 
						spa_list_init(&impl->source_list);
 | 
				
			||||||
 | 
						spa_list_init(&impl->destroy_list);
 | 
				
			||||||
	spa_hook_list_init(&impl->hooks_list);
 | 
						spa_hook_list_init(&impl->hooks_list);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	impl->buffer_data = SPA_PTR_ALIGN(impl->buffer_mem, MAX_ALIGN, uint8_t);
 | 
						impl->buffer_data = SPA_PTR_ALIGN(impl->buffer_mem, MAX_ALIGN, uint8_t);
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue