[ltt-dev] [UST PATCH 5/6] Make root see all available pids

[Mathieu Desnoyers]

* Nils Carlson (nils.carlson at ericsson.com) wrote:
> Allow root (geteuid() == 0) to see all pids. This way the super-user
> can connect to any program. A step on the way of carefully outlining
> what UST does and doesn't.
> 
> Signed-off-by: Nils Carlson <nils.carlson at ericsson.com>
> ---
>  libustcomm/ustcomm.h  |    4 ++-
>  libustctl/libustctl.c |   61 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 63 insertions(+), 2 deletions(-)
> 
> diff --git a/libustcomm/ustcomm.h b/libustcomm/ustcomm.h
> index d16aec7..9952958 100644
> --- a/libustcomm/ustcomm.h
> +++ b/libustcomm/ustcomm.h
> @@ -25,7 +25,9 @@
>  #include <ust/kcompat/kcompat.h>
>  
>  #define SOCK_DIR "/tmp/ust-app-socks"
> -#define USER_SOCK_DIR "/tmp/ust-socks-"
> +#define USER_TMP_DIR "/tmp"
> +#define USER_SOCK_DIR_BASE "ust-socks-"
> +#define USER_SOCK_DIR USER_TMP_DIR "/" USER_SOCK_DIR_BASE
>  
>  struct ustcomm_sock {
>  	struct cds_list_head list;
> diff --git a/libustctl/libustctl.c b/libustctl/libustctl.c
> index 5625f43..083ded2 100644
> --- a/libustctl/libustctl.c
> +++ b/libustctl/libustctl.c
> @@ -106,6 +106,7 @@ static void get_pids_in_dir(DIR *dir, pid_t **pid_list,
>  		sscanf(dirent->d_name, "%u", &read_pid);
>  
>  		(*pid_list)[*pid_list_size - 1] = read_pid;
> +

Useless whiteline.

>  		/* FIXME: Here we previously called pid_is_online, which

Comment about the multi-line comment style: if we want to follow the
kernel style, we should go for:

/*
 * some text...
 * more text...
 */

(with an empty line after the first /*)

>  		 * always returned 1, now I replaced it with just 1.
>  		 * We need to figure out an intelligent way of solving
> @@ -121,7 +122,7 @@ static void get_pids_in_dir(DIR *dir, pid_t **pid_list,
>  	(*pid_list)[*pid_list_size - 1] = 0; /* Array end */
>  }
>  
> -pid_t *ustctl_get_online_pids(void)
> +pid_t *get_pids_non_root(void)

Why remove the "ustctl_" prefix here ? (for a non-static function ?)

>  {
>  	char *dir_name;
>  	DIR *dir;
> @@ -139,6 +140,9 @@ pid_t *ustctl_get_online_pids(void)
>  	}
>  
>  	pid_list = malloc(pid_list_size * sizeof(pid_t));
> +	if (!pid_list) {
> +		goto close_dir;
> +	}
>  
>  	get_pids_in_dir(dir, &pid_list, &pid_list_size);
>  
> @@ -158,6 +162,61 @@ free_dir_name:
>  	return pid_list;
>  }
>  
> +pid_t *get_pids_root(void)

Same question about prefix here.

> +{
> +	char *dir_name;
> +	DIR *tmp_dir, *dir;
> +	unsigned int pid_list_size = 1;
> +	pid_t *pid_list = NULL;
> +	struct dirent *dirent;
> +
> +	tmp_dir = opendir(USER_TMP_DIR);
> +	if (!tmp_dir) {
> +		return NULL;
> +	}
> +
> +	pid_list = malloc(pid_list_size * sizeof(pid_t));
> +	if (!pid_list) {
> +		goto close_tmp_dir;
> +	}
> +
> +	while ((dirent = readdir(tmp_dir))) {
> +		if (!strncmp(dirent->d_name, USER_SOCK_DIR_BASE,
> +			     strlen(USER_SOCK_DIR_BASE))) {

I think there is an off by one here. strlen does not include the final
\0, so if we have:

string 1:   "blahaaaa\0"
string 2:   "blah\0"

They will appear as identical although they are different.

Thanks,

Mathieu

> +
> +			if (asprintf(&dir_name, USER_TMP_DIR "/%s", dirent->d_name) < 0) {
> +				goto close_tmp_dir;
> +			}
> +
> +			dir = opendir(dir_name);
> +
> +			free(dir_name);
> +
> +			if (!dir) {
> +				continue;
> +			}
> +
> +			get_pids_in_dir(dir, &pid_list, &pid_list_size);
> +
> +			closedir(dir);
> +		}
> +	}
> +
> +close_tmp_dir:
> +	closedir(tmp_dir);
> +
> +	return pid_list;
> +}
> +
> +pid_t *ustctl_get_online_pids(void)
> +{
> +	if (geteuid()) {
> +		return get_pids_non_root();
> +	} else {
> +		return get_pids_root();
> +	}
> +}
> +
>  /**
>   * Sets marker state (USTCTL_MS_ON or USTCTL_MS_OFF).
>   *
> -- 
> 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