src/sys/security/mac_do/mac_do.c, branch main

MAC/do: Clarify comments about flags attached per-ID or per-ID-type

2026-06-04T11:06:39Z

No functional change. MFC after: 3 days Sponsored by: The FreeBSD Foundation

MAC/do: Fix double-free on parse error after "executable paths" feature

2026-06-01T15:23:51Z

parse_rules() has been calling toast_rules() in case of a parse error in order to deallocate the 'struct rule' objects it has constructed up to that point. toast_rules() would take a pointer to a full 'struct rules' object, and besides freeing all 'struct rule' referenced by it, would also free the holding 'struct rules' itself. With the introduction of the "executable paths" feature, and the embedding of 'struct rules' into 'struct conf', meaning that the lifecycle for 'struct rules' was no longer independent, toast_rules() was changed not to free the passed 'struct rules' (as it was a field of a 'struct conf' object). Unfortunately, this change was not completed with a reinitialization of the rules list head, so the 'struct conf' object would continue to reference just-freed rules, which then would be freed a second time on destruction of that container. So, make toast_rules() re-initialize the rules list in 'struct rules', which it logically has been having to do since not freeing the enclosing 'struct rules'. This alone is enough to fix the bug, but let's use the occasion to change the contract of parse_rules() and bring its herald comment up-to-date: On error, parse_rules() now simply leaves already constructed 'struct rule' objects in 'conf'. The latter is eventually destroyed and the rule objects reclaimed at that point. Add a test trying to set an invalid rules configuration with the first rule being valid and the second being invalid, which triggers the bug (and an immediate panic() on an INVARIANTS kernel). Reported by: impost0r(ret2plt) Reviewed by: markj Fixes: 9818224174c4 ("MAC/do: Executable paths feature (GSoC 2025's final state)") Sponsored by: The FreeBSD Foundation

MAC/do: Update copyright

2026-05-29T15:39:20Z

Update years for the Foundation. While here, remove the initial '/*-' which has been useless for a long time. While here, add a missing space on bapt@'s copyright line (approved by him). Reviewed by: bapt MFC after: 1 month Sponsored by: The FreeBSD Foundation Pull Request: https://ron-dev.freebsd.org/FreeBSD/src/pulls/38

MAC/do: Do not skip blanks when parsing executable paths

2026-05-29T15:37:14Z

The kind of tolerance we apply to parsing rules, whose format we have defined, cannot be applied to paths since blank characters are allowed there. There is still the limitation that no escape character is currently supported, and so it is not possible to configure a path having a ':' character. Reviewed by: bapt Fixes: 9818224174c4 ("MAC/do: Executable paths feature (GSoC 2025's final state)") MFC after: 1 month Sponsored by: The FreeBSD Foundation Pull Request: https://ron-dev.freebsd.org/FreeBSD/src/pulls/38

MAC/do: Serialize installing/modifying some jail's configuration

2026-05-29T15:36:29Z

See the immediately preceding commit for explanations on what this is fixing. When setting 'mac.do' to 'inherit' on a jail with 'mac.do.rules' and 'mac.do.exec_paths' also specified in the same call, ensure that the check that these passed parameters are the same as those to be inherited is atomic with respect to enabling the inheritance (i.e., removing the jail's 'struct conf' object). (See previous commit "MAC/do: Fix the recent logic to set jail parameters, make it more tolerant" as for why this check exists.) Because we currently only modify a single configuration object per transaction, we introduce the parse_and_commit_conf() wrapper around parse_and_set_conf() to remove duplicated code that would ensue from calling the latter directly, namely, releasing the 'mac_do_rwl' lock and freeing the old configuration object (if any). Taking the 'mac_do_rwl' lock for writing as a way to freeze all accesses to mac_do(4) configurations was deemed too thin an operation to be worth wrapping. Reviewed by: bapt (older version) Fixes: 9818224174c4 ("MAC/do: Executable paths feature (GSoC 2025's final state)") MFC after: 1 month Sponsored by: The FreeBSD Foundation Pull Request: https://ron-dev.freebsd.org/FreeBSD/src/pulls/38

MAC/do: Support for atomically modifying configurations

2026-05-29T15:35:19Z

