| 8e135b8a | 02-Mar-2026 |
John Johansen <john.johansen@canonical.com> |
apparmor: fix race between freeing data and fs accessing it
AppArmor was putting the reference to i_private data on its end after removing the original entry from the file system. However the inode
apparmor: fix race between freeing data and fs accessing it
AppArmor was putting the reference to i_private data on its end after removing the original entry from the file system. However the inode can aand does live beyond that point and it is possible that some of the fs call back functions will be invoked after the reference has been put, which results in a race between freeing the data and accessing it through the fs.
While the rawdata/loaddata is the most likely candidate to fail the race, as it has the fewest references. If properly crafted it might be possible to trigger a race for the other types stored in i_private.
Fix this by moving the put of i_private referenced data to the correct place which is during inode eviction.
Fixes: c961ee5f21b20 ("apparmor: convert from securityfs to apparmorfs for policy ns files") Reported-by: Qualys Security Advisory <qsa@qualys.com> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Maxime Bélair <maxime.belair@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| a0b7091c | 24-Feb-2026 |
John Johansen <john.johansen@canonical.com> |
apparmor: fix race on rawdata dereference
There is a race condition that leads to a use-after-free situation: because the rawdata inodes are not refcounted, an attacker can start open()ing one of th
apparmor: fix race on rawdata dereference
There is a race condition that leads to a use-after-free situation: because the rawdata inodes are not refcounted, an attacker can start open()ing one of the rawdata files, and at the same time remove the last reference to this rawdata (by removing the corresponding profile, for example), which frees its struct aa_loaddata; as a result, when seq_rawdata_open() is reached, i_private is a dangling pointer and freed memory is accessed.
The rawdata inodes weren't refcounted to avoid a circular refcount and were supposed to be held by the profile rawdata reference. However during profile removal there is a window where the vfs and profile destruction race, resulting in the use after free.
Fix this by moving to a double refcount scheme. Where the profile refcount on rawdata is used to break the circular dependency. Allowing for freeing of the rawdata once all inode references to the rawdata are put.
Fixes: 5d5182cae401 ("apparmor: move to per loaddata files, instead of replicating in profiles") Reported-by: Qualys Security Advisory <qsa@qualys.com> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Maxime Bélair <maxime.belair@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Tested-by: Salvatore Bonaccorso <carnil@debian.org> Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| 39440b13 | 17-Oct-2025 |
John Johansen <john.johansen@canonical.com> |
apparmor: fix differential encoding verification
Differential encoding allows loops to be created if it is abused. To prevent this the unpack should verify that a diff-encode chain terminates.
Unfo
apparmor: fix differential encoding verification
Differential encoding allows loops to be created if it is abused. To prevent this the unpack should verify that a diff-encode chain terminates.
Unfortunately the differential encode verification had two bugs.
1. it conflated states that had gone through check and already been marked, with states that were currently being checked and marked. This means that loops in the current chain being verified are treated as a chain that has already been verified.
2. the order bailout on already checked states compared current chain check iterators j,k instead of using the outer loop iterator i. Meaning a step backwards in states in the current chain verification was being mistaken for moving to an already verified state.
Move to a double mark scheme where already verified states get a different mark, than the current chain being kept. This enables us to also drop the backwards verification check that was the cause of the second error as any already verified state is already marked.
Fixes: 031dcc8f4e84 ("apparmor: dfa add support for state differential encoding") Reported-by: Qualys Security Advisory <qsa@qualys.com> Tested-by: Salvatore Bonaccorso <carnil@debian.org> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| 6601e13e | 07-Nov-2025 |
John Johansen <john.johansen@canonical.com> |
apparmor: fix unprivileged local user can do privileged policy management
An unprivileged local user can load, replace, and remove profiles by opening the apparmorfs interfaces, via a confused deput
apparmor: fix unprivileged local user can do privileged policy management
An unprivileged local user can load, replace, and remove profiles by opening the apparmorfs interfaces, via a confused deputy attack, by passing the opened fd to a privileged process, and getting the privileged process to write to the interface.
This does require a privileged target that can be manipulated to do the write for the unprivileged process, but once such access is achieved full policy management is possible and all the possible implications that implies: removing confinement, DoS of system or target applications by denying all execution, by-passing the unprivileged user namespace restriction, to exploiting kernel bugs for a local privilege escalation.
The policy management interface can not have its permissions simply changed from 0666 to 0600 because non-root processes need to be able to load policy to different policy namespaces.
Instead ensure the task writing the interface has privileges that are a subset of the task that opened the interface. This is already done via policy for confined processes, but unconfined can delegate access to the opened fd, by-passing the usual policy check.
Fixes: b7fd2c0340eac ("apparmor: add per policy ns .load, .replace, .remove interface files") Reported-by: Qualys Security Advisory <qsa@qualys.com> Tested-by: Salvatore Bonaccorso <carnil@debian.org> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| 796c146f | 25-Dec-2025 |
John Johansen <john.johansen@canonical.com> |
apparmor: split xxx_in_ns into its two separate semantic use cases
This patch doesn't change current functionality, it switches the two uses of the in_ns fns and macros into the two semantically dif
apparmor: split xxx_in_ns into its two separate semantic use cases
This patch doesn't change current functionality, it switches the two uses of the in_ns fns and macros into the two semantically different cases they are used for.
xxx_in_scope for checking mediation interaction between profiles xxx_in_view to determine which profiles are visible.The scope will always be a subset of the view as profiles that can not see each other can not interact.
The split can not be completely done for label_match because it has to distinct uses matching permission against label in scope, and checking if a transition to a profile is allowed. The transition to a profile can include profiles that are in view but not in scope, so retain this distinction as a parameter.
While at the moment the two uses are very similar, in the future there will be additional differences. So make sure the semantics differences are present in the code.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| acf2a94a | 08-Nov-2025 |
John Johansen <john.johansen@canonical.com> |
apparmor: refactor/cleanup cred helper fns.
aa_cred_raw_label() and cred_label() now do the same things so consolidate to cred_label()
Document the crit section use and constraints better and refac
apparmor: refactor/cleanup cred helper fns.
aa_cred_raw_label() and cred_label() now do the same things so consolidate to cred_label()
Document the crit section use and constraints better and refactor __begin_current_label_crit_section() into a base fn __begin_cred_crit_section() and a wrapper that calls the base with current cred.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| 3d28e239 | 02-Apr-2025 |
John Johansen <john.johansen@canonical.com> |
apparmor: add support loading per permission tagging
Add support for the per permission tag index for a given permission set. This will be used by both meta-data tagging, to allow annotating accept
apparmor: add support loading per permission tagging
Add support for the per permission tag index for a given permission set. This will be used by both meta-data tagging, to allow annotating accept states with context and debug information. As well as by rule tainting and triggers to specify the taint or trigger to be applied.
Since these are low frequency ancillary data items they are stored in a tighter packed format to that allows for sharing and reuse of the strings between permissions and accept states. Reducing the amount of kernel memory use at the cost of having to go through a couple if index based indirections.
The tags are just strings that has no meaning with out context. When used as meta-data for auditing and debugging its entirely information for userspace, but triggers, and tainting can be used to affect the domain. However they all exist in the same packed data set and can be shared between different uses.
Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| 6456ccbd | 04-Jun-2025 |
John Johansen <john.johansen@canonical.com> |
apparmor: fix regression in fs based unix sockets when using old abi
Policy loaded using abi 7 socket mediation was not being applied correctly in all cases. In some cases with fs based unix sockets
apparmor: fix regression in fs based unix sockets when using old abi
Policy loaded using abi 7 socket mediation was not being applied correctly in all cases. In some cases with fs based unix sockets a subset of permissions where allowed when they should have been denied.
This was happening because the check for if the socket was an fs based unix socket came before the abi check. But the abi check is where the correct path is selected, so having the fs unix socket check occur early would cause the wrong code path to be used.
Fix this by pushing the fs unix to be done after the abi check.
Fixes: dcd7a559411e ("apparmor: gate make fine grained unix mediation behind v9 abi") Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| 50d56a1a | 14-Jun-2025 |
John Johansen <john.johansen@canonical.com> |
apparmor: fix AA_DEBUG_LABEL()
AA_DEBUG_LABEL() was not specifying it vargs, which is needed so it can output debug parameters.
Fixes: 71e6cff3e0dd ("apparmor: Improve debug print infrastructure")
apparmor: fix AA_DEBUG_LABEL()
AA_DEBUG_LABEL() was not specifying it vargs, which is needed so it can output debug parameters.
Fixes: 71e6cff3e0dd ("apparmor: Improve debug print infrastructure") Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| a30a9fdb | 14-Jun-2025 |
John Johansen <john.johansen@canonical.com> |
apparmor: fix af_unix auditing to include all address information
The auditing of addresses currently doesn't include the source address and mixes source and foreign/peer under the same audit name.
apparmor: fix af_unix auditing to include all address information
The auditing of addresses currently doesn't include the source address and mixes source and foreign/peer under the same audit name. Fix this so source is always addr, and the foreign/peer is peer_addr.
Fixes: c05e705812d1 ("apparmor: add fine grained af_unix mediation") Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| 6afb0a7b | 22-Jun-2025 |
John Johansen <john.johansen@canonical.com> |
apparmor: update kernel doc comments for xxx_label_crit_section
Add a kernel doc header for __end_current_label_crit_section(), and update the header for __begin_current_label_crit_section().
Fixes
apparmor: update kernel doc comments for xxx_label_crit_section
Add a kernel doc header for __end_current_label_crit_section(), and update the header for __begin_current_label_crit_section().
Fixes: b42ecc5f58ef ("apparmor: make __begin_current_label_crit_section() indicate whether put is needed") Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| 87cc7b00 | 18-Mar-2025 |
Mateusz Guzik <mjguzik@gmail.com> |
apparmor: make __begin_current_label_crit_section() indicate whether put is needed
Same as aa_get_newest_cred_label_condref().
This avoids a bunch of work overall and allows the compiler to note wh
apparmor: make __begin_current_label_crit_section() indicate whether put is needed
Same as aa_get_newest_cred_label_condref().
This avoids a bunch of work overall and allows the compiler to note when no clean up is necessary, allowing for tail calls.
This in particular happens in apparmor_file_permission(), which manages to tail call aa_file_perm() 105 bytes in (vs a regular call 112 bytes in followed by branches to figure out if clean up is needed).
Signed-off-by: Mateusz Guzik <mjguzik@gmail.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| aff426f3 | 24-May-2025 |
John Johansen <john.johansen@canonical.com> |
apparmor: mitigate parser generating large xtables
Some versions of the parser are generating an xtable transition per state in the state machine, even when the state machine isn't using the transit
apparmor: mitigate parser generating large xtables
Some versions of the parser are generating an xtable transition per state in the state machine, even when the state machine isn't using the transition table.
The parser bug is triggered by commit 2e12c5f06017 ("apparmor: add additional flags to extended permission.")
In addition to fixing this in userspace, mitigate this in the kernel as part of the policy verification checks by detecting this situation and adjusting to what is actually used, or if not used at all freeing it, so we are not wasting unneeded memory on policy.
Fixes: 2e12c5f06017 ("apparmor: add additional flags to extended permission.") Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| b1f87be7 | 16-Feb-2025 |
John Johansen <john.johansen@canonical.com> |
apparmor: Document that label must be last member in struct aa_profile
The label struct is variable length. While its use in struct aa_profile is fixed length at 2 entries the variable length member
apparmor: Document that label must be last member in struct aa_profile
The label struct is variable length. While its use in struct aa_profile is fixed length at 2 entries the variable length member needs to be the last member in the structure.
The code already does this but the comment has it in the wrong location. Also add a comment to ensure it stays at the end of the structure.
While we are at it, update the documentation for other profile members as well.
Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| a88db916 | 01-May-2025 |
Ryan Lee <ryan.lee@canonical.com> |
apparmor: fix loop detection used in conflicting attachment resolution
Conflicting attachment resolution is based on the number of states traversed to reach an accepting state in the attachment DFA,
apparmor: fix loop detection used in conflicting attachment resolution
Conflicting attachment resolution is based on the number of states traversed to reach an accepting state in the attachment DFA, accounting for DFA loops traversed during the matching process. However, the loop counting logic had multiple bugs:
- The inc_wb_pos macro increments both position and length, but length is supposed to saturate upon hitting buffer capacity, instead of wrapping around. - If no revisited state is found when traversing the history, is_loop would still return true, as if there was a loop found the length of the history buffer, instead of returning false and signalling that no loop was found. As a result, the adjustment step of aa_dfa_leftmatch would sometimes produce negative counts with loop- free DFAs that traversed enough states. - The iteration in the is_loop for loop is supposed to stop before i = wb->len, so the conditional should be < instead of <=.
This patch fixes the above bugs as well as the following nits: - The count and size fields in struct match_workbuf were not used, so they can be removed. - The history buffer in match_workbuf semantically stores aa_state_t and not unsigned ints, even if aa_state_t is currently unsigned int. - The local variables in is_loop are counters, and thus should be unsigned ints instead of aa_state_t's.
Fixes: 21f606610502 ("apparmor: improve overlapping domain attachment resolution")
Signed-off-by: Ryan Lee <ryan.lee@canonical.com> Co-developed-by: John Johansen <john.johansen@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|
| 6c055e62 | 01-May-2025 |
Ryan Lee <ryan.lee@canonical.com> |
apparmor: ensure WB_HISTORY_SIZE value is a power of 2
WB_HISTORY_SIZE was defined to be a value not a power of 2, despite a comment in the declaration of struct match_workbuf stating it is and a mo
apparmor: ensure WB_HISTORY_SIZE value is a power of 2
WB_HISTORY_SIZE was defined to be a value not a power of 2, despite a comment in the declaration of struct match_workbuf stating it is and a modular arithmetic usage in the inc_wb_pos macro assuming that it is. Bump WB_HISTORY_SIZE's value up to 32 and add a BUILD_BUG_ON_NOT_POWER_OF_2 line to ensure that any future changes to the value of WB_HISTORY_SIZE respect this requirement.
Fixes: 136db994852a ("apparmor: increase left match history buffer size")
Signed-off-by: Ryan Lee <ryan.lee@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com>
show more ...
|