FAILED: patch "[PATCH] apparmor: check accept index + 1 is in bounds" failed to apply to 6.12-stable tree

FAILED: patch "[PATCH] apparmor: check accept index + 1 is in bounds" failed to apply to 6.12-stable tree

[Sasha Levin]

The patch below does not apply to the 6.12-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.12.y
git checkout FETCH_HEAD
git cherry-pick -x afe5d5a747393cb3d00795db31d7fce5108a766a
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '20261107052758-failed-6.12-afe5d5a74739@kernel.org' --subject-prefix 'PATCH 6.12.y' 'HEAD^..'

Thanks,
Sasha

------------------ original commit in Linus's tree ------------------

From afe5d5a747393cb3d00795db31d7fce5108a766a Mon Sep 17 00:00:00 2001
From: Guanglei Zhu <zhugl3@xiaopeng.com>
Date: Wed, 16 Sep 2026 19:53:07 +0800
Subject: [PATCH] apparmor: check accept index + 1 is in bounds

aa_lookup_condperms() returns perms[index + 1] when an accept entry has
ACCEPT_FLAG_OWNER set and the subject uid does not match the file uid,
so such an entry consumes two slots in the perms table.  But
verify_dfa_accept_index() only checks that index is within the perms
table, so a policy that sets the owner flag on the last entry is
accepted, and the runtime lookup then reads perms[index + 1] one entry
past the end of the table.

The perms table is allocated by unpack_perms_table() with a u16 size
read from the policy blob, so a crafted policy can place the owner flag
at index size - 1 to reach the out-of-bounds read.

Check that index + 1 is also in bounds when the owner flag is set.

Fixes: 6f442d42c0d89 ("apparmor: fix profile verification and enable it")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/policy_unpack.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 3ffbab46eec30..7ebffb4ff74fa 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -1495,6 +1495,15 @@ static bool verify_dfa_accept_index(const struct aa_dfa *dfa, int table_size)
 	for (i = 0; i < dfa->tables[YYTD_ID_ACCEPT]->td_lolen; i++) {
 		if (ACCEPT_TABLE(dfa)[i] >= table_size)
 			return false;
+		/*
+		 * Accept indexes for owner-conditional permissions come in
+		 * pairs, so the non-owner entry at index + 1 must also be
+		 * in bounds.
+		 */
+		if (dfa->tables[YYTD_ID_ACCEPT2] &&
+		    (ACCEPT_TABLE2(dfa)[i] & ACCEPT_FLAG_OWNER) &&
+		    ACCEPT_TABLE(dfa)[i] + 1 >= table_size)
+			return false;
 	}
 	return true;
 }
-- 
2.53.0

[PATCH 6.12.y] apparmor: check accept index + 1 is in bounds

[Guanglei Zhu]

[ Upstream commit afe5d5a747393cb3d00795db31d7fce5108a766a ]

aa_lookup_condperms() returns perms[index + 1] when an accept entry has
ACCEPT_FLAG_OWNER set and the subject uid does not match the file uid,
so such an entry consumes two slots in the perms table.  But
verify_dfa_accept_index() only checks that index is within the perms
table, so a policy that sets the owner flag on the last entry is
accepted, and the runtime lookup then reads perms[index + 1] one entry
past the end of the table.

The perms table is allocated by unpack_perms_table() with a u16 size
read from the policy blob, so a crafted policy can place the owner flag
at index size - 1 to reach the out-of-bounds read.

Check that index + 1 is also in bounds when the owner flag is set.

Fixes: 6f442d42c0d89 ("apparmor: fix profile verification and enable it")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
[ Adapted for 6.12.y: verify_dfa_accept_index() takes a non-const dfa here. ]
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
 security/apparmor/policy_unpack.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 4642058..ffff8d8 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -1209,6 +1209,15 @@ static bool verify_dfa_accept_index(struct aa_dfa *dfa, int table_size)
 	for (i = 0; i < dfa->tables[YYTD_ID_ACCEPT]->td_lolen; i++) {
 		if (ACCEPT_TABLE(dfa)[i] >= table_size)
 			return false;
+			/*
+			 * Accept indexes for owner-conditional permissions come in
+			 * pairs, so the non-owner entry at index + 1 must also be
+			 * in bounds.
+			 */
+			if (dfa->tables[YYTD_ID_ACCEPT2] &&
+			    (ACCEPT_TABLE2(dfa)[i] & ACCEPT_FLAG_OWNER) &&
+			    ACCEPT_TABLE(dfa)[i] + 1 >= table_size)
+				return false;
 	}
 	return true;
 }
-- 
2.43.0

Re: [PATCH 6.12.y] apparmor: check accept index + 1 is in bounds

[Sasha Levin]

> apparmor: check accept index + 1 is in bounds

Both backports use ACCEPT_FLAG_OWNER, which does not exist in 6.12 or 6.6 (it
came with 2e12c5f06017 ("apparmor: add additional flags to extended
permission.")), so they break the build.

In those trees aa_lookup_fperms() reads perms[index + 1] whenever fsuid !=
cond->uid, without the owner flag, so the bound check needs to be written
against that code. Please rework and resend.

-- 
Thanks,
Sasha

[PATCH 6.12.y v2] apparmor: check accept index + 1 is in bounds

[Guanglei Zhu]

[ Upstream commit afe5d5a747393cb3d00795db31d7fce5108a766a ]

aa_lookup_fperms() returns perms[index + 1] when the current fsuid
does not match cond->uid, so such an entry consumes two slots in the
perms table.  But verify_dfa_accept_index() only checks that index is
within the perms table, so a policy whose last accept entry is reached
with a non-matching fsuid reads perms[index + 1] one entry past the
end of the table.

The perms table is allocated by unpack_perms_table() with a u16 size
read from the policy blob, so a crafted policy can place an accept
entry at index size - 1 to reach the out-of-bounds read.

Check that index + 1 is also in bounds.

Fixes: 6f442d42c0d89 ("apparmor: fix profile verification and enable it")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
[ Adapted for 6.12.y: this tree predates ACCEPT_FLAG_OWNER (2e12c5f06017), and aa_lookup_fperms() here reads perms[index + 1] whenever fsuid does not match cond->uid, with no flag to narrow the check to, so every accept index gets the pair bound. ]
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
v2: drop the ACCEPT_FLAG_OWNER reference (absent here -
    2e12c5f06017 is not in this tree, so it broke the build)
    and check index + 1 unconditionally: aa_lookup_fperms()
    in this tree reads perms[index + 1] whenever fsuid does
    not match cond->uid, with no flag to narrow the check
    to, per review.

 security/apparmor/policy_unpack.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 4642058..684a21d 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -1209,6 +1209,14 @@ static bool verify_dfa_accept_index(struct aa_dfa *dfa, int table_size)
 	for (i = 0; i < dfa->tables[YYTD_ID_ACCEPT]->td_lolen; i++) {
 		if (ACCEPT_TABLE(dfa)[i] >= table_size)
 			return false;
+			/*
+			 * Conditional permission entries consume two slots:
+			 * aa_lookup_fperms() reads perms[index] when fsuid matches
+			 * cond->uid and perms[index + 1] otherwise, so index + 1
+			 * must also be in bounds.
+			 */
+			if (ACCEPT_TABLE(dfa)[i] + 1 >= table_size)
+				return false;
 	}
 	return true;
 }
-- 
2.43.0

Re: [PATCH 6.12.y v2] apparmor: check accept index + 1 is in bounds

[Sasha Levin]

> apparmor: check accept index + 1 is in bounds

In v2 the index + 1 bound check sits in verify_dfa_accept_index(), which runs
for both rules->file->dfa and rules->policy->dfa. The file dfa has two perms
slots per state, so the check is right there, but the policy dfa has one slot
per state (accept[state] == state, table size == state_count), so its last
state always fails the new check and profiles with a policy dfa are rejected on
load.

Please limit the bound check to the file dfa and resend both trees (6.12 and
6.6).

-- 
Thanks,
Sasha

[PATCH 6.12.y v3] apparmor: check accept index + 1 is in bounds

[Guanglei Zhu]

[ Upstream commit afe5d5a747393cb3d00795db31d7fce5108a766a ]

aa_lookup_fperms() returns perms[index + 1] when the current fsuid
does not match cond->uid, so such an entry consumes two slots in the
perms table.  But verify_dfa_accept_index() only checks that index is
within the perms table, so a policy whose last accept entry is reached
with a non-matching fsuid reads perms[index + 1] one entry past the
end of the table.

The perms table is allocated by unpack_perms_table() with a u16 size
read from the policy blob, so a crafted policy can place an accept
entry at index size - 1 to reach the out-of-bounds read.

Check that index + 1 is also in bounds.

Fixes: 6f442d42c0d89 ("apparmor: fix profile verification and enable it")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
[ Adapted for 6.12.y: this tree predates ACCEPT_FLAG_OWNER (2e12c5f06017), so aa_lookup_fperms() reads perms[index + 1] whenever fsuid does not match cond->uid and every accept index of the file dfa gets the pair bound.  The policy dfa has one perms slot per state (accept[state] == state, table size == state_count) and aa_lookup_perms() never reads index + 1, so verify_dfa_accept_index() takes a pair flag that is set only for rules->file->dfa. ]
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
v3: limit the index + 1 bound check to the file dfa.  v2 applied
    the check to every accept index of both the file and the policy
    dfa, but the policy dfa has one perms slot per state
    (accept[state] == state, table size == state_count), so its
    last state always failed the new check and profiles with a
    policy dfa were rejected on load, per review.

 security/apparmor/policy_unpack.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 4642058..53e95d9 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -1202,13 +1202,22 @@ static int verify_header(struct aa_ext *e, int required, const char **ns)
  * verify_dfa_accept_index - verify accept indexes are in range of perms table
  * @dfa: the dfa to check accept indexes are in range
  * @table_size: the permission table size the indexes should be within
+ * @pair: the accept indexes reference two slots, index and index + 1
  */
-static bool verify_dfa_accept_index(struct aa_dfa *dfa, int table_size)
+static bool verify_dfa_accept_index(struct aa_dfa *dfa, int table_size,
+				    bool pair)
 {
 	int i;
 	for (i = 0; i < dfa->tables[YYTD_ID_ACCEPT]->td_lolen; i++) {
 		if (ACCEPT_TABLE(dfa)[i] >= table_size)
 			return false;
+		/*
+		 * Accept indexes for owner-conditional permissions come
+		 * in pairs, so the non-owner entry at index + 1 must also
+		 * be in bounds.
+		 */
+		if (pair && ACCEPT_TABLE(dfa)[i] + 1 >= table_size)
+			return false;
 	}
 	return true;
 }
@@ -1273,14 +1282,16 @@ static int verify_profile(struct aa_profile *profile)
 		return 0;
 
 	if (rules->file->dfa && !verify_dfa_accept_index(rules->file->dfa,
-							rules->file->size)) {
+							rules->file->size,
+							true)) {
 		audit_iface(profile, NULL, NULL,
 			    "Unpack: file Invalid named transition", NULL,
 			    -EPROTO);
 		return -EPROTO;
 	}
 	if (rules->policy->dfa &&
-	    !verify_dfa_accept_index(rules->policy->dfa, rules->policy->size)) {
+	    !verify_dfa_accept_index(rules->policy->dfa, rules->policy->size,
+				     false)) {
 		audit_iface(profile, NULL, NULL,
 			    "Unpack: policy Invalid named transition", NULL,
 			    -EPROTO);
-- 
2.43.0

Re: [PATCH 6.12.y v3] apparmor: check accept index + 1 is in bounds

[Sasha Levin]

> [PATCH 6.12.y v3] apparmor: check accept index + 1 is in bounds
This regresses profiles that have no separate file dfa. In that case
unpack_profile() aliases rules->file to rules->policy, and the policy pdb is
compat-mapped with size == state_count and accept[state] == state. The new
'index + 1 < size' check then fails for its last state and the profile is
rejected at load with -EPROTO. Such profiles load today. Upstream only bounds
entries with ACCEPT_FLAG_OWNER, so it does not hit this. Both 6.12 and 6.6 are
affected.

Could you send a v4 that passes 'rules->file != rules->policy' as the pair
argument for the file dfa call?

-- 
Thanks,
Sasha

[PATCH 6.12.y v4] apparmor: check accept index + 1 is in bounds

[Guanglei Zhu]

[ Upstream commit afe5d5a747393cb3d00795db31d7fce5108a766a ]

aa_lookup_fperms() returns perms[index + 1] when the current fsuid
does not match cond->uid, so such an entry consumes two slots in the
perms table.  But verify_dfa_accept_index() only checks that index is
within the perms table, so a policy whose last accept entry is reached
with a non-matching fsuid reads perms[index + 1] one entry past the
end of the table.

The perms table is allocated by unpack_perms_table() with a u16 size
read from the policy blob, so a crafted policy can place an accept
entry at index size - 1 to reach the out-of-bounds read.

Check that index + 1 is also in bounds.

Fixes: 6f442d42c0d89 ("apparmor: fix profile verification and enable it")
Cc: stable@vger.kernel.org
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
[ Adapted for 6.12.y: this tree predates ACCEPT_FLAG_OWNER (2e12c5f06017), so aa_lookup_fperms() reads perms[index + 1] whenever fsuid does not match cond->uid and every accept index of a standalone file pdb gets the pair bound.  A compat-mapped policy dfa has one perms slot per state (accept[state] == state, table size == state_count) and aa_lookup_perms() never reads index + 1, and unpack_profile() aliases rules->file to a compat-mapped rules->policy when the profile has no separate file dfa, so verify_dfa_accept_index() takes a pair flag that is set only when rules->file != rules->policy.  The pair check is thus skipped whenever rules->file aliases rules->policy, including when the policy pdb supplies an explicit permission table; unlike upstream, this tree still uses aa_lookup_fperms(), which can access index + 1 on this path, so this backport does not address the aliased-policy case. ]
Signed-off-by: Guanglei Zhu <zhugl3@xiaopeng.com>
---
v4: pass 'rules->file != rules->policy' instead of 'true' as the pair
    argument for the file dfa call.  v3 rejected profiles that have no
    separate file dfa: unpack_profile() aliases rules->file to
    rules->policy for such profiles, and a compat-mapped policy pdb has
    one perms slot per state (size == state_count, accept[state] == state),
    so the index + 1 check failed for its last state and the profile was
    rejected at load with -EPROTO, per review.

    The pair check is now skipped whenever rules->file aliases
    rules->policy, including when the policy pdb supplies an explicit
    permission table, so this backport does not address the
    aliased-policy case: unlike upstream, this tree still uses
    aa_lookup_fperms(), which can access index + 1 on that path.  Please
    let me know whether this partial fix is acceptable as is, or whether
    the runtime side should be closed first (e.g. evaluating 2e12c5f06017
    and its dependencies for these trees).

 security/apparmor/policy_unpack.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 4642058..61d7ef2 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -1202,13 +1202,22 @@ static int verify_header(struct aa_ext *e, int required, const char **ns)
  * verify_dfa_accept_index - verify accept indexes are in range of perms table
  * @dfa: the dfa to check accept indexes are in range
  * @table_size: the permission table size the indexes should be within
+ * @pair: the accept indexes reference two slots, index and index + 1
  */
-static bool verify_dfa_accept_index(struct aa_dfa *dfa, int table_size)
+static bool verify_dfa_accept_index(struct aa_dfa *dfa, int table_size,
+				    bool pair)
 {
 	int i;
 	for (i = 0; i < dfa->tables[YYTD_ID_ACCEPT]->td_lolen; i++) {
 		if (ACCEPT_TABLE(dfa)[i] >= table_size)
 			return false;
+		/*
+		 * Accept indexes for owner-conditional permissions come
+		 * in pairs, so the non-owner entry at index + 1 must also
+		 * be in bounds.
+		 */
+		if (pair && ACCEPT_TABLE(dfa)[i] + 1 >= table_size)
+			return false;
 	}
 	return true;
 }
@@ -1272,15 +1281,17 @@ static int verify_profile(struct aa_profile *profile)
 	if (!rules)
 		return 0;
 
-	if (rules->file->dfa && !verify_dfa_accept_index(rules->file->dfa,
-							rules->file->size)) {
+	if (rules->file->dfa &&
+	    !verify_dfa_accept_index(rules->file->dfa, rules->file->size,
+				     rules->file != rules->policy)) {
 		audit_iface(profile, NULL, NULL,
 			    "Unpack: file Invalid named transition", NULL,
 			    -EPROTO);
 		return -EPROTO;
 	}
 	if (rules->policy->dfa &&
-	    !verify_dfa_accept_index(rules->policy->dfa, rules->policy->size)) {
+	    !verify_dfa_accept_index(rules->policy->dfa, rules->policy->size,
+				     false)) {
 		audit_iface(profile, NULL, NULL,
 			    "Unpack: policy Invalid named transition", NULL,
 			    -EPROTO);
-- 
2.43.0

Re: [PATCH 6.12.y v4] apparmor: check accept index + 1 is in bounds

[Sasha Levin]

On 2026-11-12, Guanglei Zhu wrote:
> [PATCH 6.12.y v4] apparmor: check accept index + 1 is in bounds
> aa_lookup_fperms() returns perms[index + 1] when the current fsuid
> does not match cond->uid, so such an entry consumes two slots in the
> perms table.  But verify_dfa_accept_index() only checks that index is

Queued for 6.12 and 6.6, thanks.


-- 
Thanks,
Sasha