selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rahul Sandhu <nvraxn@gmail.com>
To: stephen.smalley.work@gmail.com
Cc: nvraxn@gmail.com, omosnace@redhat.com, paul@paul-moore.com,
	selinux@vger.kernel.org
Subject: [PATCH v3] libselinux: fix parsing of the enforcing kernel cmdline parameter
Date: Thu, 24 Jul 2025 14:05:11 +0100	[thread overview]
Message-ID: <20250724130511.317098-1-nvraxn@gmail.com> (raw)
In-Reply-To: <CAEjxPJ6AuwTX9soXmHSiJbE2r69mRt8qFTTOQj-FhWUjnnYdQg@mail.gmail.com>

Currently, parsing of the cmdline has two issues:
- By using atoi, no error checking is done. What happens if an argument
  that isn't an integer is provided, e.g. enforcing=foo? And as there
  is also no validation that the number provided is actually valid, 1
  or 0, what happens if enforcing=2?

- After the first strstr, no arguments that follow are searched for; if
  I have enforcing=0 enforcing=1, the latter enforcing=1 is not taken
  into account. This is made even worse due to halting searching after
  finding the first "enforcing=" token, meaning that if the cmdline was
  as follows:

  fooenforcing=0 enforcing=0

  the enforcing parameter is entirely ignored.

This patch fixes this by:

  - Using strtol to actually validate that we got passed a number, and
    then validating that that number is either 0 or 1. If instead we
    get passed an invalid value, we skip over the argument entirely.

  - Looping until the last "enforcing=" in the cmdline. Latter (valid)
    arguments take precedence over previous arguments.

For the case where "enforcing=" is provided with a valid integer, 0 is
treated as permissive mode, and anything else (such as 1 or 2, etc) is
treated as enforcing mode. When "enforcing=" is passed an argument that
is not a valid integer (such as "on"), default to enforcing=0, i.e.
permissive mode. This is in line with how the kernel parses the
enforcing parameter.

Signed-off-by: Rahul Sandhu <nvraxn@gmail.com>
---
 libselinux/src/load_policy.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

v2: Follow the same argument parsing behaviour as the kernel does.
v3: Actually follow the kernel's behaviour where "enforcing=" is not
    provided with a valid integer...

diff --git a/libselinux/src/load_policy.c b/libselinux/src/load_policy.c
index dc1e4b6e..ec2d5614 100644
--- a/libselinux/src/load_policy.c
+++ b/libselinux/src/load_policy.c
@@ -244,17 +244,28 @@ int selinux_init_load_policy(int *enforce)
 	rc = mount("proc", "/proc", "proc", 0, 0);
 	cfg = fopen("/proc/cmdline", "re");
 	if (cfg) {
-		char *tmp;
 		buf = malloc(selinux_page_size);
 		if (!buf) {
 			fclose(cfg);
 			return -1;
 		}
-		if (fgets(buf, selinux_page_size, cfg) &&
-		    (tmp = strstr(buf, "enforcing="))) {
-			if (tmp == buf || isspace((unsigned char)*(tmp - 1))) {
-				secmdline =
-				    atoi(tmp + sizeof("enforcing=") - 1);
+		if (fgets(buf, selinux_page_size, cfg)) {
+			char *search = buf;
+			char *tmp;
+			while ((tmp = strstr(search, "enforcing="))) {
+				if (tmp == buf || isspace((unsigned char)*(tmp - 1))) {
+					char *valstr = tmp + sizeof("enforcing=") - 1;
+					char *endptr;
+					errno = 0;
+					const long val = strtol(valstr, &endptr, 0);
+					if (endptr != valstr && errno == 0) {
+						secmdline = val ? 1 : 0;
+					} else {
+						secmdline = 0;
+					}
+				}
+				/* advance past the current substring, latter arguments take precedence */
+				search = tmp + 1;
 			}
 		}
 		fclose(cfg);
-- 
2.50.1


  parent reply	other threads:[~2025-07-24 13:05 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-20 12:52 [PATCH] libselinux: fix parsing of the enforcing kernel cmdline parameter Rahul Sandhu
2025-07-21  9:01 ` robinshao007
2025-07-21  9:47   ` Rahul Sandhu
2025-07-21  9:58     ` robinshao007
2025-07-21 12:56 ` Stephen Smalley
2025-07-21 14:18   ` Stephen Smalley
2025-07-22  5:42     ` Rahul Sandhu
2025-07-22 13:05       ` Stephen Smalley
2025-07-22 15:36         ` Stephen Smalley
2025-07-24  9:13   ` [PATCH v2] " Rahul Sandhu
2025-07-24 12:28     ` Stephen Smalley
2025-07-24 12:33       ` Rahul Sandhu
2025-07-24 13:05       ` Rahul Sandhu [this message]
2025-07-24 13:27         ` [PATCH v3] " Stephen Smalley
2025-07-24 13:30           ` Stephen Smalley
2025-07-24 13:51           ` [PATCH v4] " Rahul Sandhu
2025-07-24 19:29             ` Stephen Smalley
2025-07-25 22:03               ` Rahul Sandhu
2025-07-25 22:15               ` [PATCH v5] " Rahul Sandhu
2025-07-28 14:04                 ` Stephen Smalley
2025-07-30 13:06                   ` Stephen Smalley
     [not found] <CAEjxPJ6-ZbOKxtpbpD4NixZeQy gU6Z3T8C8jLRvCPDHC-mL3w@mail.gmail.com>
2025-07-24 13:40 ` [PATCH v3] " Rahul Sandhu

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=20250724130511.317098-1-nvraxn@gmail.com \
    --to=nvraxn@gmail.com \
    --cc=omosnace@redhat.com \
    --cc=paul@paul-moore.com \
    --cc=selinux@vger.kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    /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 NNTP newsgroup(s).