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: 18+ 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 ` [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 ` [PATCH AUTOSEL 6.16-6.1] i3c: master: Initialize ret in i3c_i2c_notifier_call() 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox