[lttng-dev] [PATCH lttng-ust v2] Fix: fd of an elf object must be registered to the fd tracker

[Mathieu Desnoyers]

Merged into master, 2.10, 2.9, thanks!

Mathieu

----- On Nov 10, 2017, at 11:06 AM, Jonathan Rajotte jonathan.rajotte-julien at efficios.com wrote:

> The open call take place inside ust, it must be tracked to prevent external
> closing.
> 
> The bug can be hit during tracing of an application for which the probe
> provider is loaded using LD_PRELOAD in combination with the fd utility
> shared object. The application is responsible for closing all possible fd.
> 
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
> 
> v2:
> Cleanup error path of lttng_ust_elf_create by using lttng_ust_elf_destroy.
> This take care of my previous mistake regarding double locking in the error
> path and move the cleanup logic to a single place.
> 
> lttng_ust_elf_destroy now handle cases where fd < 0.
> ---
> liblttng-ust/lttng-ust-elf.c | 34 +++++++++++++++++++++-------------
> 1 file changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/liblttng-ust/lttng-ust-elf.c b/liblttng-ust/lttng-ust-elf.c
> index a496841a..f2c09829 100644
> --- a/liblttng-ust/lttng-ust-elf.c
> +++ b/liblttng-ust/lttng-ust-elf.c
> @@ -27,6 +27,7 @@
> #include <fcntl.h>
> #include <unistd.h>
> #include <stdbool.h>
> +#include <ust-fd.h>
> #include "lttng-tracer-core.h"
> 
> #define BUF_LEN	4096
> @@ -248,15 +249,20 @@ struct lttng_ust_elf *lttng_ust_elf_create(const char
> *path)
> 		goto error;
> 	}
> 
> +
> 	elf->path = strdup(path);
> 	if (!elf->path) {
> 		goto error;
> 	}
> 
> +	lttng_ust_lock_fd_tracker();
> 	elf->fd = open(elf->path, O_RDONLY | O_CLOEXEC);
> 	if (elf->fd < 0) {
> +		lttng_ust_unlock_fd_tracker();
> 		goto error;
> 	}
> +	lttng_ust_add_fd_to_tracker(elf->fd);
> +	lttng_ust_unlock_fd_tracker();
> 
> 	if (lttng_ust_read(elf->fd, e_ident, EI_NIDENT) < EI_NIDENT) {
> 		goto error;
> @@ -309,16 +315,7 @@ struct lttng_ust_elf *lttng_ust_elf_create(const char
> *path)
> 	return elf;
> 
> error:
> -	if (elf) {
> -		free(elf->ehdr);
> -		if (elf->fd >= 0) {
> -			if (close(elf->fd)) {
> -				abort();
> -			}
> -		}
> -		free(elf->path);
> -		free(elf);
> -	}
> +	lttng_ust_elf_destroy(elf);
> 	return NULL;
> }
> 
> @@ -339,14 +336,25 @@ uint8_t lttng_ust_elf_is_pic(struct lttng_ust_elf *elf)
>  */
> void lttng_ust_elf_destroy(struct lttng_ust_elf *elf)
> {
> +	int ret;
> +
> 	if (!elf) {
> 		return;
> 	}
> 
> -	free(elf->ehdr);
> -	if (close(elf->fd)) {
> -		abort();
> +	if (elf->fd >= 0) {
> +		lttng_ust_lock_fd_tracker();
> +		ret = close(elf->fd);
> +		if (!ret) {
> +			lttng_ust_delete_fd_from_tracker(elf->fd);
> +		} else {
> +			PERROR("close");
> +			abort();
> +		}
> +		lttng_ust_unlock_fd_tracker();
> 	}
> +
> +	free(elf->ehdr);
> 	free(elf->path);
> 	free(elf);
> }
> --
> 2.11.0

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com