The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/libresource/pull/9

This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.

=== Description (from pull-request) ===
Added size field in interfaces which will check if enough memory is allocated to hold requested information.

Signed-off-by: Rahul Yadav <rahul.x.ya...@oracle.com>
From 5494ffe8ac8f49ce7bf0973aab0ca180c02ff466 Mon Sep 17 00:00:00 2001
From: Rahul Yadav <rahul.x.ya...@oracle.com>
Date: Mon, 10 Jun 2019 13:31:09 -0700
Subject: [PATCH] Added size field in interfaces which will check if enough
 memory is allocated to hold requested information.

Signed-off-by: Rahul Yadav <rahul.x.ya...@oracle.com>
---
 resmem.c   | 81 ++++++++++++++++++++++++++++++++++++++++++++++++------
 resmem.h   |  3 +-
 resnet.c   | 19 +++++++++++--
 resnet.h   |  3 +-
 resource.c | 38 ++++++++++++++++++-------
 resource.h |  7 +++--
 6 files changed, 126 insertions(+), 25 deletions(-)

diff --git a/resmem.c b/resmem.c
index 1cb036c..6cc3ba4 100644
--- a/resmem.c
+++ b/resmem.c
@@ -92,7 +92,7 @@ static size_t cgmemlimit(char *cg, char *f)
 
        dir = dirname(copy);
 
-       /*read memory limit for parant cg */
+       /*read memory limit for parent cg */
        if (strcmp(dir, "/") != 0) {
                if((mem = cgmemread(dir, f)) == 0) {
                        free(copy);
@@ -216,7 +216,7 @@ static int getmeminfoall(char *cg, void *out)
 }
 
 /* Read resource information corresponding to res_id */
-int getmeminfo(int res_id, void *out, void *hint, int pid, int flags)
+int getmeminfo(int res_id, void *out, size_t sz, void *hint, int pid, int 
flags)
 {
        char buf[MEMBUF_128];
        FILE *fp;
@@ -232,11 +232,22 @@ int getmeminfo(int res_id, void *out, void *hint, int 
pid, int flags)
                clean_init(cg);
        }
 
+#define CHECK_SIZE(sz, req_sz)                                         \
+       if (sz < req_sz) {                                              \
+               eprintf("memory (%ld) is not enough to hold data (%ld)",\
+               sz, req_sz);                                            \
+               errno = ENOMEM;                                         \
+               return -1;                                              \
+       }
+
+
        switch (res_id) {
                /* if process is part of a cgroup then return memory info
                 * for that cgroup only.
                 */
        case RES_MEM_FREE:
+               CHECK_SIZE(sz, sizeof(size_t));
+
                if (cg) {
                        mmusage = cgmemread(cg, "memory.usage_in_bytes");
                        if (get_info_infile(MEMINFO_FILE, "MemTotal:",
@@ -254,6 +265,8 @@ int getmeminfo(int res_id, void *out, void *hint, int pid, 
int flags)
                }
 
        case RES_MEM_AVAILABLE:
+               CHECK_SIZE(sz, sizeof(size_t));
+
                if (cg) {
                        mmusage = cgmemread(cg, "memory.usage_in_bytes");
                        if (get_info_infile(MEMINFO_FILE, "MemTotal:",
@@ -276,6 +289,8 @@ int getmeminfo(int res_id, void *out, void *hint, int pid, 
int flags)
                }
 
        case RES_MEM_TOTAL:
+               CHECK_SIZE(sz, sizeof(size_t));
+
                ret = get_info_infile(MEMINFO_FILE, "MemTotal:", out);
                if (cg) {
                        mmtot = cgmemlimit(cg, "memory.limit_in_bytes");
@@ -286,6 +301,8 @@ int getmeminfo(int res_id, void *out, void *hint, int pid, 
int flags)
                return ret;
 
        case RES_MEM_ACTIVE:
+               CHECK_SIZE(sz, sizeof(size_t));
+
                if (cg) {
                        active_anon = cgmemstat(cg, "\nactive_anon");
                        active_file = cgmemstat(cg, "\nactive_file");
@@ -296,6 +313,8 @@ int getmeminfo(int res_id, void *out, void *hint, int pid, 
int flags)
                }
 
        case RES_MEM_INACTIVE:
+               CHECK_SIZE(sz, sizeof(size_t));
+
                if (cg) {
                        inactive_anon = cgmemstat(cg, "\ninactive_anon");
                        inactive_file = cgmemstat(cg, "\ninactive_file");
@@ -306,6 +325,8 @@ int getmeminfo(int res_id, void *out, void *hint, int pid, 
int flags)
                }
 
        case RES_MEM_SWAPTOTAL:
+               CHECK_SIZE(sz, sizeof(size_t));
+
                ret = get_info_infile(MEMINFO_FILE, "SwapTotal:", out);
                if (cg) {
                        swtot = cgmemlimit(cg,
@@ -318,6 +339,8 @@ int getmeminfo(int res_id, void *out, void *hint, int pid, 
int flags)
                return ret;
 
        case RES_MEM_SWAPFREE:
+               CHECK_SIZE(sz, sizeof(size_t));
+
                if (cg) {
                        swusage = cgmemread(cg, "memory.memsw.usage_in_bytes");
                        swtot = cgmemlimit(cg,
@@ -346,6 +369,8 @@ int getmeminfo(int res_id, void *out, void *hint, int pid, 
int flags)
                return get_info_infile(MEMINFO_FILE, "SwapFree:", out);
 
        case RES_MEM_INFOALL:
+               CHECK_SIZE(sz, sizeof(res_mem_infoall_t));
+
                if (cg) {
                        if (getmeminfoall(cg, out) == -1) {
                                return -1;
@@ -386,6 +411,8 @@ int getmeminfo(int res_id, void *out, void *hint, int pid, 
int flags)
                break;
 
        case RES_MEM_PAGESIZE:
+               CHECK_SIZE(sz, sizeof(long));
+
                *(size_t *)out = sysconf(_SC_PAGESIZE);
                break;
 
@@ -395,6 +422,8 @@ int getmeminfo(int res_id, void *out, void *hint, int pid, 
int flags)
                return -1;
        }
 
+#undef CHECK_SIZE
+
        return 0;
 }
 
@@ -415,8 +444,19 @@ int populate_meminfo(res_blk_t *res, int pid, int flags)
        }
 
        if (file_to_buf(MEMINFO_FILE, buf, MEMBUF_2048) == -1) {
-               for (int i = 0 ; i < res->res_count; i++)
-                       res->res_unit[i]->status = errno;
+               for (int i = 0 ; i < res->res_count; i++) {
+                       switch (res->res_unit[i]->res_id) {
+                       case RES_MEM_FREE:
+                       case RES_MEM_AVAILABLE:
+                       case RES_MEM_TOTAL:
+                       case RES_MEM_ACTIVE:
+                       case RES_MEM_INACTIVE:
+                       case RES_MEM_SWAPTOTAL:
+                       case RES_MEM_SWAPFREE:
+                       case RES_MEM_INFOALL:
+                               res->res_unit[i]->status = errno;
+                       }
+               }
                return -1;
        }
 
@@ -435,11 +475,22 @@ int populate_meminfo(res_blk_t *res, int pid, int flags)
        } \
 } while (0)\
 
+/* Macro to check if enough memory is allocated to hold the data.
+ */
+#define CHECK_SIZE(sz, data_sz)                                                
\
+       if (sz < data_sz) {                                             \
+               eprintf("memory (%ld) is not enough to hold data (%ld)",\
+                        sz, data_sz);                                  \
+               res->res_unit[i]->status = ENOMEM;                      \
+               break;                                                  \
+       }
 
        for (int i = 0; i < res->res_count; i++) {
                loc = NULL;
                switch (res->res_unit[i]->res_id) {
                case RES_MEM_FREE:
+                       CHECK_SIZE(res->res_unit[i]->data_sz, sizeof(size_t));
+
                        if (cg) {
                                if (!mmusage)
                                        mmusage = cgmemread(cg,
@@ -462,6 +513,8 @@ int populate_meminfo(res_blk_t *res, int pid, int flags)
                        break;
 
                case RES_MEM_AVAILABLE:
+                       CHECK_SIZE(res->res_unit[i]->data_sz, sizeof(size_t));
+
                        if (cg) {
                                if (!mmusage)
                                        mmusage = cgmemread(cg,
@@ -486,6 +539,8 @@ int populate_meminfo(res_blk_t *res, int pid, int flags)
                        break;
 
                case RES_MEM_TOTAL:
+                       CHECK_SIZE(res->res_unit[i]->data_sz, sizeof(size_t));
+
                        if (!memtotal)
                                SCANMEMSTR("MemTotal:", memtotal);
                        else
@@ -502,6 +557,8 @@ int populate_meminfo(res_blk_t *res, int pid, int flags)
                        break;
 
                case RES_MEM_ACTIVE:
+                       CHECK_SIZE(res->res_unit[i]->data_sz, sizeof(size_t));
+
                        if (cg) {
                                active_anon = cgmemstat(cg, "\nactive_anon");
                                active_file = cgmemstat(cg, "\nactive_file");
@@ -514,6 +571,8 @@ int populate_meminfo(res_blk_t *res, int pid, int flags)
                        break;
 
                case RES_MEM_INACTIVE:
+                       CHECK_SIZE(res->res_unit[i]->data_sz, sizeof(size_t));
+
                        if (cg) {
                                inactive_anon = cgmemstat(cg,
                                        "\ninactive_anon");
@@ -528,6 +587,8 @@ int populate_meminfo(res_blk_t *res, int pid, int flags)
                        break;
 
                case RES_MEM_SWAPTOTAL:
+                       CHECK_SIZE(res->res_unit[i]->data_sz, sizeof(size_t));
+
                        if (!swaptotal)
                                SCANMEMSTR("SwapTotal:", swaptotal);
                        else
@@ -544,6 +605,8 @@ int populate_meminfo(res_blk_t *res, int pid, int flags)
                        break;
 
                case RES_MEM_SWAPFREE:
+                       CHECK_SIZE(res->res_unit[i]->data_sz, sizeof(size_t));
+
                        SCANMEMSTR("SwapFree:", swapfree);
 
                        if (cg) {
@@ -576,11 +639,10 @@ int populate_meminfo(res_blk_t *res, int pid, int flags)
                        }
                        break;
 
-               case RES_MEM_PAGESIZE:
-                       (res->res_unit[i]->data).sz = sysconf(_SC_PAGESIZE);
-                       break;
-
                case RES_MEM_INFOALL:
+                       CHECK_SIZE(res->res_unit[i]->data_sz,
+                               sizeof(res_mem_infoall_t));
+
                        if (cg) {
                                if (getmeminfoall(cg,
                                        (res->res_unit[i]->data).ptr) == -1) {
@@ -612,5 +674,8 @@ int populate_meminfo(res_blk_t *res, int pid, int flags)
                        break;
                }
        }
+
+#undef CHECK_SIZE
+
        return 0;
 }
diff --git a/resmem.h b/resmem.h
index 96f5a93..c630bd6 100644
--- a/resmem.h
+++ b/resmem.h
@@ -29,6 +29,7 @@
 #define MEMCGNAME      "memory"
 
 extern int populate_meminfo(struct res_blk *res, int pid, int flags);
-extern int getmeminfo(int res_id, void *out, void *hint, int pid, int flags);
+extern int getmeminfo(int res_id, void *out, size_t sz,
+               void *hint, int pid, int flags);
 
 #endif /* _RESMEM_H */
diff --git a/resnet.c b/resnet.c
index 3e8cce4..9559b5a 100644
--- a/resnet.c
+++ b/resnet.c
@@ -249,7 +249,7 @@ static inline int getallnetinfo(void *out, void *hint)
 }
 
 /* Get resource information related to network */
-int getnetinfo(int res_id, void *out, void *hint, int pid, int flags)
+int getnetinfo(int res_id, void *out, size_t sz, void *hint, int pid, int 
flags)
 {
        char buf[NETBUF_1024];
        FILE *fp;
@@ -258,12 +258,22 @@ int getnetinfo(int res_id, void *out, void *hint, int 
pid, int flags)
        int ver = 0;
        char *p;
 
+#define CHECK_SIZE(sz, req_sz)                                         \
+       if (sz < req_sz) {                                              \
+               eprintf("memory (%ld) is not enough to hold data (%ld)",\
+                       sz, req_sz);                                    \
+               errno = ENOMEM;                                         \
+               return -1;                                              \
+       }
+
        switch (res_id) {
        case RES_NET_IFSTAT:
+               CHECK_SIZE(sz, sizeof(res_net_ifstat_t));
+
                /* Interface name should be provided */
                if (hint == NULL) {
                        eprintf(
-                               "Interface name is not provided for 
RES_NET_IFSTAT");
+                               "Interface name is not provided");
                        errno = EINVAL;
                        return -1;
                }
@@ -319,6 +329,8 @@ int getnetinfo(int res_id, void *out, void *hint, int pid, 
int flags)
                return -1;
        }
 
+#undef CHECK_SIZE
+
        return 0;
 }
 
@@ -334,6 +346,7 @@ int populate_netinfo(res_blk_t *res, int pid, int flags)
                case RES_NET_IFSTAT:
                        if (getnetinfo(RES_NET_IFSTAT,
                                res->res_unit[i]->data.ptr,
+                               res->res_unit[i]->data_sz,
                                res->res_unit[i]->hint, 0, 0) == -1) {
                                res->res_unit[i]->status = errno;
                        } else
@@ -345,7 +358,7 @@ int populate_netinfo(res_blk_t *res, int pid, int flags)
                         */
                        res->res_unit[i]->hint = (int)0;
 
-                       if (getnetinfo(RES_NET_ALLIFSTAT, &p,
+                       if (getnetinfo(RES_NET_ALLIFSTAT, &p, 0,
                                &(res->res_unit[i]->hint), 0, 0) == -1) {
                                res->res_unit[i]->status = errno;
                        } else {
diff --git a/resnet.h b/resnet.h
index 83a3b41..2accbc3 100644
--- a/resnet.h
+++ b/resnet.h
@@ -25,6 +25,7 @@
 #define NETBUF_1024            1024
 
 extern int populate_netinfo(struct res_blk *res, int pid, int flags);
-extern int getnetinfo(int res_id, void *out, void *hint, int pid, int flags);
+extern int getnetinfo(int res_id, void *out, size_t sz, void *hint, int pid,
+               int flags);
 
 #endif /* _RESNET_H */
diff --git a/resource.c b/resource.c
index 030157b..52dc405 100644
--- a/resource.c
+++ b/resource.c
@@ -81,6 +81,7 @@ res_blk_t *res_build_blk(int *res_ids, int res_count)
                case RES_KERN_COMPILE_TIME:
                case RES_KERN_RELEASE:
                case RES_NET_ALLIFSTAT:
+                       temp->data_sz = sizeof(union r_data);
                        break;
 
                case RES_NET_IFSTAT:
@@ -92,6 +93,7 @@ res_blk_t *res_build_blk(int *res_ids, int res_count)
                                errno = ENOMEM;
                                return NULL;
                        }
+                       temp->data_sz = sizeof(res_net_ifstat_t);
                        break;
 
                case RES_MEM_INFOALL:
@@ -103,6 +105,7 @@ res_blk_t *res_build_blk(int *res_ids, int res_count)
                                errno = ENOMEM;
                                return NULL;
                        }
+                       temp->data_sz = sizeof(res_mem_infoall_t);
                        break;
 
                default:
@@ -144,7 +147,7 @@ void res_destroy_blk(res_blk_t *res)
 /* read resource information corresponding to res_id, out should have been
  * properly allocated by caller if required.
  */
-int res_read(int res_id, void *out, void *hint, int pid, int flags)
+int res_read(int res_id, void *out, size_t out_sz, void *hint, int pid, int 
flags)
 {
        struct utsname t;
        int ret;
@@ -168,11 +171,11 @@ int res_read(int res_id, void *out, void *hint, int pid, 
int flags)
 
        /* Check if memory proc file is needed to open */
        if (res_id >= MEM_MIN && res_id < MEM_MAX)
-               return getmeminfo(res_id, out, hint, pid, flags);
+               return getmeminfo(res_id, out, out_sz, hint, pid, flags);
 
        /* Check if net proc file is needed to open */
        if (res_id >= NET_MIN && res_id < NET_MAX)
-               return getnetinfo(res_id, out, hint, pid, flags);
+               return getnetinfo(res_id, out, out_sz, hint, pid, flags);
 
        switch (res_id) {
        case RES_KERN_RELEASE:
@@ -183,7 +186,8 @@ int res_read(int res_id, void *out, void *hint, int pid, 
int flags)
                        errno = err;
                        return -1;
                }
-               strncpy(out, t.release, RESOURCE_64);
+               strncpy(out, t.release, out_sz-1);
+               ((char *)out)[out_sz-1] = '\0';
                break;
 
        case RES_KERN_COMPILE_TIME:
@@ -194,7 +198,9 @@ int res_read(int res_id, void *out, void *hint, int pid, 
int flags)
                        errno = err;
                        return -1;
                }
-               sscanf(t.version, "%*s%*s%*s%[^\t\n]", (char *) out);
+               sscanf(t.version, "%*s%*s%*s%[^\t\n]", t.version);
+               strncpy(out, t.version, out_sz-1);
+               ((char *)out)[out_sz-1] = '\0';
                break;
 
        default:
@@ -212,6 +218,8 @@ int res_read_blk(res_blk_t *res, int pid, int flags)
        int isnetdevreq = 0;
        struct utsname t;
        int ret;
+       char *out;
+       size_t len;
 
        /* Loop through all resource information. If it can be filled through
         * a syscall or such method then fill it. Else set flags which tell
@@ -231,8 +239,12 @@ int res_read_blk(res_blk_t *res, int pid, int flags)
                        break;
 
                case RES_MEM_PAGESIZE:
-                       (res->res_unit[i]->data).sz = sysconf(_SC_PAGESIZE);
-                       res->res_unit[i]->status = RES_STATUS_FILLED;
+                       if (res->res_unit[i]->data_sz < sizeof(long)) {
+                               res->res_unit[i]->status = ENOMEM;
+                       } else {
+                               (res->res_unit[i]->data).sz = 
sysconf(_SC_PAGESIZE);
+                               res->res_unit[i]->status = RES_STATUS_FILLED;
+                       }
                        break;
 
                case RES_KERN_RELEASE:
@@ -240,8 +252,10 @@ int res_read_blk(res_blk_t *res, int pid, int flags)
                        if (ret == -1) {
                                res->res_unit[i]->status = errno;
                        } else {
-                               strncpy((res->res_unit[i]->data).str,
-                                       t.release, RESOURCE_64);
+                               out = (res->res_unit[i]->data).str;
+                               len = sizeof(union r_data);
+                               strncpy(out, t.release, len-1);
+                               out[len-1] = '\0';
                                res->res_unit[i]->status = RES_STATUS_FILLED;
                        }
                        break;
@@ -252,7 +266,11 @@ int res_read_blk(res_blk_t *res, int pid, int flags)
                                res->res_unit[i]->status = errno;
                        } else {
                                sscanf(t.version, "%*s%*s%*s%[^\t\n]",
-                                       (res->res_unit[i]->data).str);
+                                       t.version);
+                               out = (res->res_unit[i]->data).str;
+                               len = sizeof(union r_data);
+                               strncpy(out, t.version, len-1);
+                               out[len-1] = '\0';
                                res->res_unit[i]->status = RES_STATUS_FILLED;
                        }
                        break;
diff --git a/resource.h b/resource.h
index f5dfeb3..6ade910 100644
--- a/resource.h
+++ b/resource.h
@@ -68,6 +68,7 @@ typedef struct res_unit {
        unsigned int res_id;
        void *hint;
        union r_data data;
+       size_t data_sz;
 } res_unit_t;
 
 /* In case of bulk read (res_read_blk), this structure will hold all required
@@ -157,7 +158,9 @@ extern int res_read_blk(res_blk_t *resblk, int pid, int 
flags);
 /* Free allocated memory from res_build_blk */
 extern void res_destroy_blk(res_blk_t *resblk);
 
-/* Read a resource information. Memory for out should be properly allocated */
-extern int res_read(int res_id, void *out, void *hint, int pid, int flags);
+/* Read a resource information. Memory for out should be properly allocated.
+ * Also size of allocated memory should be passed in out_sz.
+ */
+extern int res_read(int res_id, void *out, size_t out_sz, void *hint, int pid, 
int flags);
 
 #endif /* RESOURCE_H */
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to