From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Gabriel Totev <gabriel.totev@zetier.com>,
John Johansen <john.johansen@canonical.com>,
Sasha Levin <sashal@kernel.org>,
apparmor@lists.ubuntu.com
Subject: [PATCH AUTOSEL 6.16-6.6] apparmor: shift ouid when mediating hard links in userns
Date: Fri, 8 Aug 2025 11:30:41 -0400 [thread overview]
Message-ID: <20250808153054.1250675-1-sashal@kernel.org> (raw)
From: Gabriel Totev <gabriel.totev@zetier.com>
[ Upstream commit c5bf96d20fd787e4909b755de4705d52f3458836 ]
When using AppArmor profiles inside an unprivileged container,
the link operation observes an unshifted ouid.
(tested with LXD and Incus)
For example, root inside container and uid 1000000 outside, with
`owner /root/link l,` profile entry for ln:
/root$ touch chain && ln chain link
==> dmesg
apparmor="DENIED" operation="link" class="file"
namespace="root//lxd-feet_<var-snap-lxd-common-lxd>" profile="linkit"
name="/root/link" pid=1655 comm="ln" requested_mask="l" denied_mask="l"
fsuid=1000000 ouid=0 [<== should be 1000000] target="/root/chain"
Fix by mapping inode uid of old_dentry in aa_path_link() rather than
using it directly, similarly to how it's mapped in __file_path_perm()
later in the file.
Signed-off-by: Gabriel Totev <gabriel.totev@zetier.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
**Backport Status: YES**
This commit should be backported to stable kernel trees for the
following reasons:
1. **Clear Bug Fix**: This fixes a real bug where AppArmor incorrectly
reports the unshifted uid when mediating hard link operations inside
user namespaces/containers. The commit message provides a concrete
example showing ouid=0 instead of the expected ouid=1000000 in
container logs.
2. **Security Impact**: This is a security-relevant bug that causes
AppArmor policy enforcement to behave incorrectly in containerized
environments. The owner-based AppArmor rules (like `owner /root/link
l,`) won't work correctly because the uid comparison is done with the
wrong (unshifted) value.
3. **Minimal and Contained Fix**: The change is small and surgical - it
only modifies the aa_path_link() function to properly map the inode
uid through the mount's idmapping, following the exact same pattern
already used in __file_path_perm():
- Uses `i_uid_into_vfsuid(mnt_idmap(target.mnt), inode)` to get the
vfsuid
- Converts it back with `vfsuid_into_kuid(vfsuid)` for the path_cond
structure
4. **No New Features or Architecture Changes**: This is purely a bug fix
that makes aa_path_link() consistent with how __file_path_perm()
already handles uid mapping. No new functionality is added.
5. **Container/User Namespace Compatibility**: With the widespread use
of containers (LXD, Incus, Docker with userns), this bug affects many
production systems. The fix ensures AppArmor policies work correctly
in these environments.
6. **Low Risk**: The change follows an established pattern in the
codebase (from __file_path_perm) and only affects the specific case
of hard link permission checks in user namespaces. The risk of
regression is minimal.
7. **Clear Testing**: The commit message indicates this was tested with
LXD and Incus containers, showing the issue is reproducible and the
fix validated.
The code change is straightforward - replacing direct access to
`d_backing_inode(old_dentry)->i_uid` with proper idmapping-aware
conversion that respects user namespace uid shifts. This makes
aa_path_link() consistent with other AppArmor file operations that
already handle idmapped mounts correctly.
security/apparmor/file.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index d52a5b14dad4..62bc46e03758 100644
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -423,9 +423,11 @@ int aa_path_link(const struct cred *subj_cred,
{
struct path link = { .mnt = new_dir->mnt, .dentry = new_dentry };
struct path target = { .mnt = new_dir->mnt, .dentry = old_dentry };
+ struct inode *inode = d_backing_inode(old_dentry);
+ vfsuid_t vfsuid = i_uid_into_vfsuid(mnt_idmap(target.mnt), inode);
struct path_cond cond = {
- d_backing_inode(old_dentry)->i_uid,
- d_backing_inode(old_dentry)->i_mode
+ .uid = vfsuid_into_kuid(vfsuid),
+ .mode = inode->i_mode,
};
char *buffer = NULL, *buffer2 = NULL;
struct aa_profile *profile;
--
2.39.5
next reply other threads:[~2025-08-08 15:30 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-08 15:30 Sasha Levin [this message]
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.10] md: dm-zoned-target: Initialize return variable r to avoid uninitialized use Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.4] i3c: don't fail if GETHDRCAP is unsupported Sasha Levin
2025-08-08 15:30 ` Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.10] dm-mpath: don't print the "loaded" message if registering fails Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.10] rtc: ds1307: handle oscillator stop flag (OSF) for ds1341 Sasha Levin
2025-08-11 16:46 ` Meagan Lloyd
2025-08-16 13:07 ` Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.4] i3c: add missing include to internal header Sasha Levin
2025-08-08 15:30 ` Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-6.1] i3c: master: Initialize ret in i3c_i2c_notifier_call() Sasha Levin
2025-08-08 15:30 ` Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.4] PCI: pnv_php: Work around switches with broken presence detection Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-6.1] module: Prevent silent truncation of module name in delete_module(2) Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-6.1] apparmor: use the condition in AA_BUG_FMT even with debug disabled Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-6.6] powerpc/eeh: Make EEH driver device hotplug safe Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-6.12] apparmor: fix x_table_lookup when stacking is not the first entry Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-6.1] dm-table: fix checking for rq stackable devices Sasha Levin
2025-08-08 15:30 ` [PATCH AUTOSEL 6.16-5.10] PCI: pnv_php: Clean up allocated IRQs on unplug Sasha Levin
2025-08-08 15:59 ` Timothy Pearson
2025-08-08 17:04 ` Sasha Levin
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=20250808153054.1250675-1-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=apparmor@lists.ubuntu.com \
--cc=gabriel.totev@zetier.com \
--cc=john.johansen@canonical.com \
--cc=patches@lists.linux.dev \
--cc=stable@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.