[lttng-dev] [PATCH lttng-ust 2/2] Use ust_lock on fd tracker add operation for atomicity against fork

[Mathieu Desnoyers]

----- On Feb 1, 2018, at 5:28 PM, Jonathan Rajotte jonathan.rajotte-julien at efficios.com wrote:

> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
> liblttng-ust/lttng-ust-comm.c | 8 ++++++++
> liblttng-ust/lttng-ust-elf.c  | 8 ++++++++
> 2 files changed, 16 insertions(+)
> 
> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
> index 32ba3c1..f4b7b53 100644
> --- a/liblttng-ust/lttng-ust-comm.c
> +++ b/liblttng-ust/lttng-ust-comm.c
> @@ -1255,10 +1255,16 @@ char *get_map_shm(struct sock_info *sock_info)
> 		goto error;
> 	}
> 
> +	if (ust_lock()) {
> +		ust_unlock();
> +		goto error;
> +	}

get_map_shm() is called from a context that already holds the ust_lock().

This is a double-lock...


> +
> 	lttng_ust_lock_fd_tracker();
> 	wait_shm_fd = get_wait_shm(sock_info, page_size);
> 	if (wait_shm_fd < 0) {
> 		lttng_ust_unlock_fd_tracker();
> +		ust_unlock();
> 		goto error;
> 	}
> 
> @@ -1269,11 +1275,13 @@ char *get_map_shm(struct sock_info *sock_info)
> 			PERROR("Error closing fd");
> 		}
> 		lttng_ust_unlock_fd_tracker();
> +		ust_unlock();
> 		goto error;
> 	}
> 
> 	wait_shm_fd = ret;
> 	lttng_ust_unlock_fd_tracker();
> +	ust_unlock();
> 
> 	wait_shm_mmap = mmap(NULL, page_size, PROT_READ,
> 		  MAP_SHARED, wait_shm_fd, 0);
> diff --git a/liblttng-ust/lttng-ust-elf.c b/liblttng-ust/lttng-ust-elf.c
> index c073e7a..03c715a 100644
> --- a/liblttng-ust/lttng-ust-elf.c
> +++ b/liblttng-ust/lttng-ust-elf.c
> @@ -256,10 +256,16 @@ struct lttng_ust_elf *lttng_ust_elf_create(const char
> *path)
> 		goto error;
> 	}
> 
> +	if (ust_lock()) {
> +		ust_unlock();
> +		goto error;
> +	}

same here. iter_begin in liblttng-ust/lttng-ust-statedump.c already grabs the
ust lock. This won't work.

I wonder if any part of this patch is needed ?

Thanks,

Mathieu

> +
> 	lttng_ust_lock_fd_tracker();
> 	fd = open(elf->path, O_RDONLY | O_CLOEXEC);
> 	if (fd < 0) {
> 		lttng_ust_unlock_fd_tracker();
> +		ust_unlock();
> 		goto error;
> 	}
> 
> @@ -271,10 +277,12 @@ struct lttng_ust_elf *lttng_ust_elf_create(const char
> *path)
> 		}
> 		ret = -1;
> 		lttng_ust_unlock_fd_tracker();
> +		ust_unlock();
> 		goto error;
> 	}
> 	elf->fd = ret;
> 	lttng_ust_unlock_fd_tracker();
> +	ust_unlock();
> 
> 	if (lttng_ust_read(elf->fd, e_ident, EI_NIDENT) < EI_NIDENT) {
> 		goto error;
> --
> 2.7.4

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