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