[ltt-dev] [UST PATCH 1/6] Make app socket directories per-user v2

[Mathieu Desnoyers]

* Nils Carlson (nils.carlson at ericsson.com) wrote:
> Changes since v1:
> 	* Document memory allocation
> 
> Make a separate app socket directories for each user, providing
> some basic security and also the possibility of consistent cleanup.
> 
> Signed-off-by: Nils Carlson <nils.carlson at ericsson.com>
> ---
>  libust/tracectl.c    |   34 +++++++++++++++++++---------------
>  libustcomm/ustcomm.c |   34 +++++++++++++++++++++++++++++-----
>  libustcomm/ustcomm.h |    4 ++++
>  3 files changed, 52 insertions(+), 20 deletions(-)
> 
> diff --git a/libust/tracectl.c b/libust/tracectl.c
> index 33c7280..ae92b7e 100644
> --- a/libust/tracectl.c
> +++ b/libust/tracectl.c
> @@ -1221,37 +1221,41 @@ static void auto_probe_connect(struct marker *m)
>  
>  static struct ustcomm_sock * init_app_socket(int epoll_fd)

Hi Nils,

I think we really need to get a straight naming scheme for init vs alloc
functions.

init = initialization of a structure (never allocates)
alloc = allocation of a structure (maybe can also initialize it, but not
necessarily. This would be specified by the function header
documentation)

Otherwise reviewing correct allocation and free of a patch like this one
is made much more difficult than it should, and the problem will only
grow as we add more code.

Thank you,

Mathieu

>  {
> -	char *name;
> +	char *dir_name, *sock_name;
>  	int result;
> -	struct ustcomm_sock *sock;
> +	struct ustcomm_sock *sock = NULL;
>  
> -	result = asprintf(&name, "%s/%d", SOCK_DIR, (int)getpid());
> +	dir_name = ustcomm_user_sock_dir();
> +	if (!dir_name)
> +		return NULL;
> +
> +	result = asprintf(&sock_name, "%s/%d", dir_name, (int)getpid());
>  	if (result < 0) {
>  		ERR("string overflow allocating socket name, "
>  		    "UST thread bailing");
> -		return NULL;
> +		goto free_dir_name;
>  	}
>  
> -	result = ensure_dir_exists(SOCK_DIR);
> +	result = ensure_dir_exists(dir_name);
>  	if (result == -1) {
>  		ERR("Unable to create socket directory %s, UST thread bailing",
> -		    SOCK_DIR);
> -		goto free_name;
> +		    dir_name);
> +		goto free_sock_name;
>  	}
>  
> -	sock = ustcomm_init_named_socket(name, epoll_fd);
> +	sock = ustcomm_init_named_socket(sock_name, epoll_fd);
>  	if (!sock) {
>  		ERR("Error initializing named socket (%s). Check that directory"
> -		    "exists and that it is writable. UST thread bailing", name);
> -		goto free_name;
> +		    "exists and that it is writable. UST thread bailing", sock_name);
> +		goto free_sock_name;
>  	}
>  
> -	free(name);
> -	return sock;
> +free_sock_name:
> +	free(sock_name);
> +free_dir_name:
> +	free(dir_name);
>  
> -free_name:
> -	free(name);
> -	return NULL;
> +	return sock;
>  }
>  
>  static void __attribute__((constructor)) init()
> diff --git a/libustcomm/ustcomm.c b/libustcomm/ustcomm.c
> index 43f4289..dce1e52 100644
> --- a/libustcomm/ustcomm.c
> +++ b/libustcomm/ustcomm.c
> @@ -533,6 +533,21 @@ close_sock:
>  	return -1;
>  }
>  
> +/* Returns the current users socket directory, must be freed */
> +char *ustcomm_user_sock_dir(void)
> +{
> +	int result;
> +	char *sock_dir = NULL;
> +
> +	result = asprintf(&sock_dir, "%s%s", USER_SOCK_DIR,
> +			  cuserid(NULL));
> +	if (result < 0) {
> +		ERR("string overflow allocating directory name");
> +		return NULL;
> +	}
> +
> +	return sock_dir;
> +}
>  
>  /* Open a connection to a traceable app.
>   *
> @@ -545,21 +560,30 @@ int ustcomm_connect_app(pid_t pid, int *app_fd)
>  {
>  	int result;
>  	int retval = 0;
> -	char *name;
> +	char *dir_name, *sock_name;
> +
> +	dir_name = ustcomm_user_sock_dir();
> +	if (!dir_name)
> +		return -ENOMEM;
>  
> -	result = asprintf(&name, "%s/%d", SOCK_DIR, pid);
> +	result = asprintf(&sock_name, "%s/%d", dir_name, pid);
>  	if (result < 0) {
>  		ERR("failed to allocate socket name");
> -		return -1;
> +		retval = -1;
> +		goto free_dir_name;
>  	}
>  
> -	result = ustcomm_connect_path(name, app_fd);
> +	result = ustcomm_connect_path(sock_name, app_fd);
>  	if (result < 0) {
>  		ERR("failed to connect to app");
>  		retval = -1;
> +		goto free_sock_name;
>  	}
>  
> -	free(name);
> +free_sock_name:
> +	free(sock_name);
> +free_dir_name:
> +	free(dir_name);
>  
>  	return retval;
>  }
> diff --git a/libustcomm/ustcomm.h b/libustcomm/ustcomm.h
> index 0ec04fc..db38119 100644
> --- a/libustcomm/ustcomm.h
> +++ b/libustcomm/ustcomm.h
> @@ -25,6 +25,7 @@
>  #include <ust/kcompat/kcompat.h>
>  
>  #define SOCK_DIR "/tmp/ust-app-socks"
> +#define USER_SOCK_DIR "/tmp/ust-socks-"
>  
>  struct ustcomm_sock {
>  	struct cds_list_head list;
> @@ -156,6 +157,9 @@ extern int ustcomm_req(int sock,
>  		       char *res_data);
>  
>  extern int ustcomm_request_consumer(pid_t pid, const char *channel);
> +
> +/* Returns the current users socket directory, must be freed */
> +extern char *ustcomm_user_sock_dir(void);
>  extern int ustcomm_connect_app(pid_t pid, int *app_fd);
>  extern int ustcomm_connect_path(const char *path, int *connection_fd);
>  
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com