All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.