[lxc-devel] [lxcfs/master] bugfixes
brauner on Github
lxc-bot at linuxcontainers.org
Thu Mar 5 10:26:41 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 365 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200305/06aa2e7a/attachment.bin>
-------------- next part --------------
From f75d5b75d11e8dd9060155569f8c093b20ab8d37 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 5 Mar 2020 11:02:22 +0100
Subject: [PATCH 1/4] proc_fuse: cleanup
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/proc_fuse.c | 172 ++++++++++++++++++++++--------------------------
1 file changed, 77 insertions(+), 95 deletions(-)
diff --git a/src/proc_fuse.c b/src/proc_fuse.c
index 392d10d..5cb62c4 100644
--- a/src/proc_fuse.c
+++ b/src/proc_fuse.c
@@ -79,6 +79,7 @@ int proc_getattr(const char *path, struct stat *sb)
memset(sb, 0, sizeof(struct stat));
if (clock_gettime(CLOCK_REALTIME, &now) < 0)
return -EINVAL;
+
sb->st_uid = sb->st_gid = 0;
sb->st_atim = sb->st_mtim = sb->st_ctim = now;
if (strcmp(path, "/proc") == 0) {
@@ -86,13 +87,14 @@ int proc_getattr(const char *path, struct stat *sb)
sb->st_nlink = 2;
return 0;
}
- if (strcmp(path, "/proc/meminfo") == 0 ||
- strcmp(path, "/proc/cpuinfo") == 0 ||
- strcmp(path, "/proc/uptime") == 0 ||
- strcmp(path, "/proc/stat") == 0 ||
- strcmp(path, "/proc/diskstats") == 0 ||
- strcmp(path, "/proc/swaps") == 0 ||
- strcmp(path, "/proc/loadavg") == 0) {
+
+ if (strcmp(path, "/proc/meminfo") == 0 ||
+ strcmp(path, "/proc/cpuinfo") == 0 ||
+ strcmp(path, "/proc/uptime") == 0 ||
+ strcmp(path, "/proc/stat") == 0 ||
+ strcmp(path, "/proc/diskstats") == 0 ||
+ strcmp(path, "/proc/swaps") == 0 ||
+ strcmp(path, "/proc/loadavg") == 0) {
sb->st_size = 0;
sb->st_mode = S_IFREG | 00444;
sb->st_nlink = 1;
@@ -119,19 +121,19 @@ int proc_readdir(const char *path, void *buf, fuse_fill_dir_t filler,
return 0;
}
-static off_t get_procfile_size(const char *which)
+static off_t get_procfile_size(const char *path)
{
- FILE *f = fopen(which, "re");
- char *line = NULL;
+ __do_fclose FILE *f = NULL;
+ __do_free char *line = NULL;
size_t len = 0;
ssize_t sz, answer = 0;
+
+ f = fopen(path, "re");
if (!f)
return 0;
while ((sz = getline(&line, &len, f)) != -1)
answer += sz;
- fclose (f);
- free(line);
return answer;
}
@@ -187,6 +189,7 @@ int proc_access(const char *path, int mask)
/* these are all read-only */
if ((mask & ~R_OK) != 0)
return -EACCES;
+
return 0;
}
@@ -198,9 +201,9 @@ int proc_release(const char *path, struct fuse_file_info *fi)
static unsigned long get_memlimit(const char *cgroup, bool swap)
{
- int ret;
__do_free char *memlimit_str = NULL;
unsigned long memlimit = -1;
+ int ret;
if (swap)
ret = cgroup_ops->get_memory_swap_max(cgroup_ops, cgroup, &memlimit_str);
@@ -219,6 +222,9 @@ static unsigned long get_min_memlimit(const char *cgroup, bool swap)
unsigned long retlimit;
copy = strdup(cgroup);
+ if (!copy)
+ return log_error_errno(0, ENOMEM, "Failed to allocate memory");
+
retlimit = get_memlimit(copy, swap);
while (strcmp(copy, "/") != 0) {
@@ -233,11 +239,9 @@ static unsigned long get_min_memlimit(const char *cgroup, bool swap)
return retlimit;
}
-static bool startswith(const char *line, const char *pref)
+static inline bool startswith(const char *line, const char *pref)
{
- if (strncmp(line, pref, strlen(pref)) == 0)
- return true;
- return false;
+ return strncmp(line, pref, strlen(pref)) == 0;
}
static int proc_swaps_read(char *buf, size_t size, off_t offset,
@@ -325,16 +329,16 @@ static int proc_swaps_read(char *buf, size_t size, off_t offset,
total_len += l;
}
- if (total_len < 0 || l < 0) {
- perror("Error writing to cache");
- return 0;
- }
+ if (total_len < 0 || l < 0)
+ return log_error(0, "Failed writing to cache");
d->cached = 1;
d->size = (int)total_len;
- if (total_len > size) total_len = size;
+ if (total_len > size)
+ total_len = size;
memcpy(buf, d->buf, total_len);
+
return total_len;
}
@@ -343,13 +347,13 @@ static void get_blkio_io_value(char *str, unsigned major, unsigned minor,
{
char *eol;
char key[32];
+ size_t len;
memset(key, 0, 32);
snprintf(key, 32, "%u:%u %s", major, minor, iotype);
- size_t len = strlen(key);
*v = 0;
-
+ len = strlen(key);
while (*str) {
if (startswith(str, key)) {
sscanf(str + len, "%lu", v);
@@ -487,14 +491,11 @@ static int proc_diskstats_read(char *buf, size_t size, off_t offset,
continue;
l = snprintf(cache, cache_size, "%s", lbuf);
- if (l < 0) {
- perror("Error writing to fuse buf");
- return 0;
- }
- if (l >= cache_size) {
- lxcfs_error("%s\n", "Internal error: truncated write to cache.");
- return 0;
- }
+ if (l < 0)
+ return log_error(0, "Failed to write cache");
+ if (l >= cache_size)
+ return log_error(0, "Write to cache was truncated");
+
cache += l;
cache_size -= l;
total_len += l;
@@ -624,18 +625,12 @@ static double get_reaper_start_time_in_sec(pid_t pid)
double res = 0;
clockticks = get_reaper_start_time(pid);
- if (clockticks == 0 && errno == EINVAL) {
- lxcfs_debug("failed to retrieve start time of pid %d\n", pid);
- return 0;
- }
+ if (clockticks == 0 && errno == EINVAL)
+ return log_debug(0, "Failed to retrieve start time of pid %d", pid);
ret = sysconf(_SC_CLK_TCK);
- if (ret < 0 && errno == EINVAL) {
- lxcfs_debug(
- "%s\n",
- "failed to determine number of clock ticks in a second");
- return 0;
- }
+ if (ret < 0 && errno == EINVAL)
+ return log_debug(0, "Failed to determine number of clock ticks in a second");
ticks_per_sec = (uint64_t)ret;
res = (double)clockticks / ticks_per_sec;
@@ -692,19 +687,25 @@ static int proc_uptime_read(char *buf, size_t size, off_t offset,
iwashere();
#endif
- if (offset){
+ if (offset) {
+ int left;
+
if (!d->cached)
return 0;
+
if (offset > d->size)
return -EINVAL;
- int left = d->size - offset;
- total_len = left > size ? size: left;
+
+ left = d->size - offset;
+ total_len = left > size ? size : left;
memcpy(buf, cache + offset, total_len);
+
return total_len;
}
reaperage = get_reaper_age(fc->pid);
- /* To understand why this is done, please read the comment to the
+ /*
+ * To understand why this is done, please read the comment to the
* get_reaper_busy() function.
*/
idletime = reaperage;
@@ -712,15 +713,14 @@ static int proc_uptime_read(char *buf, size_t size, off_t offset,
idletime = reaperage - busytime;
total_len = snprintf(d->buf, d->buflen, "%.2lf %.2lf\n", reaperage, idletime);
- if (total_len < 0 || total_len >= d->buflen){
- lxcfs_error("%s\n", "failed to write to cache");
- return 0;
- }
+ if (total_len < 0 || total_len >= d->buflen)
+ return log_error(0, "Failed to write to cache");
d->size = (int)total_len;
d->cached = 1;
- if (total_len > size) total_len = size;
+ if (total_len > size)
+ total_len = size;
memcpy(buf, d->buf, total_len);
return total_len;
@@ -800,10 +800,8 @@ static int proc_stat_read(char *buf, size_t size, off_t offset,
return 0;
//skip first line
- if (getline(&line, &linelen, f) < 0) {
- lxcfs_error("%s\n", "proc_stat_read read first line failed.");
- return 0;
- }
+ if (getline(&line, &linelen, f) < 0)
+ return log_error(0, "proc_stat_read read first line failed");
if (cgroup_ops->can_use_cpuview(cgroup_ops) && cg_cpu_usage) {
total_len = cpuview_proc_stat(cg, cpuset, cg_cpu_usage, cg_cpu_usage_size,
@@ -823,24 +821,24 @@ static int proc_stat_read(char *buf, size_t size, off_t offset,
if (sscanf(line, "cpu%9[^ ]", cpu_char) != 1) {
/* not a ^cpuN line containing a number N, just print it */
l = snprintf(cache, cache_size, "%s", line);
- if (l < 0) {
- perror("Error writing to cache");
- return 0;
- }
- if (l >= cache_size) {
- lxcfs_error("%s\n", "Internal error: truncated write to cache.");
- return 0;
- }
+ if (l < 0)
+ return log_error(0, "Failed to write cache");
+ if (l >= cache_size)
+ return log_error(0, "Write to cache was truncated");
+
cache += l;
cache_size -= l;
total_len += l;
+
continue;
}
if (sscanf(cpu_char, "%d", &physcpu) != 1)
continue;
+
if (!cpu_in_cpuset(physcpu, cpuset))
continue;
+
curcpu++;
ret = sscanf(line, "%*s %lu %lu %lu %lu %lu %lu %lu %lu %lu %lu",
@@ -854,21 +852,16 @@ static int proc_stat_read(char *buf, size_t size, off_t offset,
&steal,
&guest,
&guest_nice);
-
if (ret != 10 || !cg_cpu_usage) {
c = strchr(line, ' ');
if (!c)
continue;
- l = snprintf(cache, cache_size, "cpu%d%s", curcpu, c);
- if (l < 0) {
- perror("Error writing to cache");
- return 0;
- }
- if (l >= cache_size) {
- lxcfs_error("%s\n", "Internal error: truncated write to cache.");
- return 0;
- }
+ l = snprintf(cache, cache_size, "cpu%d%s", curcpu, c);
+ if (l < 0)
+ return log_error(0, "Failed to write cache");
+ if (l >= cache_size)
+ return log_error(0, "Write to cache was truncated");
cache += l;
cache_size -= l;
@@ -898,16 +891,10 @@ static int proc_stat_read(char *buf, size_t size, off_t offset,
"cpu%d %" PRIu64 " 0 %" PRIu64 " %" PRIu64 " 0 0 0 0 0 0\n",
curcpu, cg_cpu_usage[physcpu].user,
cg_cpu_usage[physcpu].system, new_idle);
-
- if (l < 0) {
- perror("Error writing to cache");
- return 0;
-
- }
- if (l >= cache_size) {
- lxcfs_error("%s\n", "Internal error: truncated write to cache.");
- return 0;
- }
+ if (l < 0)
+ return log_error(0, "Failed to write cache");
+ if (l >= cache_size)
+ return log_error(0, "Write to cache was truncated");
cache += l;
cache_size -= l;
@@ -916,7 +903,6 @@ static int proc_stat_read(char *buf, size_t size, off_t offset,
user_sum += cg_cpu_usage[physcpu].user;
system_sum += cg_cpu_usage[physcpu].system;
idle_sum += new_idle;
-
} else {
user_sum += user;
nice_sum += nice;
@@ -949,7 +935,7 @@ static int proc_stat_read(char *buf, size_t size, off_t offset,
cache += cpuall_len;
} else {
/* shouldn't happen */
- lxcfs_error("proc_stat_read copy cpuall failed, cpuall_len=%d.", cpuall_len);
+ lxcfs_error("proc_stat_read copy cpuall failed, cpuall_len=%d", cpuall_len);
cpuall_len = 0;
}
@@ -1235,15 +1221,10 @@ static int proc_meminfo_read(char *buf, size_t size, off_t offset,
}
l = snprintf(cache, cache_size, "%s", printme);
- if (l < 0) {
- perror("Error writing to cache");
- return 0;
-
- }
- if (l >= cache_size) {
- lxcfs_error("%s\n", "Internal error: truncated write to cache.");
- return 0;
- }
+ if (l < 0)
+ return log_error(0, "Failed to write cache");
+ if (l >= cache_size)
+ return log_error(0, "Write to cache was truncated");
cache += l;
cache_size -= l;
@@ -1252,7 +1233,8 @@ static int proc_meminfo_read(char *buf, size_t size, off_t offset,
d->cached = 1;
d->size = total_len;
- if (total_len > size ) total_len = size;
+ if (total_len > size)
+ total_len = size;
memcpy(buf, d->buf, total_len);
return total_len;
From 285194776d0046f8a5b6c40afd8ce5b0592d18d3 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 5 Mar 2020 11:22:34 +0100
Subject: [PATCH 2/4] proc_fuse: introduce and use fdopen_cached()
Note that in contrast to libc's fdopen() it doesn't consume the fd it is
passed.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/proc_fuse.c | 5 ++--
src/utils.c | 79 ++++++++++++++++++++++++++++++++++++++++---------
src/utils.h | 1 +
3 files changed, 68 insertions(+), 17 deletions(-)
diff --git a/src/proc_fuse.c b/src/proc_fuse.c
index 5cb62c4..64092fe 100644
--- a/src/proc_fuse.c
+++ b/src/proc_fuse.c
@@ -958,6 +958,7 @@ static bool cgroup_parse_memory_stat(const char *cgroup, struct memory_stat *mst
__do_close_prot_errno int fd = -EBADF;
__do_fclose FILE *f = NULL;
__do_free char *line = NULL;
+ __do_free void *fdopen_cache = NULL;
bool unified;
size_t len = 0;
ssize_t linelen;
@@ -966,11 +967,9 @@ static bool cgroup_parse_memory_stat(const char *cgroup, struct memory_stat *mst
if (fd < 0)
return false;
- f = fdopen(fd, "re");
+ f = fdopen_cached(fd, "re", &fdopen_cache);
if (!f)
return false;
- /* Transferring ownership to fdopen(). */
- move_fd(fd);
unified = pure_unified_layout(cgroup_ops);
while ((linelen = getline(&line, &len, f)) != -1) {
diff --git a/src/utils.c b/src/utils.c
index 0bc99c1..3235eaa 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -429,38 +429,49 @@ static void *must_realloc(void *orig, size_t sz)
return ret;
}
-static char *file_to_buf(const char *path, size_t *length)
+static char *fd_to_buf(int fd, size_t *length)
{
- __do_close_prot_errno int fd = -EBADF;
__do_free char *copy = NULL;
- char buf[PATH_MAX];
if (!length)
return NULL;
- fd = open(path, O_RDONLY | O_CLOEXEC);
- if (fd < 0)
- return NULL;
-
*length = 0;
for (;;) {
- int n;
+ ssize_t bytes_read;
+ char buf[4096];
char *old = copy;
- n = read_nointr(fd, buf, sizeof(buf));
- if (n < 0)
+ bytes_read = read_nointr(fd, buf, sizeof(buf));
+ if (bytes_read < 0)
return NULL;
- if (!n)
+
+ if (!bytes_read)
break;
- copy = must_realloc(old, (*length + n) * sizeof(*old));
- memcpy(copy + *length, buf, n);
- *length += n;
+ copy = must_realloc(old, (*length + bytes_read) * sizeof(*old));
+ memcpy(copy + *length, buf, bytes_read);
+ *length += bytes_read;
}
return move_ptr(copy);
}
+static char *file_to_buf(const char *path, size_t *length)
+{
+ __do_close_prot_errno int fd = -EBADF;
+ char buf[PATH_MAX];
+
+ if (!length)
+ return NULL;
+
+ fd = open(path, O_RDONLY | O_CLOEXEC);
+ if (fd < 0)
+ return NULL;
+
+ return fd_to_buf(fd, length);
+}
+
FILE *fopen_cached(const char *path, const char *mode, void **caller_freed_buffer)
{
__do_free char *buf = NULL;
@@ -477,3 +488,43 @@ FILE *fopen_cached(const char *path, const char *mode, void **caller_freed_buffe
*caller_freed_buffer = move_ptr(buf);
return f;
}
+
+static int fd_cloexec(int fd, bool cloexec)
+{
+ int oflags, nflags;
+
+ oflags = fcntl(fd, F_GETFD, 0);
+ if (oflags < 0)
+ return -errno;
+
+ if (cloexec)
+ nflags = oflags | FD_CLOEXEC;
+ else
+ nflags = oflags & ~FD_CLOEXEC;
+
+ if (nflags == oflags)
+ return 0;
+
+ if (fcntl(fd, F_SETFD, nflags) < 0)
+ return -errno;
+
+ return 0;
+}
+
+FILE *fdopen_cached(int fd, const char *mode, void **caller_freed_buffer)
+{
+ __do_free char *buf = NULL;
+ size_t len = 0;
+ FILE *f;
+
+ buf = fd_to_buf(fd, &len);
+ if (!buf)
+ return NULL;
+
+ f = fmemopen(buf, len, mode);
+ if (!f)
+ return NULL;
+
+ *caller_freed_buffer = move_ptr(buf);
+ return f;
+}
diff --git a/src/utils.h b/src/utils.h
index 7450fb6..05b9a42 100644
--- a/src/utils.h
+++ b/src/utils.h
@@ -73,5 +73,6 @@ static inline int pidfd_send_signal(int pidfd, int sig, siginfo_t *info,
extern FILE *fopen_cached(const char *path, const char *mode,
void **caller_freed_buffer);
+extern FILE *fdopen_cached(int fd, const char *mode, void **caller_freed_buffer);
#endif /* __LXCFS_UTILS_H */
From 965cb950f70348a333f2d2ac52c140498a265adc Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 5 Mar 2020 11:23:29 +0100
Subject: [PATCH 3/4] cgroup_utils: only transfer ownership of fd after
fdopen() has succeeded
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/cgroups/cgroup_utils.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/cgroups/cgroup_utils.c b/src/cgroups/cgroup_utils.c
index af9b403..7bdf65b 100644
--- a/src/cgroups/cgroup_utils.c
+++ b/src/cgroups/cgroup_utils.c
@@ -684,10 +684,11 @@ char *readat_file(int dirfd, const char *path)
if (fd < 0)
return NULL;
- /* transfer ownership of fd */
- f = fdopen(move_fd(fd), "re");
+ f = fdopen(fd, "re");
if (!f)
return NULL;
+ /* Transfer ownership of fd */
+ move_fd(fd);
while ((linelen = getline(&line, &len, f)) != -1) {
append_line(&buf, fulllen, line, linelen);
From 9b817e41497e2a52691ab187c41c7a485fa2a757 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 5 Mar 2020 11:24:38 +0100
Subject: [PATCH 4/4] proc_loadavg: use fdopen_cached()
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/proc_loadavg.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/proc_loadavg.c b/src/proc_loadavg.c
index e48fcab..6ca5d06 100644
--- a/src/proc_loadavg.c
+++ b/src/proc_loadavg.c
@@ -271,6 +271,7 @@ int proc_loadavg_read(char *buf, size_t size, off_t offset,
static int calc_pid(char ***pid_buf, char *dpath, int depth, int sum, int cfd)
{
__do_free char *path = NULL;
+ __do_free void *fdopen_cache = NULL;
__do_close_prot_errno int fd = -EBADF;
__do_fclose FILE *f = NULL;
__do_closedir DIR *dir = NULL;
@@ -322,7 +323,7 @@ static int calc_pid(char ***pid_buf, char *dpath, int depth, int sum, int cfd)
if (fd < 0)
return sum;
- f = fdopen(move_fd(fd), "r");
+ f = fdopen_cached(fd, "re", &fdopen_cache);
if (!f)
return sum;
More information about the lxc-devel
mailing list