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

[Nils Carlson]

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

> * 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 /*)

can fix.
>
>> 		 * 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 ?)

good point, this will be static.
>
>> {
>> 	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.

should also be static.
>
>> +{
>> +	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.
>

No, this is the whole point. We only compare to see if we have a  
directory prefixed with "/tmp/ust-socks-", if we included the \0 this  
wouldn't work.

/Nils


> 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
>
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev