* Nils Carlson ([email protected]) 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 <[email protected]>
> ---
> 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
> [email protected]
> 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
_______________________________________________
ltt-dev mailing list
[email protected]
http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
