On 3/9/21 2:47 PM, Andrea Bolognani wrote:
Calling prlimit() requires elevated privileges, specifically
CAP_SYS_RESOURCE, and getrlimit() only works for the current
process which is too limiting for our needs; /proc/$pid/limits,
on the other hand, can be read by any process, so implement
parsing that file as a fallback for when prlimit() fails.

This is useful in containerized environments.

Signed-off-by: Andrea Bolognani <abolo...@redhat.com>
---
  src/util/virprocess.c | 102 ++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 102 insertions(+)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 8428c91182..4fa854090d 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -757,6 +757,99 @@ virProcessSetRLimit(int resource,
  #endif /* WITH_SETRLIMIT */
#if WITH_GETRLIMIT
+static const char*
+virProcessLimitResourceToLabel(int resource)
+{
+    switch (resource) {
+# if defined(RLIMIT_MEMLOCK)
+        case RLIMIT_MEMLOCK:
+            return "Max locked memory";
+# endif /* defined(RLIMIT_MEMLOCK) */
+
+# if defined(RLIMIT_NPROC)
+        case RLIMIT_NPROC:
+            return "Max processes";
+# endif /* defined(RLIMIT_NPROC) */
+
+# if defined(RLIMIT_NOFILE)
+        case RLIMIT_NOFILE:
+            return "Max open files";
+# endif /* defined(RLIMIT_NOFILE) */
+
+# if defined(RLIMIT_CORE)
+        case RLIMIT_CORE:
+            return "Max core file size";
+# endif /* defined(RLIMIT_CORE) */
+
+        default:
+            return NULL;
+    }
+}
+
+# if defined(__linux__)
+static int
+virProcessGetLimitFromProc(pid_t pid,
+                           int resource,
+                           struct rlimit *limit)
+{
+    g_autofree char *procfile = NULL;
+    g_autofree char *buf = NULL;
+    g_auto(GStrv) lines = NULL;
+    const char *label;
+    size_t i;
+
+    if (!(label = virProcessLimitResourceToLabel(resource))) {
+        return -1;
+    }

While I am in favor of curly braces, this is incosistent with the rest of the function. However, you'll need to add another line here, probably so in the end you'll need to keep them. See [1].

+
+    procfile = g_strdup_printf("/proc/%lld/limits", (long long)pid);
+
+    if (virFileReadAllQuiet(procfile, 2048, &buf) < 0)
+        return -1;
+
+    if (!(lines = g_strsplit(buf, "\n", 0)))
+        return -1;
+
+    for (i = 0; lines[i]; i++) {
+        g_autofree char *softLimit = NULL;
+        g_autofree char *hardLimit = NULL;
+        char *line = lines[i];
+        unsigned long long tmp;
+
+        if (!(line = STRSKIP(line, label)))
+            continue;
+
+        if (sscanf(line, "%ms %ms %*s", &softLimit, &hardLimit) < 2)
+            return -1;
+
+        if (STREQ(softLimit, "unlimited")) {
+            limit->rlim_cur = RLIM_INFINITY;
+        } else {
+            if (virStrToLong_ull(softLimit, NULL, 10, &tmp) < 0)
+                return -1;
+            limit->rlim_cur = tmp;
+        }
+        if (STREQ(hardLimit, "unlimited")) {
+            limit->rlim_max = RLIM_INFINITY;
+        } else {
+            if (virStrToLong_ull(hardLimit, NULL, 10, &tmp) < 0)
+                return -1;
+            limit->rlim_max = tmp;
+        }
+    }
+
+    return 0;
+}
+# else /* !defined(__linux__) */
+static int
+virProcessGetLimitFromProc(pid_t pid G_GNUC_UNUSED,
+                           int resource G_GNUC_UNUSED,
+                           struct rlimit *limit G_GNUC_UNUSED)
+{
+    return -1;
+}
+# endif /* !defined(__linux__) */
+
  static int
  virProcessGetLimit(pid_t pid,
                     int resource,
@@ -768,6 +861,15 @@ virProcessGetLimit(pid_t pid,
      if (virProcessPrLimit(pid, resource, NULL, old_limit) == 0)
          return 0;
+ /* For whatever reason, using prlimit() on another process - even
+     * when it's just to obtain the current limit rather than changing
+     * it - requires CAP_SYS_RESOURCE, which we might not have in a
+     * containerized environment; on the other hand, no particular
+     * permission is needed to poke around /proc, so try that if going
+     * through the syscall didn't work */
+    if (virProcessGetLimitFromProc(pid, resource, old_limit) == 0)
+        return 0;

There is just a single caller of this virProcessGetLimit() function and it expects errno to be set on failure. But that's not always the case with your patch. Moreover, I'd expect the !Linux stub to set ENOSYS.

+
      if (same_process && virProcessGetRLimit(resource, old_limit) == 0)
          return 0;

Michal

Reply via email to