As mentioned in previous commits "MAC/do: parse_and_set_conf(): Require the model configuration" and "MAC/do: Sequential consistency for configuration retrieval", the introduction of the "executable path" feature, more fundamentally, the fact that there is now more than one per-jail parameter and that parameters can be independently modified or copied, causes an atomicity problem in case of concurrent accesses to of a jail's applicable configuration. Partially modifying a configuration is indeed akin to a read-modify-write operation, where the read is either to the current or an inherited configuration. More precisely, once pointed to by a jail, a configuration object is immutable, and changing the jail's configuration means making the jail point to another configuration object. To change a jail's configuration, a new configuration object is thus built, and if only some parameters have been explicitly specified, those that have not been are set by copying the corresponding values from an existing configuration object (in case of partial modification of the existing configuration, from the original configuration object that is going to be replaced; in case of breakage of inheritance or at jail creation, from the applicable configuration object, which is on an ancestor jail). This process is not immune to concurrent modifications because nothing prevents changes of configurations between reading existing values and setting the new configuration. Thus, some other thread could change the value of a parameter after a copy of it has been made into the new object but before that copy is actually installed, which effectively will erase the other thread's modification. To avoid this, we introduce support for serializing configuration changes on a given jail. To this end, we move the jail climbing process from find_conf() into find_conf_locked(), and make the former call the latter in a read-locked section. Similarly, we isolate setting a configuration in the new set_conf_locked() function, and make set_conf() call it inside a write-locked section. The new *_unlocked() variants make it possible to prevent any configuration access between determining and reading an applicable configuration, computing from it a new configuration object and finally setting it, by holding a write lock over the whole process (there is a trade-off here, as read-mostly locks cannot be upgraded), effectively making it atomic and realizing full sequential consistency of configuration changes. Also, the 'mac_do_rm' global read-mostly lock is made sleepable so that it can be write-locked over sysctl_handle*() functions or memory allocations (eases implementation, at the expense of a potential loss of concurrency which is most probably irrelevant). No functional change (intended) at this point. Reviewed by: bapt Fixes: 9818224174c4 ("MAC/do: Executable paths feature (GSoC 2025's final state)") MFC after: 1 month Sponsored by: The FreeBSD Foundation Pull Request: https://ron-dev.freebsd.org/FreeBSD/src/pulls/38

MAC/do: Sequential consistency for configuration retrieval

2026-05-29T15:34:04Z

Since the inception of mac_do(4), find_conf(), used to retrieve the applicable configuration, has been weakly consistent with respect to concurrent modifications to configuration inheritance that influence its result (and it has been sequentially consistent with respect to other configuration modifications, which the initial executable paths feature and introduction of implicit parameters broke and which will be fixed in a subsequent commit). Indeed, find_conf() climbs the jail tree to find an applicable configuration, which is not an atomic operation. It examines the current jail's configuration pointer for each browsed jail, which does not prevent concurrent modifications of the configuration pointer for jails below or above it. Modifications above the current jail are not a problem, since if climbing needs to continue (i.e., the current jail inherits), these modifications will be seen if performed before that check (and may or may not be seen if performed after that check). However, modifications below the current jail impair sequential consistency, because they could be done before other modifications at the current jail or higher up, and the latter could still be picked up by the rest of the climb, effectively ignoring that the former should have blocked the climb earlier, making them look as if they had happened after for the climbing thread. As a concrete example of this situation, let's examine a scenario where some jail A is the parent of some jail B, and B inherits its configuration from A. An administrator may want to relax the rules only for jail A but not B. To this end, he first copies the current rules on B over to A and then relaxes the rules on A. He can intuitively and reasonably expect that changing B's rules first will prevent A's relaxed rules to leak to threads in B. Unfortunately, that is not the case: As explained in the previous paragraph, there can be a time window where threads from B can still pick up A's new configuration just after it has been installed. This arguably makes changing inheritance in mac_do(4) in a fully secure fashion almost impossible. If preserving fine-grained locking of prisons, we could prevent this problem by having find_conf(), once it has climbed to a non-NULL pointer (actual, non-inherited configuration), do another climb to check that it can reach the same configuration on the same jail again. If the new climb gives another pointer or jail, it could make it the new candidate and do a climb check again until the situation stabilizes. A climb check detects whether changes in jails below that of the candidate configuration object happened, catching in particular such changes that happened before changes to the candidate object. However, that process alone would still be subject to ABA problems, and we would additionally need to tag each prison with some modification timestamp (global, or local but necessitating allocating memory during the check) to fix them. In the end, we considered this direction to be unnecessarily complex, given that configuration changes are to be rare events and most uses will just be configuration determination. Consequently, switch protecting jail configurations with a single read-mostly lock. While here, adapt set_conf() to accept NULL as the new configuration object, and have remove_conf() call it, which removes duplicated code. While here, add a comment explaining why we do not need to take any more locks when climbing the jail tree. Reviewed by: bapt MFC after: 1 month Sponsored by: The FreeBSD Foundation Pull Request: https://ron-dev.freebsd.org/FreeBSD/src/pulls/38

MAC/do: Comment to explain the main invariant for configurations

2026-05-29T15:33:24Z

Once visible, configuration structures must *never* change. Spell that out in a comment to help future readers/contributors understand the design. Reviewed by: bapt MFC after: 1 month Sponsored by: The FreeBSD Foundation Pull Request: https://ron-dev.freebsd.org/FreeBSD/src/pulls/38

MAC/do: Allocate only one default configuration

2026-05-29T15:32:44Z

When mac_do(4) is loaded, all jails get the same default configuration (disabled, with only one allowed executable path: '/usr/bin/mdo'). Share it between all jails instead of creating a separate copy for each. Reviewed by: bapt Fixes: 9818224174c4 ("MAC/do: Executable paths feature (GSoC 2025's final state)") MFC after: 1 month Sponsored by: The FreeBSD Foundation Pull Request: https://ron-dev.freebsd.org/FreeBSD/src/pulls/38

MAC/do: Visually separate some file sections

2026-05-29T15:29:02Z

With additional empty lines. No functional change (intended). Reviewed by: bapt MFC after: 1 month Sponsored by: The FreeBSD Foundation Pull Request: https://ron-dev.freebsd.org/FreeBSD/src/pulls/38