Ken'ichi Ohmichi wrote:
> Hi,
>
> There are some similar functions for getting process's data (uid,
> gid) from /proc/<pid>/status file, so this patch integrates these
> functions into one cgroup_get_procdata_from_status().
> In addition, cgroup_get_procdata_from_status() can get also a
> process name from /proc/<pid>/status file.
>
>
> Thanks
> Ken'ichi Ohmichi
>
> Signed-off-by: Ken'ichi Ohmichi <[email protected]>
> ---
> include/libcgroup.h | 13 +++++++
> src/api.c | 55 +++++++++++++++++++++++++++++
> src/daemon/cgrulesengd.c | 48 +------------------------
> src/libcgroup.map | 1 +
> src/tools/cgclassify.c | 86 +--------------------------------------------
> 5 files changed, 73 insertions(+), 130 deletions(-)
>
> diff --git a/include/libcgroup.h b/include/libcgroup.h
> index 085c17a..7db02d4 100644
> --- a/include/libcgroup.h
> +++ b/include/libcgroup.h
> @@ -285,6 +285,19 @@ int cgroup_get_task_begin(char *cgroup, char
> *controller, void **handle,
> pid_t *pid);
>
> /**
> + * Get process data (euid, egid, and process name) from /proc/<pid>status
> + * file.
> + * @param pid: The process id
> + * @param euid: If this is NULL, the euid is not taken
> + * @param egid: If this is NULL, the egid is not taken
> + * @param procname: If this is NULL, the process name is not taken
> + * @param size_procname: The buffer size of procname
> + * @return 0 on success, > 0 on error.
> + */
> +int cgroup_get_procdata_from_status(pid_t pid, uid_t *euid, gid_t *egid,
> + char *procname, size_t size_procname);
> +
I am not sure whether this function should be public. It has nothing to
do with cgroups, it's helper function of tools/daemons. What do others say?
> +/**
> * Read the next task value
> * @handle: The handle used for iterating
> * @pid: The variable where the value will be stored
> diff --git a/src/api.c b/src/api.c
> index 9da3ebb..0739642 100644
> --- a/src/api.c
> +++ b/src/api.c
> @@ -2477,3 +2477,58 @@ int cgroup_get_task_begin(char *cgroup, char
> *controller, void **handle,
>
> return ret;
> }
> +
> +int cgroup_get_procdata_from_status(pid_t pid, uid_t *euid, gid_t *egid,
> + char *procname, size_t size_procname)
> +{
> + FILE *f;
> + char path[FILENAME_MAX];
> + char buf[4092];
> + uid_t ruid, suid, fsuid;
> + gid_t rgid, sgid, fsgid;
> + bool find_euid = false;
> + bool find_egid = false;
> + bool find_procname = false;
> +
> + if (euid)
> + find_euid = true;
> + if (egid)
> + find_egid = true;
> + if (procname)
> + find_procname = true;
> +
> + sprintf(path, "/proc/%d/status", pid);
> + f = fopen(path, "r");
> + if (!f)
> + return 1;
> +
> + memset(buf, '\0', sizeof(buf));
Is the memset necessary?
> + while (fgets(buf, sizeof(buf), f)) {
> + if (!strncmp(buf, "Uid:", 4) && find_euid) {
> + sscanf((buf + 5), "%d%d%d%d", &ruid, euid,
> + &suid, &fsuid);
How stable is this /proc file format? Should there be a check that
sscanf succeeded?
> + cgroup_dbg("Scanned proc values are %d %d %d %d\n",
> + ruid, *euid, suid, fsuid);
> + find_euid = false;
> + } else if (!strncmp(buf, "Gid:", 4) && find_egid) {
dtto, check sscanf return value?
> + sscanf((buf + 5), "%d%d%d%d", &rgid, egid,
> + &sgid, &fsgid);
> + cgroup_dbg("Scanned proc values are %d %d %d %d\n",
> + rgid, *egid, sgid, fsgid);
> + find_egid = false;
> + } else if (!strncmp(buf, "Name:", 5) && find_procname &&
> + (strlen(buf + 6) < size_procname)) {
If the size_procname is not big enough, the function returns magic value
'1', without any other indication what's wrong. Should we introduce
something like 'ECG NOT ENOUGH MEMORY'? And return amount of needed
memory in size_procname (which then needs to be size_t*)??? Or use 'char
procname[PATH_MAX] instead?
> + sscanf((buf + 6), "%s", procname);
> + find_procname = false;
> + }
> + if (!find_euid && !find_egid && !find_procname) {
> + fclose(f);
> + return 0;
> + }
> + memset(buf, '\0', sizeof(buf));
> + }
> + /* failed to find some process data. */
> + fclose(f);
> + return 1;
should ECGFAIL or something more appropriate be returned?
> +}
> +
> diff --git a/src/daemon/cgrulesengd.c b/src/daemon/cgrulesengd.c
> index 0c18709..e85f3d5 100644
> --- a/src/daemon/cgrulesengd.c
> +++ b/src/daemon/cgrulesengd.c
> @@ -140,50 +140,6 @@ void flog(int level, const char *format, ...)
> }
> }
>
> -static int cgre_get_euid_egid_from_status(pid_t pid, uid_t *euid, gid_t
> *egid)
> -{
> - /* Handle for the /proc/PID/status file */
> - FILE *f;
> -
> - /* Path for /proc/PID/status file */
> - char path[FILENAME_MAX];
> -
> - /* Temporary buffer */
> - char buf[4092];
> -
> - /* UID data */
> - uid_t ruid, suid, fsuid;
> -
> - /* GID data */
> - gid_t rgid, sgid, fsgid;
> -
> - /*
> - * First, we need to open the /proc/PID/status file so that we can
> - * get the effective UID and GID for the process that we're working
> - * on. This process is probably not us, so we can't just call
> - * geteuid() or getegid().
> - */
> - sprintf(path, "/proc/%d/status", pid);
> - f = fopen(path, "r");
> - if (!f)
> - return 1;
> -
> - /* Have the eUID, need to find the eGID. */
> - memset(buf, '\0', sizeof(buf));
> - while (fgets(buf, sizeof(buf), f)) {
> - if (!strncmp(buf, "Uid:", 4)) {
> - sscanf((buf + 5), "%d%d%d%d", &ruid, euid,
> - &suid, &fsuid);
> - } else if (!strncmp(buf, "Gid:", 4)) {
> - sscanf((buf + 5), "%d%d%d%d", &rgid, egid,
> - &sgid, &fsgid);
> - }
> - memset(buf, '\0', sizeof(buf));
> - }
> - fclose(f);
> - return 0;
> -}
> -
> struct parent_info {
> __u64 timestamp;
> pid_t pid;
> @@ -323,8 +279,8 @@ int cgre_process_event(const struct proc_event *ev, const
> int type)
> default:
> break;
> }
> - if (cgre_get_euid_egid_from_status(pid, &euid, &egid))
> - /* cgre_get_euid_egid_from_status() returns 1 if it fails to
> + if (cgroup_get_procdata_from_status(pid, &euid, &egid, NULL, 0))
> + /* cgroup_get_procdata_from_status() returns 1 if it fails to
> * open /proc/<pid>/status file and that is not a problem. */
> return 0;
>
> diff --git a/src/libcgroup.map b/src/libcgroup.map
> index 4b95daa..b8eff4e 100644
> --- a/src/libcgroup.map
> +++ b/src/libcgroup.map
> @@ -62,4 +62,5 @@ global:
> cgroup_read_stats_begin;
> cgroup_read_stats_next;
> cgroup_read_stats_end;
> + cgroup_get_procdata_from_status;
> } CGROUP_0.33;
> diff --git a/src/tools/cgclassify.c b/src/tools/cgclassify.c
> index de10510..f7d32ff 100644
> --- a/src/tools/cgclassify.c
> +++ b/src/tools/cgclassify.c
> @@ -31,80 +31,6 @@
> #define TEMP_BUF 81
>
> /*
> - * Go through /proc/<pid>/status file to determine the euid of the
> - * process.
> - * It returns 0 on success and negative values on failure.
> - */
> -
> -int euid_of_pid(pid_t pid)
> -{
> - FILE *fp;
> - char path[FILENAME_MAX];
> - char buf[TEMP_BUF];
> - uid_t ruid, euid, suid, fsuid;
> -
> - sprintf(path, "/proc/%d/status", pid);
> - fp = fopen(path, "r");
> - if (!fp) {
> - cgroup_dbg("Error in opening file %s:%s\n", path,
> - strerror(errno));
> - return -1;
> - }
> -
> - while (fgets(buf, TEMP_BUF, fp)) {
> - if (!strncmp(buf, "Uid:", 4)) {
> - sscanf((buf + 5), "%d%d%d%d", (int *)&ruid,
> - (int *)&euid, (int *)&suid, (int *)&fsuid);
> - cgroup_dbg("Scanned proc values are %d %d %d %d\n",
> - ruid, euid, suid, fsuid);
> - fclose(fp);
> - return euid;
> - }
> - }
> - fclose(fp);
> -
> - /* If we are here, we could not find euid. Return error. */
> - return -1;
> -}
> -
> -/*
> - * Go through /proc/<pid>/status file to determine the egid of the
> - * process.
> - * It returns 0 on success and negative values on failure.
> - */
> -
> -int egid_of_pid(pid_t pid)
> -{
> - FILE *fp;
> - char path[FILENAME_MAX];
> - char buf[TEMP_BUF];
> - gid_t rgid, egid, sgid, fsgid;
> -
> - sprintf(path, "/proc/%d/status", pid);
> - fp = fopen(path, "r");
> - if (!fp) {
> - cgroup_dbg("Error in opening file %s:%s\n", path,
> - strerror(errno));
> - return -1;
> - }
> -
> - while (fgets(buf, TEMP_BUF, fp)) {
> - if (!strncmp(buf, "Gid:", 4)) {
> - sscanf((buf + 5), "%d%d%d%d", (int *)&rgid,
> - (int *)&egid, (int *)&sgid, (int *)&fsgid);
> - cgroup_dbg("Scanned proc values are %d %d %d %d\n",
> - rgid, egid, sgid, fsgid);
> - fclose(fp);
> - return egid;
> - }
> - }
> - fclose(fp);
> -
> - /* If we are here, we could not find egid. Return error. */
> - return -1;
> -}
> -
> -/*
> * Change process group as specified on command line.
> */
> int change_group_path(pid_t pid, struct cgroup_group_spec *cgroup_list[])
> @@ -137,16 +63,8 @@ int change_group_uid_gid(pid_t pid)
> int ret;
>
> /* Put pid into right cgroup as per rules in /etc/cgrules.conf */
> - euid = euid_of_pid(pid);
> - if (euid == -1) {
> - fprintf(stderr, "Error in determining euid of"
> - " pid %d\n", pid);
> - return -1;
> - }
> -
> - egid = egid_of_pid(pid);
> - if (egid == -1) {
> - fprintf(stderr, "Error in determining egid of"
> + if (cgroup_get_procdata_from_status(pid, &euid, &egid, NULL, 0)) {
> + fprintf(stderr, "Error in determining euid/egid of"
> " pid %d\n", pid);
> return -1;
> }
>
> ------------------------------------------------------------------------------
> Register Now for Creativity and Technology (CaT), June 3rd, NYC. CaT
> is a gathering of tech-side developers & brand creativity professionals. Meet
> the minds behind Google Creative Lab, Visual Complexity, Processing, &
> iPhoneDevCamp asthey present alongside digital heavyweights like Barbarian
> Group, R/GA, & Big Spaceship. http://www.creativitycat.com
> _______________________________________________
> Libcg-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/libcg-devel
------------------------------------------------------------------------------
Register Now for Creativity and Technology (CaT), June 3rd, NYC. CaT
is a gathering of tech-side developers & brand creativity professionals. Meet
the minds behind Google Creative Lab, Visual Complexity, Processing, &
iPhoneDevCamp as they present alongside digital heavyweights like Barbarian
Group, R/GA, & Big Spaceship. http://p.sf.net/sfu/creativitycat-com
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel
