----- On Nov 9, 2017, at 5:36 PM, Jonathan Rajotte 
[email protected] 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 <[email protected]>
> ---
> liblttng-ust/lttng-ust-elf.c | 34 +++++++++++++++++++++++++++++-----
> 1 file changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/liblttng-ust/lttng-ust-elf.c b/liblttng-ust/lttng-ust-elf.c
> index a496841a..5f3b280e 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
> @@ -242,6 +243,8 @@ struct lttng_ust_elf *lttng_ust_elf_create(const char 
> *path)
>       uint8_t e_ident[EI_NIDENT];
>       struct lttng_ust_elf_shdr *section_names_shdr;
>       struct lttng_ust_elf *elf = NULL;
> +     int fd;
> +     int ret;
> 
>       elf = zmalloc(sizeof(struct lttng_ust_elf));
>       if (!elf) {
> @@ -253,10 +256,16 @@ struct lttng_ust_elf *lttng_ust_elf_create(const char
> *path)
>               goto error;
>       }
> 
> -     elf->fd = open(elf->path, O_RDONLY | O_CLOEXEC);
> -     if (elf->fd < 0) {
> +     lttng_ust_lock_fd_tracker();
> +     fd = open(elf->path, O_RDONLY | O_CLOEXEC);
> +     if (fd < 0) {
> +             lttng_ust_unlock_fd_tracker();
>               goto error;
>       }
> +     lttng_ust_add_fd_to_tracker(fd);
> +     lttng_ust_unlock_fd_tracker();
> +
> +     elf->fd = fd;
> 
>       if (lttng_ust_read(elf->fd, e_ident, EI_NIDENT) < EI_NIDENT) {
>               goto error;
> @@ -312,9 +321,15 @@ error:
>       if (elf) {
>               free(elf->ehdr);
>               if (elf->fd >= 0) {
> -                     if (close(elf->fd)) {
> +                     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_lock_fd_tracker();

This appears to be a double-lock.

How did you test this code path ?

Thanks,

Mathieu


>               }
>               free(elf->path);
>               free(elf);
> @@ -339,14 +354,23 @@ 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)) {
> +     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
_______________________________________________
lttng-dev mailing list
[email protected]
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Reply via email to