lted in a
kernel BUG print due to the null pointer dereference.
To avoid this issue, first check that (attach->xmatch) is not null.
The one-line patch is attached to the email.
Ryan
From b1ac2f6e110b0281a19b65b9005b019c0e996b12 Mon Sep 17 00:00:00 2001
From: Ryan Lee
Date: Mon, 19 Aug 2024 11
1:05 PM Ryan Lee wrote:
>
> find_attach loops over profile entries and first checks for a DFA, falling
> back onto a strcmp otherwise. However, the check if (attach->xmatch->dfa)
> did not account for the possibility that (attach->xmatch) could be null.
> This occured wi
I just realized that I forgot to add sign off on my patch, so I'm
resending it with the Signed-off-by line added.
On Wed, Aug 21, 2024 at 11:12 AM Ryan Lee wrote:
>
> After further analysis, the root cause turned out to be the xmatch not
> being set up properly when allocating a nu
ff-by: Ryan Lee
From 010a01ba597eeae87d34617da453664d84e696b1 Mon Sep 17 00:00:00 2001
From: Ryan Lee
Date: Fri, 23 Aug 2024 10:14:02 -0700
Subject: [PATCH] apparmor: properly handle cx/px lookup failure for complain
mode profiles
When a cx/px lookup fails, apparmor would deny execution of the b
o's secure-execution mode.
Link: https://gitlab.com/apparmor/apparmor/-/merge_requests/1315 is a
merge request that does similar updating for apparmor userspace.
Signed-off-by: Ryan Lee
---
security/apparmor/domain.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff -
The variable aa_unprivileged_uring_restricted is still exposed to
userspace even when CONFIG_IO_URING is disabled and the variable would
do nothing. This patch hides both the apparmorfs entry and the sysctl
when CONFIG_IO_URING is disabled.
Signed-off-by: Ryan Lee
---
security/apparmor
This adds a helper for apparmor sysctls to allow world-read, root-write
unsigned integer sysctls. This is used by the next patch in the series.
Signed-off-by: Ryan Lee
---
security/apparmor/lsm.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/security/apparmor/lsm.c b/security
ter denial+audit occurred on a different CPU.
To resolve this, record the last time a capability was audited for a
profile and add a timestamp expiration check before doing the audit. This
first patch hardcodes a small duration for the timeout period.
Signed-off-by: Ryan Lee
---
security/apparmor/ca
Instead of hardcoding the Apparmor capability audit cache timeout, expose
it as a sysctl. This uses the helper introduced in the previous patch of
this series.
Signed-off-by: Ryan Lee
---
security/apparmor/capability.c | 6 --
security/apparmor/include/capability.h | 2 ++
security
its very first use of ad is to dereference ad->class without
checking if ad is NULL.
Thus, document profile_capable's ad parameter as not accepting NULL.
Signed-off-by: Ryan Lee
---
security/apparmor/capability.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/
sen.
- Cache the expiration time instead of the last-audited time. This value
can never be zero, which lets us drop the kernel_cap_t caps field from
the cache struct.
Signed-off-by: Ryan Lee
---
security/apparmor/capability.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
Noting for additional clarity that I have withdrawn this RFC patchset
in favor of a later PATCH v2 with the same subject line.
On Fri, Sep 13, 2024 at 4:21 PM Ryan Lee wrote:
>
> When auditing capabilities, AppArmor uses a per-CPU, per-profile cache
> such that the same capability for
profile) or a profile was attached to a container and its
processes.
Instead, perform audit_cap deduping over ad->subj_cred, which ensures the
deduping only occurs across a single process, instead of across all
processes that match the current one's profile.
Signed-off-by: Ryan Lee
---
uot;getattr" denied="getattr"
that audit the denial as an unlink instead of as a getattr.
Not only was apparmor_inode_getattr passing in a hardcoded OP_UNLINK
to the common_mqueue_path_perm helper, but the helper was also discarding
the op argument and auditing as a hardcoded OP_UNLINK
> /@{HOME}/.foo_file rw,
>
>
>
>
>
> Is the example incorrect?
>
>
>
> Thanks,
>
>
>
> Ian
>
>
>
>
>
>
>
> From: Ryan Lee
> Sent: Friday, February 7, 2025 1:06 PM
> To: Ian Merin
> Cc: apparmor@lists.ubuntu.com
> Subje
Hi Ian,
Can you check if the rule
@{lib}/**.so* mr,
works for you?
If so, the issue is that your use of the variable creates a rule that
starts with two slashes, which currently isn't collapsed down into a single
slash. You can check https://gitlab.com/apparmor/apparmor/-/issues/450 for
more in
Due to a soft scheduling conflict I would prefer the December 17th
date, although I would still be able to make it to a December 11th
meeting.
On Tue, Dec 10, 2024 at 1:42 AM John Johansen
wrote:
>
> The next irc meeting planned for Tuesday Dec 10, at 18:00 UTC in #apparmor on
> oftc.net is post
-- Forwarded message -
From: Tanya Agarwal
Date: Thu, Jan 23, 2025 at 11:30 AM
Subject: [PATCH] apparmor: fix typos and spelling errors
To: , ,
,
Cc: ,
, ,
, Tanya Agarwal ,
Mimi Zohar
From: Tanya Agarwal
Fix typos and spelling errors in apparmor module comments that were
ide
Reviewed-by: Ryan Lee
On Thu, Jan 23, 2025 at 11:30 AM Tanya Agarwal
wrote:
>
> From: Tanya Agarwal
>
> Fix typos and spelling errors in apparmor module comments that were
> identified using the codespell tool.
> No functional changes - documentation only.
>
> Sign
For the record, a previous patch that removes the sock variable was
previously accepted:
https://lists.ubuntu.com/archives/apparmor/2025-January/013449.html
(patch) and https://lists.ubuntu.com/archives/apparmor/2025-January/013463.html
(ack from John Johansen)
On Mon, Jan 27, 2025 at 12:54 PM Mat
O_PATH fd
inheritance better, the best we can do for now is disable the AA_BUG line
when the profile is in complain mode.
Signed-off-by: Ryan Lee
---
v1 -> v2: only skip the AA_BUG line in complain mode
security/apparmor/file.c | 14 +-
1 file changed, 13 insertions(+), 1 dele
This is a variant of AA_SFS_TYPE_BOOLEAN that gets printed to userspace
as the integers 0/1 instead of as the strings "no"/"yes", for backwards
compatibility with userspace applications expecting integer values for
semantic booleans.
Signed-off-by: Ryan Lee
---
security/a
/unconfined_restrictions/userns
yes
After this patchset (matching the old behavior):
$ cat
/sys/kernel/security/apparmor/features/policy/unconfined_restrictions/userns
1
Signed-off-by: Ryan Lee
---
security/apparmor/apparmorfs.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a
parts of our userspace were expecting 0/1 values for these sysctls
instead. Because there may have been other (Ubuntu-specific) consumers of
these sysctls expecting 0/1 values, we should fix the API break instead of
just fixing our own userspace patches.
Ryan Lee (2):
apparmor: create an
address_family_names and sock_type_names were created as const char *a[],
which declares them as (non-const) pointers to const chars. Since the
pointers themselves would not be changed, they should be generated as
const char *const a[].
Signed-off-by: Ryan Lee
---
security/apparmor/Makefile | 4
find_attach may set info if something unusual happens during that process
(currently only used to signal conflicting attachments, but this could be
expanded in the future). This is information that should be propagated to
userspace via an audit message.
Signed-off-by: Ryan Lee
---
security
Instead of having a literal, making this a constant will allow for (hacky)
detection of conflicting profile attachments from inspection of the info
pointer. This is used in the next patch to augment the information provided
through domain.c:x_to_label for ix/ux fallback.
Signed-off-by: Ryan Lee
error.
Signed-off-by: Ryan Lee
---
security/apparmor/domain.c | 33 +++--
1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index a73307ee1c7f..e8cd9badfb54 100644
--- a/security/apparmor/domain.c
+++ b
Conflicting attachment paths are an error state that result in the
binary in question executing under an unexpected ix/ux fallback. As such,
it should be audited to record the occurrence of conflicting attachments.
Signed-off-by: Ryan Lee
---
security/apparmor/domain.c | 7 +++
1 file
should be audited unconditionally.
This
patchset implements such auditing.
Ryan Lee (4):
apparmor: force audit on unconfined exec if info is set by find_attach
apparmor: move the "conflicting profile attachments" infostr to a
const declaration
apparmor: include conflicting attac
(void) aa_unpack_u32(e, &profile->signal, "kill");
> - if (profile->signal < 1 && profile->signal > MAXMAPPED_SIG) {
> + if (profile->signal < 1 || profile->signal > MAXMAPPED_SIG) {
> info = "profile kill.signal invalid value";
> goto fail;
> }
> --
> 2.49.0
Reviewed-by: Ryan Lee
>
>
ne 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
---
security/apparmor/include/match.h | 3 ++-
security/apparmor/match.c | 1 +
2 f
enable conflicting attachments to be detected in more cases.
Ryan Lee (2):
apparmor: ensure WB_HISTORY_SIZE value is a power of 2
apparmor: fix loop detection used in conflicting attachment resolution
security/apparmor/include/match.h | 8 +++-
security/apparmor/match.c | 23
tate_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")
Co-developed-by: John Johansen
Signed-off-by: John Johan
struct aa_perms *addend)
> @@ -128,8 +128,8 @@ static inline void aa_perms_accum_raw(st
>
> /**
> * aa_perms_accum - accumulate perms, masking off overlapping perms
> - * @accum - perms struct to accumulate into
> - * @addend - perms struct to add to @accum
> + * @accum: perms struct to accumulate into
> + * @addend: perms struct to add to @accum
> */
> static inline void aa_perms_accum(struct aa_perms *accum,
> struct aa_perms *addend)
>
With the commit message addition that I mentioned above:
Reviewed-by: Ryan Lee
The function in question does not just verify the header but also does
unpacking, so give the function a more descriptive name.
Signed-off-by: Ryan Lee
---
security/apparmor/policy_unpack.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/security/apparmor
These functions are not supposed to change the profile struct (or
component thereof), so make the pointers passed into them const.
Signed-off-by: Ryan Lee
---
security/apparmor/policy_unpack.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/security/apparmor
The constified pointer arguments are not actually modified, so encode this
into the function prototypes.
Signed-off-by: Ryan Lee
---
security/apparmor/audit.c | 10 +-
security/apparmor/include/audit.h | 10 +-
security/apparmor/include/policy.h | 2 +-
3 files
Unfortunately, the profile pointer itself cannot be constified because
aa_audit itself needs to extract non-const pointers from the profile
struct.
Signed-off-by: Ryan Lee
---
security/apparmor/mount.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/apparmor
The profile argument is not used by this function, so don't pass it in.
Signed-off-by: Ryan Lee
---
security/apparmor/file.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index 6acf0f029604..a59ba26e54d6 100644
allperms represents a permissions set that allows everything, so it should
never be changed by any of the code using it. Making it const will allow
this to be enforced or warned on at compile time.
Signed-off-by: Ryan Lee
---
security/apparmor/include/perms.h | 2 +-
security/apparmor/lib.c
nullperms and allperms are supposed to be constants that don't change. As
we can't update nullperms and the pointer arg to be const due to
prompt-mode profile updates in aa_audit_file, this is the next best way
to prevent nullperms from being written to.
Signed-off-by: Ryan Lee
---
The aa_audit_file function doesn't modify aa_perms, so pass the pointer
in as const.
Signed-off-by: Ryan Lee
---
security/apparmor/file.c | 2 +-
security/apparmor/include/file.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/apparmor/file.c b/sec
nullperms represents a permissions set that allows nothing, so it should
never be changed by any of the code using it. Making it const will allow
this to be enforced or warned on at compile time.
Signed-off-by: Ryan Lee
---
This patch requires "apparmor: constify some of the pointer argumen
binary not covered by a profile's execution rules. Because of the
path aliasing that can occur in situations with disconnected paths, do not
behave as if attach_disconnected was specified as a profile flag (unless,
of course, the loaded profile itself has that flag set).
Signed-off-by: Rya
h complain
mode profiles.
Similar checks will be needed for disconnection in the IPC case, once that
code is ready.
Ryan Lee (5):
apparmor: pass complain-mode information to aa_path_name path lookup
apparmor: don't return early in profile_path_perm for disconnected
paths in complain
This is the analogous change to profile_path_perm in an earlier patch of
this patchset, except for the mount mediation functions.
Signed-off-by: Ryan Lee
---
security/apparmor/mount.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/security/apparmor/mount.c b
O_PATH fd
inheritance better, the best we can do for now is disable the AA_BUG line.
Signed-off-by: Ryan Lee
---
security/apparmor/file.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index c430e031db31..32
w the
operation to proceed while generating a complain-mode audit log.
Signed-off-by: Ryan Lee
---
security/apparmor/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index 5e0cadb75651..c430e031db31 100644
--- a/security/app
aa_map_file_to_perms does not change the file, so the info extracted into
the allow variable can be used for the call to aa_mqueue_perm instead of
being computed again.
Signed-off-by: Ryan Lee
---
security/apparmor/lsm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a
We aren't expecting a null path mnt pointer here, and file_mnt_idmap
dereferences it without checking for null, leading to a null pointer
dereference BUG print. Instead, explicitly check for this via AA_BUG
line in order to get more useful diagnostics.
Signed-off-by: Ryan Lee
---
sec
On Wed, Mar 5, 2025 at 1:47 PM Malte Schröder wrote:
>
> On 05/03/2025 20:22, Ryan Lee wrote:
> > On Wed, Mar 5, 2025 at 11:11 AM Malte Schröder
> > wrote:
> >> Hi,
> >>
> >> I hope this is the right place to report this. Since 6.14-rc1 ff. resume
&
as special disconnected paths that may be unreliable.
This patch implements the sentinel character placement, adds a
profile_complain argument to the path lookup functions, and passes the
complain-mode information in at all the places where aa_path_perm is
called.
Signed-off-by: Ryan Lee
---
On Wed, Mar 5, 2025 at 11:11 AM Malte Schröder wrote:
>
> Hi,
>
> I hope this is the right place to report this. Since 6.14-rc1 ff. resume
> from hibernate does not work anymore. Now I finally managed to get dmesg
> from when this happens (Console is frozen, but managed to login via
> network). If
as special disconnected paths that may be unreliable.
This patch implements the sentinel character placement, adds a
profile_complain argument to the path lookup functions, and passes the
complain-mode information in at all the places where aa_path_perm is
called.
Signed-off-by: Ryan Lee
---
w the
operation to proceed while generating a complain-mode audit log.
Signed-off-by: Ryan Lee
---
security/apparmor/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index 5e0cadb75651..c430e031db31 100644
--- a/security/app
This is the analogous change to profile_path_perm in an earlier patch of
this patchset, except for the mount mediation functions.
Signed-off-by: Ryan Lee
---
security/apparmor/mount.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/security/apparmor/mount.c b
disable aa_audit_file AA_BUG(!ad.request) due to fd
inheritance": only skip the AA_BUG line in complain mode
Ryan Lee (5):
apparmor: pass complain-mode information to aa_path_name path lookup
apparmor: don't return early in profile_path_perm for disconnected
paths in complain mode
ap
On Sat, Mar 8, 2025 at 3:22 AM Christian Boltz wrote:
>
> Hello,
>
> Am Dienstag, 4. März 2025, 21:55 schrieb Ryan Lee:
> > +* error should only be set at this point if we're complain
> > mode
>
> Nitpick: ... if we're _in_ complain mode
Fixed in
Followed up with a v2 patchset
On Tue, Mar 4, 2025 at 12:56 PM Ryan Lee wrote:
>
> AppArmor was previously blocking operations with disconnected paths, even
> when the profile was loaded in complain mode. Instead, this patchset audits
> the disconnected path as being prefixed with a
On Sat, Mar 8, 2025 at 11:21 AM Christian Boltz wrote:
>
> Hello,
>
> Am Dienstag, 4. März 2025, 21:55 schrieb Ryan Lee:
> > Inheritance of fd's triggers the lookup logic, and O_PATH fd's are
> > checked with an empty request set. If the O_PATH fd corresponds
binary not covered by a profile's execution rules. Because of the
path aliasing that can occur in situations with disconnected paths, do not
behave as if attach_disconnected was specified as a profile flag (unless,
of course, the loaded profile itself has that flag set).
Signed-off-by: Rya
Now that O_PATH fds are being passed to the file_open hook,
unconditionally skip mediation of them to preserve existing behavior.
Signed-off-by: Ryan Lee
---
security/tomoyo/file.c | 4
1 file changed, 4 insertions(+)
diff --git a/security/tomoyo/file.c b/security/tomoyo/file.c
index
Now that O_PATH fds are being passed to the file_open hook,
unconditionally skip mediation of them to preserve existing behavior.
Signed-off-by: Ryan Lee
---
security/smack/smack_lsm.c | 4
1 file changed, 4 insertions(+)
diff --git a/security/smack/smack_lsm.c b/security/smack
.
Signed-off-by: Ryan Lee
---
security/apparmor/lsm.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 11ace667cbbf..2349a1dd41f4 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -647,6 +647,16 @@ static int
Currently, opening O_PATH file descriptors completely bypasses the LSM
infrastructure. Invoking the LSM file_open hook for O_PATH fds will
be necessary for e.g. mediating the fsmount() syscall.
Signed-off-by: Ryan Lee
---
fs/open.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion
Landlock currently does not have handling of O_PATH fds. Now that they
are being passed to the file_open hook, explicitly skip mediation of
them until we can handle them.
Signed-off-by: Ryan Lee
---
security/landlock/fs.c | 8
1 file changed, 8 insertions(+)
diff --git a/security
Now that O_PATH fds are being passed to the file_open hook,
unconditionally skip mediation of them to preserve existing behavior.
Signed-off-by: Ryan Lee
---
security/selinux/hooks.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
one wants to handle O_PATH fd creation.
Ryan Lee (6):
fs: invoke LSM file_open hook in do_dentry_open for O_PATH fds as well
apparmor: explicitly skip mediation of O_PATH file descriptors
landlock: explicitly skip mediation of O_PATH file descriptors
selinux: explicitly skip mediation of
On Thu, Feb 13, 2025 at 7:58 AM Hector Cao wrote:
>
> For executables dynamically linked to libnuma, the runtimer linker
> invokes libnuma functions (num_init) that try to access
> /sys/devices/system/node/ and if the application's apparmor
> profile does not allow this access, this access will be
Hi Jordan,
This is an issue that was reported in Launchpad at
https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/2098838. We
are aware of what's broken and am working on a fixed AppArmor package
that should resolve this breakage.
Ryan
On Thu, Feb 20, 2025 at 5:49 AM Jordan Esh wrote:
>
>
ract profile can be included by applications that are
> + # dynamically linked to libnuma
> + # libnuma defines the function num_init() as the .init function
> + # to be called by the runtime linker (ld) when libnuma is loaded
> +
> + @{sys}/devices/system/cpu/node/ r,
> +
> + # Include additions to the abstraction
> + include if exists
> --
> 2.45.2
>
Reviewed-by: Ryan Lee
lib/retpoline.S)
path_name (security/apparmor/file.c)
profile_path_perm (security/apparmor/file.c)
aa_file_perm (security/apparmor/file.c)
Switch the allocation flag for that call to GFP_ATOMIC instead.
Signed-off-by: Ryan Lee
---
security/apparmor/file.c | 2 +-
1 file changed, 1 insertion(+), 1
On Tue, Feb 25, 2025 at 1:06 AM Hector Cao wrote:
>
> For executables dynamically linked to libnuma, the runtimer linker
> invokes libnuma functions (num_init) that try to access
> /sys/devices/system/node/ and if the application's apparmor
> profile does not allow this access, this access will be
Forwarding message thread to the AppArmor mailing list so that it also
has a record of this patch.
-- Forwarded message -
From: Christian Brauner
Date: Mon, Jun 16, 2025 at 7:23 AM
Subject: Re: [PATCH] apparmor: file never has NULL f_path.mnt
To: Al Viro
Cc: ,
On Sun, Jun 15,
Conflicting attachment paths are an error state that result in the
binary in question executing under an unexpected ix/ux fallback. As such,
it should be audited to record the occurrence of conflicting attachments.
Signed-off-by: Ryan Lee
---
This is a v2 of https://lists.ubuntu.com/archives
This section of profile_transition that occurs after x_to_label only
happens if perms.allow already has the MAY_EXEC bit set, so we don't need
to set it again.
Fixes: 16916b17b4f8 ("apparmor: force auditing of conflicting attachment execs
from confined")
Signed-off-by: Ryan Lee
On Sun, Jun 22, 2025 at 3:00 PM John Johansen
wrote:
>
> On 6/13/25 09:32, Ryan Lee wrote:
> > Conflicting attachment paths are an error state that result in the
> > binary in question executing under an unexpected ix/ux fallback. As such,
> > it should be audited to
Update: turns out that this patch has a small but critical
typographical error (both the perms modification lines should be under
the conditional in braces), so we'll be sending a fixed patch as a v2.
On Thu, May 1, 2025 at 5:56 PM Ryan Lee wrote:
>
> Conflicting attachment paths a
79 matches
Mail list logo
