api.c: fix deleting cgroup created on shared mnt point

cgroup v1 allows mounting of two or more controllers on a single
mount point.  This fails currently when trying to delete the cgroup
recursively for all the mounted controllers because the cgroup is
already deleted by the first controller of the share mount point and
the next controllers sharing the mount point complain about
missing cgroup path.

Fix this issue by introducing shared_mnt flag to cg_mount_table_s
struct and setting it during cgroup_init() on finding a mount point
shared by controllers and repeats the check in cgroup_find_parent()
on the error to check its share mount point and ignore the error if the
file doesn't exist, similar to ignoring if the file exists during
creation.

Fixes: https://github.com/libcgroup/libcgroup/issues/127
Reported-by: Vijayendra Lakkundi <vijayendra.lakkundi@oracle.com>
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>

diff --git a/src/api.c b/src/api.c
index 7f2443156204ebb837cdb61c43a9a25e4065f1c1..8ba4a952f50be684c1c484801e78c2cb12b7a3fa 100644 (file)
--- a/src/api.c
+++ b/src/api.c
@@ -1073,11 +1073,34 @@ int cg_add_duplicate_mount(struct cg_mount_table_s *item, const char *path)
        return 0;
 }
 
+/*
+ * Tries to find if any controller in cg_mount_table have already mounted on
+ * the mount_path and if mounted sets the matching controller idx share_mnt
+ * flag and Return 1 or 0 otherwise.
+ */
+static int cgroup_set_cg_mnt_tbl_shared_mnt(char *mount_path, int *mnt_tbl_idx)
+{
+       int i, shared_mnt = 0;
+
+       /* Check if controllers share mount points */
+       for  (i = 0; i < *mnt_tbl_idx; i++) {
+               if (strncmp(mount_path, cg_mount_table[i].mount.path,
+                           FILENAME_MAX) == 0) {
+                       cg_mount_table[i].shared_mnt = 1;
+                       shared_mnt = 1;
+                       break;
+               }
+       }
+
+       return shared_mnt;
+}
+
 static void cgroup_cg_mount_table_append(const char *name,
                                         const char *mount_path,
                                         enum cg_version_t version,
                                         int *mnt_tbl_idx,
-                                        const char *mnt_opts)
+                                        const char *mnt_opts,
+                                        int shared_mnt)
 {
        int i = *mnt_tbl_idx;
 
@@ -1087,8 +1110,10 @@ static void cgroup_cg_mount_table_append(const char *name,
        strncpy(cg_mount_table[i].mount.path, mount_path, FILENAME_MAX);
        cg_mount_table[i].mount.path[FILENAME_MAX-1] = '\0';
 
+       cg_mount_table[i].shared_mnt = shared_mnt;
        cg_mount_table[i].version = version;
        cg_mount_table[i].mount.next = NULL;
+
        cgroup_dbg("Found cgroup option %s, count %d\n", mnt_opts, i);
 
        (*mnt_tbl_idx)++;
@@ -1106,7 +1131,7 @@ STATIC int cgroup_process_v1_mnt(char *controllers[], struct mntent *ent,
                                 int *mnt_tbl_idx)
 {
        char *strtok_buffer = NULL, *mntopt = NULL;
-       int duplicate = 0;
+       int shared_mnt, duplicate;
        int i, j, ret = 0;
 
        for (i = 0; controllers[i] != NULL; i++) {
@@ -1117,6 +1142,10 @@ STATIC int cgroup_process_v1_mnt(char *controllers[], struct mntent *ent,
 
                cgroup_dbg("found %s in %s\n", controllers[i], ent->mnt_opts);
 
+               /* Check if controllers share mount points */
+               shared_mnt = cgroup_set_cg_mnt_tbl_shared_mnt(ent->mnt_dir,
+                                                             mnt_tbl_idx);
+
                /* Do not have duplicates in mount table */
                duplicate = 0;
                for  (j = 0; j < *mnt_tbl_idx; j++) {
@@ -1140,7 +1169,7 @@ STATIC int cgroup_process_v1_mnt(char *controllers[], struct mntent *ent,
 
                cgroup_cg_mount_table_append(controllers[i], ent->mnt_dir,
                                             CGROUP_V1, mnt_tbl_idx,
-                                            ent->mnt_opts);
+                                            ent->mnt_opts, shared_mnt);
        }
 
        /*
@@ -1153,10 +1182,6 @@ STATIC int cgroup_process_v1_mnt(char *controllers[], struct mntent *ent,
                mntopt = strtok_r(mntopt, ",", &strtok_buffer);
                if (!mntopt)
                        goto out;
-               /*
-                * Check if it is a duplicate
-                */
-               duplicate = 0;
 
 #ifdef OPAQUE_HIERARCHY
                /*
@@ -1165,7 +1190,12 @@ STATIC int cgroup_process_v1_mnt(char *controllers[], struct mntent *ent,
                if (strcmp(mntopt, OPAQUE_HIERARCHY) == 0)
                        goto out;
 #endif
+               /* Check if controllers share mount points */
+               shared_mnt = cgroup_set_cg_mnt_tbl_shared_mnt(ent->mnt_dir,
+                                                             mnt_tbl_idx);
 
+               /* Check if it is a duplicate */
+               duplicate = 0;
                for (j = 0; j < *mnt_tbl_idx; j++) {
                        if (strncmp(mntopt, cg_mount_table[j].name,
                                    FILENAME_MAX) == 0) {
@@ -1184,7 +1214,7 @@ STATIC int cgroup_process_v1_mnt(char *controllers[], struct mntent *ent,
 
                cgroup_cg_mount_table_append(mntopt, ent->mnt_dir,
                                             CGROUP_V1, mnt_tbl_idx,
-                                            ent->mnt_opts);
+                                            ent->mnt_opts, shared_mnt);
        }
 
 out:
@@ -1202,7 +1232,7 @@ STATIC int cgroup_process_v2_mnt(struct mntent *ent, int *mnt_tbl_idx)
 {
        char cgroup_controllers_path[FILENAME_MAX];
        char *ret_c = NULL, line[LL_MAX], *stok_buff = NULL, *controller;
-       int ret = 0, i, duplicate;
+       int ret = 0, i, duplicate, shared_mnt;
        FILE *fp = NULL;
 
        /*
@@ -1263,9 +1293,12 @@ STATIC int cgroup_process_v2_mnt(struct mntent *ent, int *mnt_tbl_idx)
         */
        controller = strtok_r(ret_c, " ", &stok_buff);
        do {
+               /* Check if controllers share mount points */
+               shared_mnt = cgroup_set_cg_mnt_tbl_shared_mnt(ent->mnt_dir,
+                                                             mnt_tbl_idx);
+
                /* Do not have duplicates in mount table */
                duplicate = 0;
-
                for  (i = 0; i < *mnt_tbl_idx; i++) {
                        if (strncmp(cg_mount_table[i].name, controller,
                                    FILENAME_MAX) == 0) {
@@ -1289,7 +1322,7 @@ STATIC int cgroup_process_v2_mnt(struct mntent *ent, int *mnt_tbl_idx)
                /* This controller is not in the mount table.  add it */
                cgroup_cg_mount_table_append(controller, ent->mnt_dir,
                                             CGROUP_V2, mnt_tbl_idx,
-                                            controller);
+                                            controller, shared_mnt);
        } while ((controller = strtok_r(NULL, " ", &stok_buff)));
 
 out:
@@ -2759,6 +2792,31 @@ static int cgroup_get_parent_name(struct cgroup *cgroup, char **parent)
        return ret;
 }
 
+/*
+ * Checks if the cgroup's controller shares the mount point with any other
+ * controller in cg_mount_table. Returns 1 if shared or 0.
+ */
+static int is_cgrp_ctrl_shared_mnt(char *controller)
+{
+       int ret = 0;
+       int i;
+
+       pthread_rwlock_rdlock(&cg_mount_table_lock);
+
+       for (i = 0; cg_mount_table[i].name[0] != '\0'; i++) {
+
+               if (strncmp(cg_mount_table[i].name, controller,
+                           sizeof(cg_mount_table[i].name)) == 0 &&
+                   cg_mount_table[i].shared_mnt) {
+                       ret = 1;
+                       break;
+               }
+       }
+
+       pthread_rwlock_unlock(&cg_mount_table_lock);
+
+       return ret;
+}
 /**
  * Find the parent of the specified directory. It returns the parent in
  * hierarchy of given controller (the parent is usually name/.. unless name is
@@ -2800,6 +2858,10 @@ static int cgroup_find_parent(struct cgroup *cgroup, char *controller,
        cgroup_dbg("parent's name is %s\n", parent_path);
 
        if (stat(child_path, &stat_child) < 0) {
+               if (is_cgrp_ctrl_shared_mnt(controller)) {
+                       ret = ECGROUPNOTEXIST;
+                       goto free_parent;
+               }
                last_errno = errno;
                ret = ECGOTHER;
                goto free_parent;
@@ -3142,7 +3204,7 @@ int cgroup_delete_cgroup_ext(struct cgroup *cgroup, int flags)
                        ret = cgroup_find_parent(cgroup, controller_name,
                                                 &parent_name);
                        if (ret) {
-                               if (first_error == 0) {
+                               if (first_error == 0 && ret != ECGROUPNOTEXIST) {
                                        first_errno = last_errno;
                                        first_error = ret;
                                }

diff --git a/src/libcgroup-internal.h b/src/libcgroup-internal.h
index 1a542ba45f369cffc28b452a7db693c25fddd7e5..a138eab10b1bffc6428a8cfd6cbf56ab772ef2f1 100644 (file)
--- a/src/libcgroup-internal.h
+++ b/src/libcgroup-internal.h
@@ -132,6 +132,7 @@ struct cg_mount_table_s {
         */
        struct cg_mount_point mount;
        int index;
+       int shared_mnt;
        enum cg_version_t version;
 };