On Mar 31, 2011, at 9:27 PM, Mathieu Desnoyers wrote:

* 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.


Sure, but this patch doesn't change the name of the function or its calling. I expect that would be a separate patch fixing consistency issues.

As far as strings are concerned though adding alloc and init is just confusing I think.

/Nils

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


_______________________________________________
ltt-dev mailing list
[email protected]
http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev

Reply via email to