Ken'ichi Ohmichi wrote:
> Hi,
>
> This patch adds the parser of process name in /etc/cgrules.conf.
>
> A new rule based on process name is as the following, and the process
> name is stored into the member "procname" in struct cgroup_rule.
> <user>:<process name> <controllers> <destination>
>
>
> Thanks
> Ken'ichi Ohmichi
>
> Signed-off-by: Ken'ichi Ohmichi <[email protected]>
> ---
> include/libcgroup.h | 7 ++++++-
> src/api.c | 43 +++++++++++++++++++++++++++++++++++++++----
> src/libcgroup-internal.h | 1 +
> 3 files changed, 46 insertions(+), 5 deletions(-)
>
> diff --git a/include/libcgroup.h b/include/libcgroup.h
> index 7db02d4..10b41aa 100644
> --- a/include/libcgroup.h
> +++ b/include/libcgroup.h
> @@ -48,9 +48,14 @@ __BEGIN_DECLS
> * would require us to use a scanner/parser that can parse beyond ASCII
> */
>
> +/* Task command name length (from linux-2.6.29) */
> +#define TASK_COMM_LEN 16
> +
> +/* Maximum length of a key(<user>:<process name>) in the daemon config file
> */
> +#define CGROUP_RULE_MAXKEY (LOGIN_NAME_MAX + TASK_COMM_LEN + 1)
>
> /* Maximum length of a line in the daemon config file */
> -#define CGROUP_RULE_MAXLINE (FILENAME_MAX + LOGIN_NAME_MAX + \
> +#define CGROUP_RULE_MAXLINE (FILENAME_MAX + CGROUP_RULE_MAXKEY + \
> CG_CONTROLLER_MAX + 3)
>
> /* Definitions for the uid and gid members of a cgroup_rules */
> diff --git a/src/api.c b/src/api.c
> index 0739642..3e063cb 100644
> --- a/src/api.c
> +++ b/src/api.c
> @@ -206,7 +206,10 @@ static void cgroup_free_rule(struct cgroup_rule *r)
> cgroup_dbg("Warning: Attempted to free NULL rule.\n");
> return;
> }
> -
> + if (r->procname) {
> + free(r->procname);
> + r->procname = NULL;
> + }
> /* We must free any used controller strings, too. */
> for(i = 0; i < MAX_MNT_ELEMENTS; i++) {
> if (r->controllers[i])
> @@ -297,6 +300,9 @@ static int cgroup_parse_rules(bool cache, uid_t muid,
> gid_t mgid)
> /* Iterator for the line we're working on */
> char *itr = NULL;
>
> + /* Pointer to process name in a line of the configuration file */
> + char *procname = NULL;
> +
> /* Pointer to the list that we're using */
> struct cgroup_rule_list *lst = NULL;
>
> @@ -310,11 +316,14 @@ static int cgroup_parse_rules(bool cache, uid_t muid,
> gid_t mgid)
> struct passwd *pwd = NULL;
>
> /* Temporary storage for a configuration rule */
> + char key[CGROUP_RULE_MAXKEY] = { '\0' };
> char user[LOGIN_NAME_MAX] = { '\0' };
> char controllers[CG_CONTROLLER_MAX] = { '\0' };
> char destination[FILENAME_MAX] = { '\0' };
> uid_t uid = CGRULE_INVALID;
> gid_t gid = CGRULE_INVALID;
> + int len_username;
> + int len_procname;
>
> /* The current line number */
> unsigned int linenum = 0;
> @@ -381,16 +390,33 @@ static int cgroup_parse_rules(bool cache, uid_t muid,
> gid_t mgid)
> * there's an error in the configuration file.
> */
> skipped = false;
> + memset(key, '\0', sizeof(key));
> memset(user, '\0', sizeof(user));
> memset(controllers, '\0', sizeof(controllers));
> memset(destination, '\0', sizeof(destination));
Someone was obsessed with memset :)
> - i = sscanf(itr, "%s%s%s", user, controllers, destination);
> + i = sscanf(itr, "%s%s%s", key, controllers, destination);
Is this construction safe? Who guarantees the values fit to provided
buffers?
> if (i != 3) {
> cgroup_dbg("Failed to parse configuration file on"
> " line %d.\n", linenum);
> goto parsefail;
> }
>
> + if (strchr(key, ':')) {
> + /* <user>:<procname> <subsystem> <destination> */
> + procname = strchr(key, ':') + 1;
You call strchr(key, ':') twice. What about:
procname = strchr(key, ':');
if (procname) {
/* eventually put *procname='\0'; here, see below */
procname++; /* skip ':' */
...
> + len_username = procname - key - 1;
> + len_procname = strlen(key) - len_username - 1;
len_procname = strlen(procname)?
> + if (len_procname < 0) {
> + cgroup_dbg("Failed to parse configuration file
> on"
> + " line %d.\n", linenum);
> + goto parsefail;
> + }
> + } else {
> + len_username = strlen(key);
> + len_procname = 0;
> + }
> + strncpy(user, key, len_username);
Why is it copied? Won't char *user; user = key; work? Of course with ':'
replaced by '\0', as suggested above.
> +
> /*
> * Next, check the user/group. If it's a % sign, then we
> * are continuing another rule and UID/GID should not be
> @@ -479,7 +505,13 @@ static int cgroup_parse_rules(bool cache, uid_t muid,
> gid_t mgid)
>
> newrule->uid = uid;
> newrule->gid = gid;
> - strncpy(newrule->username, user, strlen(user));
> + strncpy(newrule->username, user, len_username);
> + if (len_procname) {
> + newrule->procname = calloc(1, len_procname + 1);
> + strncpy(newrule->procname, procname, len_procname);
strdup(procname)?
+ check for NULL (i.e. out of memory).
> + } else {
> + newrule->procname = NULL;
> + }
> strncpy(newrule->destination, destination, strlen(destination));
heh, I see the old code needs closer look... How did it pass Dhaval's
and Balbir's review? :)
> newrule->next = NULL;
>
> @@ -2008,7 +2040,10 @@ void cgroup_print_rules_config(FILE *fp)
>
> itr = rl.head;
> while (itr) {
> - fprintf(fp, "Rule: %s\n", itr->username);
> + fprintf(fp, "Rule: %s", itr->username);
> + if (itr->procname)
> + fprintf(fp, ":%s", itr->procname);
> + fprintf(fp, "\n");
>
> if (itr->uid == CGRULE_WILD)
> fprintf(fp, " UID: any\n");
> diff --git a/src/libcgroup-internal.h b/src/libcgroup-internal.h
> index 9e69f10..6f314b6 100644
> --- a/src/libcgroup-internal.h
> +++ b/src/libcgroup-internal.h
> @@ -74,6 +74,7 @@ struct cgroup_rules_data {
> struct cgroup_rule {
> uid_t uid;
> gid_t gid;
> + char *procname;
Should be the procname an array, like the other members? Or make the
other members allocated/freed as well? This state looks inconsistent to me.
> char username[LOGIN_NAME_MAX];
> char destination[FILENAME_MAX];
> char *controllers[MAX_MNT_ELEMENTS];
>
> ------------------------------------------------------------------------------
> 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
