Openembedded Core Discussions
 help / color / mirror / Atom feed
From: "Paul Gortmaker" <paul.gortmaker@windriver.com>
To: openembedded-core@lists.openembedded.org
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>,
	Luca Boccassi <luca.boccassi@microsoft.com>,
	Richard Purdie <richard.purdie@linuxfoundation.org>
Subject: [PATCH] systemd: dont spew hidepid mount errors for kernels < v5.8
Date: Fri, 15 Jan 2021 00:26:15 -0500	[thread overview]
Message-ID: <20210115052615.29893-1-paul.gortmaker@windriver.com> (raw)

Recent systemd started using ascii args to "hidepid=" mount options
for proc fs - unconditionally -- even though kernels older than v5.8
emit an error message on each attempt:

root@qemux86-64:~# cat /proc/version
Linux version 5.4.87-yocto-standard (oe-user@oe-host) (gcc version 10.2.0 (GCC)) #1 SMP PREEMPT Fri Jan 8 01:47:13 UTC 2021
root@qemux86-64:~# dmesg|grep proc:
[   29.487995] proc: Bad value for 'hidepid'
[   43.170571] proc: Bad value for 'hidepid'
[   44.175615] proc: Bad value for 'hidepid'
[   46.213300] proc: Bad value for 'hidepid'
root@qemux86-64:~#

Simply ignoring them as the systemd maintainer unconditionally says
is the resolution is clearly not acceptable, given the above.

Add a kernel version check to avoid calling mount with invalid args.
Further details are within the enclosed systemd commit.

Cc: Luca Boccassi <luca.boccassi@microsoft.com>
Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

