From: Lucas De Marchi <lucas.demarchi@intel.com>
To: "Ahelenia Ziemiańska" <nabijaczleweli@nabijaczleweli.xyz>
Cc: <linux-modules@vger.kernel.org>
Subject: Re: Re: [PATCH kmod 2/3] Add KMOD_NEW_IGNORE_CMDLINE
Date: Wed, 7 Feb 2024 00:06:08 -0600 [thread overview]
Message-ID: <khn6wo2mbrdl6jebvx3opfgjcpsqn22pq2ekui6356uqto2jut@gij3s7zgjdu7> (raw)
In-Reply-To: <z3lxzhymtgvvri2uhwylgazzlvg25junsdu4jjtxfhvfwksm3j@tarta.nabijaczleweli.xyz>
On Wed, Dec 06, 2023 at 09:37:21PM +0100, Ahelenia Ziemiańska wrote:
>On Wed, Dec 06, 2023 at 09:14:55AM -0600, Lucas De Marchi wrote:
>> On Tue, Dec 05, 2023 at 04:55:22PM +0100, Ahelenia Ziemiańska wrote:
>> touch foo
>> sudo mount --bind foo /proc/cmdline
>> This has been what I always used for local/test scenarios.
>This is also what I came up with post factum, but it's clearly not obvious,
>since the responding user did resort to rebooting.
>
>> I wonder if
>> this really needs a more "official" way like your are doing to be
>> added to the library rather than just the tools.
>I don't disagree; below is a scissor-patch that effectivaly
>canonicalises modprobe -I ... to be
>"unshare -rm sh -c 'mount --bind /dev/null /proc/cmdline; modprobe ...'"
>(the -r is removed if you're already root).
>
>I've used this approach (exactly this snippet in various arrangements)
>extensively in various test suites to string up fake procfses,
>and it ought to work in all environments you'd be validly running modprobe
>(it won't work if you're non-root in a chroot: unlikely).
>
>Best,
>-- >8 --
>Subject: [PATCH v2] Add modprobe -I/--ignore-cmdline
>
>Previously, if you'd misconfigured the cmdline your system would be
>completely poisoned.
I as mentioning that as a way for people/sysadmins to workaround that
and get out of the situation "now my system is completely poisoned".
>
>In this real scenario, ixgbe.allow_supported_sfp=1,1,1,1 was set.
>This yielded
> [ 3852.901900] ixgbe: `1,1,1,1' invalid for parameter `allow_unsupported_sfp'
> [ 3852.904595] ixgbe: unknown parameter 'allow_supported_sfp' ignored
>and
> # modprobe -r ixgbe
> # modprobe ixgbe allow_supported_sfp=1
>since, indeed,
> # modprobe -nv ixgbe
> insmod /lib/modules/5.16.0-1-amd64/kernel/drivers/net/ethernet/intel/ixgbe/ixgbe.ko allow_unsupported_sfp=1,1,1,1
> # modprobe -nv ixgbe allow_supported_sfp=1
> insmod /lib/modules/5.16.0-1-amd64/kernel/drivers/net/ethernet/intel/ixgbe/ixgbe.ko allow_unsupported_sfp=1,1,1,1 allow_supported_sfp=1
>this leaves you with a tens-of-minutes-long reboot
>(or with an explicit insmod, which no-one came up with at the time,
> and which requires manual dependency-chasing).
>
>With -I, the module can be correctly loaded since the cmdline-derived
>parameter no longer stops the module loading:
> # modprobe -nvI ixgbe allow_supported_sfp=1
> insmod /lib/modules/5.16.0-1-amd64/kernel/drivers/net/ethernet/intel/ixgbe/ixgbe.ko allow_supported_sfp=1
> # modprobe -I ixgbe allow_supported_sfp=1
> [ 4497.032342] ixgbe: Intel(R) 10 Gigabit PCI Express Network Driver
> [ 4497.034624] ixgbe: Copyright (c) 1999-2016 Intel Corporation.
>
>This in many ways mirrors -C /dev/null and -i.
yeah... but if we are indeed adding it to modprobe, then the solution
with libkmod change is probably better. I will think a bit after the
next version that should be released this week.
thanks and sorry for the delay on replying.
Lucas De Marchi
>
>Yes, you could do this manually with
> unshare -m; mount --bind /dev/null /proc/cmdline
>but if you aren't primed to look for it,
>or aren't familiar with the mechanism in the first place,
>you can't
>
>Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
>---
> man/modprobe.xml | 16 ++++++++++++++++
> tools/modprobe.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 58 insertions(+), 1 deletion(-)
>
>diff --git a/man/modprobe.xml b/man/modprobe.xml
>index 91f9e27..ab9dbb0 100644
>--- a/man/modprobe.xml
>+++ b/man/modprobe.xml
>@@ -47,6 +47,7 @@
> <arg><option>-C <replaceable>config-file</replaceable></option></arg>
> <arg><option>-n</option></arg>
> <arg><option>-i</option></arg>
>+ <arg><option>-I</option></arg>
> <arg><option>-q</option></arg>
> <arg><option>-b</option></arg>
> <arg><replaceable>modulename</replaceable></arg>
>@@ -58,6 +59,7 @@
> <arg><option>-v</option></arg>
> <arg><option>-n</option></arg>
> <arg><option>-i</option></arg>
>+ <arg><option>-I</option></arg>
> <arg rep='repeat'><option><replaceable>modulename</replaceable></option></arg>
> </cmdsynopsis>
> <cmdsynopsis>
>@@ -318,6 +320,20 @@
> </para>
> </listitem>
> </varlistentry>
>+ <varlistentry>
>+ <term>
>+ <option>-I</option>
>+ </term>
>+ <term>
>+ <option>--ignore-cmdline</option>
>+ </term>
>+ <listitem>
>+ <para>
>+ This option causes <command>modprobe</command> to ignore
>+ any configuration specified via the kernel command line.
>+ </para>
>+ </listitem>
>+ </varlistentry>
> <varlistentry>
> <term>
> <option>-n</option>
>diff --git a/tools/modprobe.c b/tools/modprobe.c
>index e891028..de013b1 100644
>--- a/tools/modprobe.c
>+++ b/tools/modprobe.c
>@@ -21,11 +21,13 @@
> #include <errno.h>
> #include <getopt.h>
> #include <limits.h>
>+#include <sched.h>
> #include <stdbool.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
>+#include <sys/mount.h>
> #include <sys/stat.h>
> #include <sys/types.h>
> #include <sys/utsname.h>
>@@ -59,7 +61,7 @@ static int remove_holders = 0;
> static unsigned long long wait_msec = 0;
> static int quiet_inuse = 0;
>
>-static const char cmdopts_s[] = "arw:RibfDcnC:d:S:sqvVh";
>+static const char cmdopts_s[] = "arw:RiIbfDcnC:d:S:sqvVh";
> static const struct option cmdopts[] = {
> {"all", no_argument, 0, 'a'},
>
>@@ -72,6 +74,7 @@ static const struct option cmdopts[] = {
> {"first-time", no_argument, 0, 3},
> {"ignore-install", no_argument, 0, 'i'},
> {"ignore-remove", no_argument, 0, 'i'},
>+ {"ignore-cmdline", no_argument, 0, 'I'},
> {"use-blacklist", no_argument, 0, 'b'},
> {"force", no_argument, 0, 'f'},
> {"force-modversion", no_argument, 0, 2},
>@@ -825,6 +828,32 @@ static char **prepend_options_from_env(int *p_argc, char **orig_argv)
> return new_argv;
> }
>
>+#define UNSHARE_REQ(...) if(!(__VA_ARGS__)) return false;
>+#define UNSHARE_FILE(path, ...) \
>+ { \
>+ FILE * f = fopen(path, "we"); \
>+ UNSHARE_REQ(f); \
>+ fprintf(f, __VA_ARGS__); \
>+ fclose(f); \
>+ }
>+static bool clear_cmdline(void)
>+{
>+ int uid = geteuid();
>+ if(uid) {
>+ int gid = getegid();
>+ UNSHARE_REQ(!unshare(CLONE_NEWUSER));
>+ UNSHARE_FILE("/proc/self/setgroups", "deny");
>+ UNSHARE_FILE("/proc/self/uid_map", "0 %d 1", uid);
>+ UNSHARE_FILE("/proc/self/gid_map", "0 %d 1", gid);
>+ }
>+
>+ UNSHARE_REQ(!unshare(CLONE_NEWNS));
>+ UNSHARE_REQ(!mount("none", "/", NULL, MS_REC | MS_PRIVATE, NULL));
>+
>+ UNSHARE_REQ(!mount("/dev/null", "/proc/cmdline", NULL, MS_BIND, NULL));
>+ return true;
>+}
>+
> static int do_modprobe(int argc, char **orig_argv)
> {
> struct kmod_ctx *ctx;
>@@ -835,6 +864,7 @@ static int do_modprobe(int argc, char **orig_argv)
> const char *dirname = NULL;
> const char *root = NULL;
> const char *kversion = NULL;
>+ int ignore_cmdline = 0;
> int use_all = 0;
> int do_remove = 0;
> int do_show_config = 0;
>@@ -881,6 +911,9 @@ static int do_modprobe(int argc, char **orig_argv)
> case 'i':
> ignore_commands = 1;
> break;
>+ case 'I':
>+ ignore_cmdline = 1;
>+ break;
> case 'b':
> use_blacklist = 1;
> break;
>@@ -1004,6 +1037,14 @@ static int do_modprobe(int argc, char **orig_argv)
> dirname = dirname_buf;
> }
>
>+ if (ignore_cmdline) {
>+ if (!clear_cmdline()) {
>+ ERR("clear_cmdline() failed!\n");
>+ err = -1;
>+ goto done;
>+ }
>+ }
>+
> ctx = kmod_new(dirname, config_paths);
> if (!ctx) {
> ERR("kmod_new() failed!\n");
>--
>2.39.2
>
next prev parent reply other threads:[~2024-02-07 6:06 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-05 15:55 [PATCH kmod 1/3] Add kmod_new_flags() variant of kmod_new(), with abiver 32 Ahelenia Ziemiańska
2023-12-05 15:55 ` [PATCH kmod 2/3] Add KMOD_NEW_IGNORE_CMDLINE Ahelenia Ziemiańska
2023-12-06 15:14 ` Lucas De Marchi
2023-12-06 20:37 ` Ahelenia Ziemiańska
2024-02-07 6:06 ` Lucas De Marchi [this message]
2023-12-05 15:55 ` [PATCH kmod 3/3] Add modprobe -I/--ignore-cmdline Ahelenia Ziemiańska
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=khn6wo2mbrdl6jebvx3opfgjcpsqn22pq2ekui6356uqto2jut@gij3s7zgjdu7 \
--to=lucas.demarchi@intel.com \
--cc=linux-modules@vger.kernel.org \
--cc=nabijaczleweli@nabijaczleweli.xyz \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).