[libvirt PATCH v2 1/4] util: Try to get limits from /proc

[Michal Privoznik]

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 <abologna at 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