diff --git a/meta/recipes-core/systemd/systemd/0027-proc-dont-trigger-mount-error-with-invalid-options-o.patch b/meta/recipes-core/systemd/systemd/0027-proc-dont-trigger-mount-error-with-invalid-options-o.patch
new file mode 100644
index 000000000000..65e7eca32d05
--- /dev/null
+++ b/meta/recipes-core/systemd/systemd/0027-proc-dont-trigger-mount-error-with-invalid-options-o.patch
@@ -0,0 +1,126 @@
+From 297aba739cd689e4dc9f43bb1422ec88d481099a Mon Sep 17 00:00:00 2001
+From: Paul Gortmaker <paul.gortmaker@windriver.com>
+Date: Wed, 13 Jan 2021 21:09:33 +0000
+Subject: [PATCH] proc: dont trigger mount error with invalid options on old
+ kernels
+
+As of commit 4e39995371738b04d98d27b0d34ea8fe09ec9fab ("core: introduce
+ProtectProc= and ProcSubset= to expose hidepid= and subset= procfs
+mount options") kernels older than v5.8 generate multple warnings at
+boot, as seen in this Yocto build from today:
+
+     qemux86-64 login: root
+     [   65.829009] proc: Bad value for 'hidepid'
+     root@qemux86-64:~# dmesg|grep proc:
+     [   16.990706] proc: Bad value for 'hidepid'
+     [   28.060178] proc: Bad value for 'hidepid'
+     [   28.874229] proc: Bad value for 'hidepid'
+     [   32.685107] proc: Bad value for 'hidepid'
+     [   65.829009] proc: Bad value for 'hidepid'
+     root@qemux86-64:~#
+
+The systemd maintainer has dismissed this as something people should
+simply ignore[1] and has no interest in trying to avoid it by
+proactively checking the kernel version, so people can safely assume
+that they will never see this version check commit upstream.
+
+However, as can be seen above, telling people to just ignore it is not
+an option, as we'll end up answering the same question and dealing with
+the same bug over and over again.
+
+The commit that triggers this is systemd v247-rc1~378^2~3 -- so any
+systemd 247 and above plus kernel v5.7 or older will need this.
+
+[1] https://github.com/systemd/systemd/issues/16896
+
+Upstream-Status: Actively hostile
+Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
+
+diff --git a/src/core/namespace.c b/src/core/namespace.c
+index cdf427a6ea93..f8fc33a89fc2 100644
+--- a/src/core/namespace.c
++++ b/src/core/namespace.c
+@@ -4,7 +4,9 @@
+ #include <linux/loop.h>
+ #include <sched.h>
+ #include <stdio.h>
++#include <stdlib.h>
+ #include <sys/mount.h>
++#include <sys/utsname.h>
+ #include <unistd.h>
+ #include <linux/fs.h>
+ 
+@@ -859,14 +861,34 @@ static int mount_sysfs(const MountEntry *m) {
+ }
+ 
+ static int mount_procfs(const MountEntry *m, const NamespaceInfo *ns_info) {
++        _cleanup_free_ char *opts = NULL;
+         const char *entry_path;
+-        int r;
++        int r, major, minor;
++        struct utsname uts;
++        bool old = false;
+ 
+         assert(m);
+         assert(ns_info);
+ 
+         entry_path = mount_entry_path(m);
+ 
++        /* If uname says that the system is older than v5.8, then the textual hidepid= stuff is not
++         * supported by the kernel, and thus the per-instance hidepid= neither, which means we
++         * really don't want to use it, since it would affect our host's /proc * mount. Hence let's
++         * gracefully fallback to a classic, unrestricted version. */
++
++        r = uname(&uts);
++        if (r < 0)
++               return errno;
++
++        major = atoi(uts.release);
++        minor = atoi(strchr(uts.release, '.') + 1);
++
++        if (major < 5 || (major == 5 && minor < 8)) {
++                log_debug("Pre v5.8 kernel detected [v%d.%d] - skipping hidepid=", major, minor);
++                old = true;
++        }
++
+         /* Mount a new instance, so that we get the one that matches our user namespace, if we are running in
+          * one. i.e we don't reuse existing mounts here under any condition, we want a new instance owned by
+          * our user namespace and with our hidepid= settings applied. Hence, let's get rid of everything
+@@ -875,9 +897,8 @@ static int mount_procfs(const MountEntry *m, const NamespaceInfo *ns_info) {
+         (void) mkdir_p_label(entry_path, 0755);
+         (void) umount_recursive(entry_path, 0);
+ 
+-        if (ns_info->protect_proc != PROTECT_PROC_DEFAULT ||
+-            ns_info->proc_subset != PROC_SUBSET_ALL) {
+-                _cleanup_free_ char *opts = NULL;
++        if (!old && (ns_info->protect_proc != PROTECT_PROC_DEFAULT ||
++            ns_info->proc_subset != PROC_SUBSET_ALL)) {
+ 
+                 /* Starting with kernel 5.8 procfs' hidepid= logic is truly per-instance (previously it
+                  * pretended to be per-instance but actually was per-namespace), hence let's make use of it
+@@ -891,21 +912,9 @@ static int mount_procfs(const MountEntry *m, const NamespaceInfo *ns_info) {
+                                ns_info->proc_subset == PROC_SUBSET_PID ? ",subset=pid" : "");
+                 if (!opts)
+                         return -ENOMEM;
+-
+-                r = mount_nofollow_verbose(LOG_DEBUG, "proc", entry_path, "proc", MS_NOSUID|MS_NOEXEC|MS_NODEV, opts);
+-                if (r < 0) {
+-                        if (r != -EINVAL)
+-                                return r;
+-
+-                        /* If this failed with EINVAL then this likely means the textual hidepid= stuff is
+-                         * not supported by the kernel, and thus the per-instance hidepid= neither, which
+-                         * means we really don't want to use it, since it would affect our host's /proc
+-                         * mount. Hence let's gracefully fallback to a classic, unrestricted version. */
+-                } else
+-                        return 1;
+         }
+ 
+-        r = mount_nofollow_verbose(LOG_DEBUG, "proc", entry_path, "proc", MS_NOSUID|MS_NOEXEC|MS_NODEV, NULL);
++        r = mount_nofollow_verbose(LOG_DEBUG, "proc", entry_path, "proc", MS_NOSUID|MS_NOEXEC|MS_NODEV, opts);
+         if (r < 0)
+                 return r;
+ 
+-- 
+2.29.2
+
diff --git a/meta/recipes-core/systemd/systemd_247.2.bb b/meta/recipes-core/systemd/systemd_247.2.bb
index 5eea78eff353..84d997196cb6 100644
--- a/meta/recipes-core/systemd/systemd_247.2.bb
+++ b/meta/recipes-core/systemd/systemd_247.2.bb
@@ -23,6 +23,7 @@ SRC_URI += "file://touchscreen.rules \
            file://0003-implment-systemd-sysv-install-for-OE.patch \
            file://0001-systemd.pc.in-use-ROOTPREFIX-without-suffixed-slash.patch \
            file://0001-logind-Restore-chvt-as-non-root-user-without-polkit.patch \
+           file://0027-proc-dont-trigger-mount-error-with-invalid-options-o.patch \
            "
 
 # patches needed by musl
-- 
2.30.0


             reply	other threads:[~2021-01-15  5:26 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-15  5:26 Paul Gortmaker [this message]
2021-01-15  6:02 ` [OE-core] [PATCH] systemd: dont spew hidepid mount errors for kernels < v5.8 Konrad Weihmann
2021-01-15  8:37 ` Richard Purdie
2021-01-15  9:57   ` Luca Boccassi
2021-01-15 10:14     ` Richard Purdie
2021-01-15 10:20       ` Luca Boccassi
2021-01-15 14:47     ` Paul Gortmaker
2021-01-15 15:37       ` Luca Boccassi
2021-01-15 16:07         ` [OE-core] " Bruce Ashfield
2021-01-15 16:31           ` Luca Boccassi
2021-01-15 16:59             ` Bruce Ashfield
2021-01-15 17:35               ` Luca Boccassi
2021-01-18 11:07                 ` Leon Woestenberg
2021-01-18 12:36                 ` Richard Purdie
2021-01-15 15:27 ` Luca Boccassi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210115052615.29893-1-paul.gortmaker@windriver.com \
    --to=paul.gortmaker@windriver.com \
    --cc=luca.boccassi@microsoft.com \
    --cc=openembedded-core@lists.openembedded.org \
    --cc=richard.purdie@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox