Skip to contentSkip to conversation

6.19.8-rc1 review

PATCH In review Greg Kroah-Hartman 10 participants 23 messages Mar 12 – Mar 13

Subsystems (2)
Stable branch Tegra architecture support
lore
Share & tools
lore
author maintainer
#1 lore raw

PATCH 1/13 net/sched: act_gate: snapshot parameters with RCU on replace

In reply to Greg Kroah-Hartman

6.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: Paul Moses <[email protected]>

commit 62413a9c3cb183afb9bb6e94dd68caf4e4145f4c upstream.

The gate action can be replaced while the hrtimer callback or dump path is walking the schedule list.

Convert the parameters to an RCU-protected snapshot and swap updates under tcf_lock, freeing the previous snapshot via call_rcu(). When REPLACE omits the entry list, preserve the existing schedule so the effective state is unchanged.

Fixes a51c328df310 ("net: qos: introduce a gate control flow action")
Cc [email protected]
Signed-off-by Paul Moses <[email protected]>
Tested-by Vladimir Oltean <[email protected]>
Acked-by Jamal Hadi Salim <[email protected]>
Reviewed-by Victor Nogueira <[email protected]>
Link https://patch.msgid.link/[email protected] open here
Signed-off-by Jakub Kicinski <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 include/net/tc_act/tc_gate.h |   33 ++++-
 net/sched/act_gate.c         |  269 ++++++++++++++++++++++++++++++-------------
 2 files changed, 214 insertions(+), 88 deletions(-)
include/net/tc_act/tc_gate.h +26 -7
@@ -32,6 +32,7 @@ struct tcf_gate_params {
 	s32			tcfg_clockid;
 	size_t			num_entries;
 	struct list_head	entries;
+	struct rcu_head		rcu;
 };
 
 #define GATE_ACT_GATE_OPEN	BIT(0)

@@ -39,7 +40,7 @@ struct tcf_gate_params {
 
 struct tcf_gate {
 	struct tc_action	common;
-	struct tcf_gate_params	param;
+	struct tcf_gate_params __rcu *param;
 	u8			current_gate_status;
 	ktime_t			current_close_time;
 	u32			current_entry_octets;

@@ -51,47 +52,65 @@ struct tcf_gate {
 
 #define to_gate(a) ((struct tcf_gate *)a)
 
+static inline struct tcf_gate_params *tcf_gate_params_locked(const struct tc_action *a)
+{
+	struct tcf_gate *gact = to_gate(a);
+
+	return rcu_dereference_protected(gact->param,
+					 lockdep_is_held(&gact->tcf_lock));
+}
+
 static inline s32 tcf_gate_prio(const struct tc_action *a)
 {
+	struct tcf_gate_params *p;
 	s32 tcfg_prio;
 
-	tcfg_prio = to_gate(a)->param.tcfg_priority;
+	p = tcf_gate_params_locked(a);
+	tcfg_prio = p->tcfg_priority;
 
 	return tcfg_prio;
 }
 
 static inline u64 tcf_gate_basetime(const struct tc_action *a)
 {
+	struct tcf_gate_params *p;
 	u64 tcfg_basetime;
 
-	tcfg_basetime = to_gate(a)->param.tcfg_basetime;
+	p = tcf_gate_params_locked(a);
+	tcfg_basetime = p->tcfg_basetime;
 
 	return tcfg_basetime;
 }
 
 static inline u64 tcf_gate_cycletime(const struct tc_action *a)
 {
+	struct tcf_gate_params *p;
 	u64 tcfg_cycletime;
 
-	tcfg_cycletime = to_gate(a)->param.tcfg_cycletime;
+	p = tcf_gate_params_locked(a);
+	tcfg_cycletime = p->tcfg_cycletime;
 
 	return tcfg_cycletime;
 }
 
 static inline u64 tcf_gate_cycletimeext(const struct tc_action *a)
 {
+	struct tcf_gate_params *p;
 	u64 tcfg_cycletimeext;
 
-	tcfg_cycletimeext = to_gate(a)->param.tcfg_cycletime_ext;
+	p = tcf_gate_params_locked(a);
+	tcfg_cycletimeext = p->tcfg_cycletime_ext;
 
 	return tcfg_cycletimeext;
 }
 
 static inline u32 tcf_gate_num_entries(const struct tc_action *a)
 {
+	struct tcf_gate_params *p;
 	u32 num_entries;
 
-	num_entries = to_gate(a)->param.num_entries;
+	p = tcf_gate_params_locked(a);
+	num_entries = p->num_entries;
 
 	return num_entries;
 }

@@ -105,7 +124,7 @@ static inline struct action_gate_entry
 	u32 num_entries;
 	int i = 0;
 
-	p = &to_gate(a)->param;
+	p = tcf_gate_params_locked(a);
 	num_entries = p->num_entries;
 
 	list_for_each_entry(entry, &p->entries, list)
net/sched/act_gate.c +188 -81

387 lines not on this page · raw message

Greg Kroah-Hartman

author maintainer
#0 lore raw

cover 6.19.8-rc1 review

This is the start of the stable review cycle for the 6.19.8 release. There are 13 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Fri, 13 Mar 2026 20:03:10 +0000. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

	https://www.kernel.org/pub/linux/kernel/v6.x/stable-review/patch-6.19.8-rc1.gz

or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.19.y
and the diffstat can be found below.

thanks,

greg k-h

-------------
Pseudo-Shortlog of commits:

Greg Kroah-Hartman <[email protected]>

    Linux 6.19.8-rc1

John Johansen <[email protected]>
apparmor: fix race between freeing data and fs accessing it

John Johansen <[email protected]>
apparmor: fix race on rawdata dereference

John Johansen <[email protected]>
apparmor: fix differential encoding verification

John Johansen <[email protected]>
apparmor: fix unprivileged local user can do privileged policy management

John Johansen <[email protected]>
apparmor: Fix double free of ns_name in aa_replace_profiles()

Massimiliano Pellizzer <[email protected]>
apparmor: fix missing bounds check on DEFAULT table in verify_dfa()

Massimiliano Pellizzer <[email protected]>
apparmor: fix side-effect bug in match_char() macro usage

John Johansen <[email protected]>
apparmor: fix: limit the number of levels of policy namespaces

Massimiliano Pellizzer <[email protected]>
apparmor: replace recursive profile removal with iterative approach

Massimiliano Pellizzer <[email protected]>
apparmor: fix memory leak in verify_header

Massimiliano Pellizzer <[email protected]>
apparmor: validate DFA start states are in bounds in unpack_pdb

Victor Nogueira <[email protected]>
net/sched: Only allow act_ct to bind to clsact/ingress qdiscs and shared blocks

Paul Moses <[email protected]>
net/sched: act_gate: snapshot parameters with RCU on replace

-------------

Diffstat:

 Makefile                                  |   4 +-
 include/net/act_api.h                     |   1 +
 include/net/tc_act/tc_gate.h              |  33 +++-
 net/sched/act_ct.c                        |   6 +
 net/sched/act_gate.c                      | 267 +++++++++++++++++++++---------
 net/sched/cls_api.c                       |   7 +
 security/apparmor/apparmorfs.c            | 225 +++++++++++++++----------
 security/apparmor/include/label.h         |  16 +-
 security/apparmor/include/lib.h           |  12 ++
 security/apparmor/include/match.h         |   1 +
 security/apparmor/include/policy.h        |  10 +-
 security/apparmor/include/policy_ns.h     |   2 +
 security/apparmor/include/policy_unpack.h |  83 ++++++----
 security/apparmor/label.c                 |  12 +-
 security/apparmor/match.c                 |  58 +++++--
 security/apparmor/policy.c                |  77 +++++++--
 security/apparmor/policy_ns.c             |   2 +
 security/apparmor/policy_unpack.c         |  65 +++++---
 18 files changed, 604 insertions(+), 277 deletions(-)
author maintainer
#10 lore raw

PATCH 10/13 apparmor: fix unprivileged local user can do privileged policy management

6.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: John Johansen <[email protected]>

commit 6601e13e82841879406bf9f369032656f441a425 upstream.

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 <[email protected]>
Tested-by Salvatore Bonaccorso <[email protected]>
Reviewed-by Georgia Garcia <[email protected]>
Reviewed-by Cengiz Can <[email protected]>
Signed-off-by John Johansen <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 security/apparmor/apparmorfs.c     |   16 +++++++++-------
 security/apparmor/include/policy.h |    2 +-
 security/apparmor/policy.c         |   34 +++++++++++++++++++++++++++++++++-
 3 files changed, 43 insertions(+), 9 deletions(-)
security/apparmor/apparmorfs.c +9 -7
@@ -417,7 +417,8 @@ static struct aa_loaddata *aa_simple_wri
 }
 
 static ssize_t policy_update(u32 mask, const char __user *buf, size_t size,
-			     loff_t *pos, struct aa_ns *ns)
+			     loff_t *pos, struct aa_ns *ns,
+			     const struct cred *ocred)
 {
 	struct aa_loaddata *data;
 	struct aa_label *label;

@@ -428,7 +429,7 @@ static ssize_t policy_update(u32 mask, c
 	/* high level check about policy management - fine grained in
 	 * below after unpack
 	 */
-	error = aa_may_manage_policy(current_cred(), label, ns, mask);
+	error = aa_may_manage_policy(current_cred(), label, ns, ocred, mask);
 	if (error)
 		goto end_section;
 

@@ -449,7 +450,8 @@ static ssize_t profile_load(struct file
 			    loff_t *pos)
 {
 	struct aa_ns *ns = aa_get_ns(f->f_inode->i_private);
-	int error = policy_update(AA_MAY_LOAD_POLICY, buf, size, pos, ns);
+	int error = policy_update(AA_MAY_LOAD_POLICY, buf, size, pos, ns,
+				  f->f_cred);
 
 	aa_put_ns(ns);
 

@@ -467,7 +469,7 @@ static ssize_t profile_replace(struct fi
 {
 	struct aa_ns *ns = aa_get_ns(f->f_inode->i_private);
 	int error = policy_update(AA_MAY_LOAD_POLICY | AA_MAY_REPLACE_POLICY,
-				  buf, size, pos, ns);
+				  buf, size, pos, ns, f->f_cred);
 	aa_put_ns(ns);
 
 	return error;

@@ -492,7 +494,7 @@ static ssize_t profile_remove(struct fil
 	 * below after unpack
 	 */
 	error = aa_may_manage_policy(current_cred(), label, ns,
-				     AA_MAY_REMOVE_POLICY);
+				     f->f_cred, AA_MAY_REMOVE_POLICY);
 	if (error)
 		goto out;
 

@@ -1826,7 +1828,7 @@ static struct dentry *ns_mkdir_op(struct
 	int error;
 
 	label = begin_current_label_crit_section();
-	error = aa_may_manage_policy(current_cred(), label, NULL,
+	error = aa_may_manage_policy(current_cred(), label, NULL, NULL,
 				     AA_MAY_LOAD_POLICY);
 	end_current_label_crit_section(label);
 	if (error)

@@ -1876,7 +1878,7 @@ static int ns_rmdir_op(struct inode *dir
 	int error;
 
 	label = begin_current_label_crit_section();
-	error = aa_may_manage_policy(current_cred(), label, NULL,
+	error = aa_may_manage_policy(current_cred(), label, NULL, NULL,
 				     AA_MAY_LOAD_POLICY);
 	end_current_label_crit_section(label);
 	if (error)
security/apparmor/include/policy.h +1 -1
@@ -419,7 +419,7 @@ bool aa_policy_admin_capable(const struc
 			     struct aa_label *label, struct aa_ns *ns);
 int aa_may_manage_policy(const struct cred *subj_cred,
 			 struct aa_label *label, struct aa_ns *ns,
-			 u32 mask);
+			 const struct cred *ocred, u32 mask);
 bool aa_current_policy_view_capable(struct aa_ns *ns);
 bool aa_current_policy_admin_capable(struct aa_ns *ns);
security/apparmor/policy.c +33 -1
@@ -925,17 +925,44 @@ bool aa_current_policy_admin_capable(str
 	return res;
 }
 
+static bool is_subset_of_obj_privilege(const struct cred *cred,
+				       struct aa_label *label,
+				       const struct cred *ocred)
+{
+	if (cred == ocred)
+		return true;
+
+	if (!aa_label_is_subset(label, cred_label(ocred)))
+		return false;
+	/* don't allow crossing userns for now */
+	if (cred->user_ns != ocred->user_ns)
+		return false;
+	if (!cap_issubset(cred->cap_inheritable, ocred->cap_inheritable))
+		return false;
+	if (!cap_issubset(cred->cap_permitted, ocred->cap_permitted))
+		return false;
+	if (!cap_issubset(cred->cap_effective, ocred->cap_effective))
+		return false;
+	if (!cap_issubset(cred->cap_bset, ocred->cap_bset))
+		return false;
+	if (!cap_issubset(cred->cap_ambient, ocred->cap_ambient))
+		return false;
+	return true;
+}
+
+
 /**
  * aa_may_manage_policy - can the current task manage policy
  * @subj_cred: subjects cred
  * @label: label to check if it can manage policy
  * @ns: namespace being managed by @label (may be NULL if @label's ns)
+ * @ocred: object cred if request is coming from an open object
  * @mask: contains the policy manipulation operation being done
  *
  * Returns: 0 if the task is allowed to manipulate policy else error
  */
 int aa_may_manage_policy(const struct cred *subj_cred, struct aa_label *label,
-			 struct aa_ns *ns, u32 mask)
+			 struct aa_ns *ns, const struct cred *ocred, u32 mask)
 {
 	const char *op;
 

@@ -951,6 +978,11 @@ int aa_may_manage_policy(const struct cr
 		return audit_policy(label, op, NULL, NULL, "policy_locked",
 				    -EACCES);
 
+	if (ocred && !is_subset_of_obj_privilege(subj_cred, label, ocred))
+		return audit_policy(label, op, NULL, NULL,
+				    "not privileged for target profile",
+				    -EACCES);
+
 	if (!aa_policy_admin_capable(subj_cred, label, ns))
 		return audit_policy(label, op, NULL, NULL, "not policy admin",
 				    -EACCES);
author maintainer
#11 lore raw

PATCH 11/13 apparmor: fix differential encoding verification

In reply to Greg Kroah-Hartman

6.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: John Johansen <[email protected]>

commit 39440b137546a3aa383cfdabc605fb73811b6093 upstream.

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 <[email protected]>
Tested-by Salvatore Bonaccorso <[email protected]>
Reviewed-by Georgia Garcia <[email protected]>
Reviewed-by Cengiz Can <[email protected]>
Signed-off-by John Johansen <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 security/apparmor/include/match.h |    1 +
 security/apparmor/match.c         |   23 +++++++++++++++++++----
 2 files changed, 20 insertions(+), 4 deletions(-)
security/apparmor/include/match.h +1 -0
@@ -185,6 +185,7 @@ static inline void aa_put_dfa(struct aa_
 #define MATCH_FLAG_DIFF_ENCODE 0x80000000
 #define MARK_DIFF_ENCODE 0x40000000
 #define MATCH_FLAG_OOB_TRANSITION 0x20000000
+#define MARK_DIFF_ENCODE_VERIFIED 0x10000000
 #define MATCH_FLAGS_MASK 0xff000000
 #define MATCH_FLAGS_VALID (MATCH_FLAG_DIFF_ENCODE | MATCH_FLAG_OOB_TRANSITION)
 #define MATCH_FLAGS_INVALID (MATCH_FLAGS_MASK & ~MATCH_FLAGS_VALID)
security/apparmor/match.c +19 -4
@@ -202,16 +202,31 @@ static int verify_dfa(struct aa_dfa *dfa
 		size_t j, k;
 
 		for (j = i;
-		     (BASE_TABLE(dfa)[j] & MATCH_FLAG_DIFF_ENCODE) &&
-		     !(BASE_TABLE(dfa)[j] & MARK_DIFF_ENCODE);
+		     ((BASE_TABLE(dfa)[j] & MATCH_FLAG_DIFF_ENCODE) &&
+		      !(BASE_TABLE(dfa)[j] & MARK_DIFF_ENCODE_VERIFIED));
 		     j = k) {
+			if (BASE_TABLE(dfa)[j] & MARK_DIFF_ENCODE)
+				/* loop in current chain */
+				goto out;
 			k = DEFAULT_TABLE(dfa)[j];
 			if (j == k)
+				/* self loop */
 				goto out;
-			if (k < j)
-				break;		/* already verified */
 			BASE_TABLE(dfa)[j] |= MARK_DIFF_ENCODE;
 		}
+		/* move mark to verified */
+		for (j = i;
+		     (BASE_TABLE(dfa)[j] & MATCH_FLAG_DIFF_ENCODE);
+		     j = k) {
+			k = DEFAULT_TABLE(dfa)[j];
+			if (j < i)
+				/* jumps to state/chain that has been
+				 * verified
+				 */
+				break;
+			BASE_TABLE(dfa)[j] &= ~MARK_DIFF_ENCODE;
+			BASE_TABLE(dfa)[j] |= MARK_DIFF_ENCODE_VERIFIED;
+		}
 	}
 	error = 0;
author maintainer
#12 lore raw

PATCH 12/13 apparmor: fix race on rawdata dereference

In reply to Greg Kroah-Hartman

6.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: John Johansen <[email protected]>

commit a0b7091c4de45a7325c8780e6934a894f92ac86b upstream.

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 <[email protected]>
Reviewed-by Georgia Garcia <[email protected]>
Reviewed-by Maxime Bélair <[email protected]>
Reviewed-by Cengiz Can <[email protected]>
Tested-by Salvatore Bonaccorso <[email protected]>
Signed-off-by John Johansen <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 security/apparmor/apparmorfs.c            |   35 ++++++++------
 security/apparmor/include/policy_unpack.h |   71 ++++++++++++++++++------------
 security/apparmor/policy.c                |   12 ++---
 security/apparmor/policy_unpack.c         |   32 +++++++++----
 4 files changed, 93 insertions(+), 57 deletions(-)
security/apparmor/apparmorfs.c +21 -14
@@ -79,7 +79,7 @@ static void rawdata_f_data_free(struct r
 	if (!private)
 		return;
 
-	aa_put_loaddata(private->loaddata);
+	aa_put_i_loaddata(private->loaddata);
 	kvfree(private);
 }
 

@@ -409,7 +409,8 @@ static struct aa_loaddata *aa_simple_wri
 
 	data->size = copy_size;
 	if (copy_from_user(data->data, userbuf, copy_size)) {
-		aa_put_loaddata(data);
+		/* trigger free - don't need to put pcount */
+		aa_put_i_loaddata(data);
 		return ERR_PTR(-EFAULT);
 	}
 

@@ -437,7 +438,10 @@ static ssize_t policy_update(u32 mask, c
 	error = PTR_ERR(data);
 	if (!IS_ERR(data)) {
 		error = aa_replace_profiles(ns, label, mask, data);
-		aa_put_loaddata(data);
+		/* put pcount, which will put count and free if no
+		 * profiles referencing it.
+		 */
+		aa_put_profile_loaddata(data);
 	}
 end_section:
 	end_current_label_crit_section(label);

@@ -508,7 +512,7 @@ static ssize_t profile_remove(struct fil
 	if (!IS_ERR(data)) {
 		data->data[size] = 0;
 		error = aa_remove_profiles(ns, label, data->data, size);
-		aa_put_loaddata(data);
+		aa_put_profile_loaddata(data);
 	}
  out:
 	end_current_label_crit_section(label);

@@ -1255,18 +1259,17 @@ static const struct file_operations seq_
 static int seq_rawdata_open(struct inode *inode, struct file *file,
 			    int (*show)(struct seq_file *, void *))
 {
-	struct aa_loaddata *data = __aa_get_loaddata(inode->i_private);
+	struct aa_loaddata *data = aa_get_i_loaddata(inode->i_private);
 	int error;
 
 	if (!data)
-		/* lost race this ent is being reaped */
 		return -ENOENT;
 
 	error = single_open(file, show, data);
 	if (error) {
 		AA_BUG(file->private_data &&
 		       ((struct seq_file *)file->private_data)->private);
-		aa_put_loaddata(data);
+		aa_put_i_loaddata(data);
 	}
 
 	return error;

@@ -1277,7 +1280,7 @@ static int seq_rawdata_release(struct in
 	struct seq_file *seq = (struct seq_file *) file->private_data;
 
 	if (seq)
-		aa_put_loaddata(seq->private);
+		aa_put_i_loaddata(seq->private);
 
 	return single_release(inode, file);
 }

@@ -1389,9 +1392,8 @@ static int rawdata_open(struct inode *in
 	if (!aa_current_policy_view_capable(NULL))
 		return -EACCES;
 
-	loaddata = __aa_get_loaddata(inode->i_private);
+	loaddata = aa_get_i_loaddata(inode->i_private);
 	if (!loaddata)
-		/* lost race: this entry is being reaped */
 		return -ENOENT;
 
 	private = rawdata_f_data_alloc(loaddata->size);

@@ -1416,7 +1418,7 @@ fail_decompress:
 	return error;
 
 fail_private_alloc:
-	aa_put_loaddata(loaddata);
+	aa_put_i_loaddata(loaddata);
 	return error;
 }
 

@@ -1433,9 +1435,9 @@ static void remove_rawdata_dents(struct
 
 	for (i = 0; i < AAFS_LOADDATA_NDENTS; i++) {
 		if (!IS_ERR_OR_NULL(rawdata->dents[i])) {
-			/* no refcounts on i_private */
 			aafs_remove(rawdata->dents[i]);
 			rawdata->dents[i] = NULL;
+			aa_put_i_loaddata(rawdata);
 		}
 	}
 }

@@ -1474,18 +1476,21 @@ int __aa_fs_create_rawdata(struct aa_ns
 	if (IS_ERR(dir))
 		/* ->name freed when rawdata freed */
 		return PTR_ERR(dir);
+	aa_get_i_loaddata(rawdata);
 	rawdata->dents[AAFS_LOADDATA_DIR] = dir;
 
 	dent = aafs_create_file("abi", S_IFREG | 0444, dir, rawdata,
 				      &seq_rawdata_abi_fops);
 	if (IS_ERR(dent))
 		goto fail;
+	aa_get_i_loaddata(rawdata);
 	rawdata->dents[AAFS_LOADDATA_ABI] = dent;
 
 	dent = aafs_create_file("revision", S_IFREG | 0444, dir, rawdata,
 				      &seq_rawdata_revision_fops);
 	if (IS_ERR(dent))
 		goto fail;
+	aa_get_i_loaddata(rawdata);
 	rawdata->dents[AAFS_LOADDATA_REVISION] = dent;
 
 	if (aa_g_hash_policy) {

@@ -1493,6 +1498,7 @@ int __aa_fs_create_rawdata(struct aa_ns
 					      rawdata, &seq_rawdata_hash_fops);
 		if (IS_ERR(dent))
 			goto fail;
+		aa_get_i_loaddata(rawdata);
 		rawdata->dents[AAFS_LOADDATA_HASH] = dent;
 	}
 

@@ -1501,24 +1507,25 @@ int __aa_fs_create_rawdata(struct aa_ns
 				&seq_rawdata_compressed_size_fops);
 	if (IS_ERR(dent))
 		goto fail;
+	aa_get_i_loaddata(rawdata);
 	rawdata->dents[AAFS_LOADDATA_COMPRESSED_SIZE] = dent;
 
 	dent = aafs_create_file("raw_data", S_IFREG | 0444,
 				      dir, rawdata, &rawdata_fops);
 	if (IS_ERR(dent))
 		goto fail;
+	aa_get_i_loaddata(rawdata);
 	rawdata->dents[AAFS_LOADDATA_DATA] = dent;
 	d_inode(dent)->i_size = rawdata->size;
 
 	rawdata->ns = aa_get_ns(ns);
 	list_add(&rawdata->list, &ns->rawdata_list);
-	/* no refcount on inode rawdata */
 
 	return 0;
 
 fail:
 	remove_rawdata_dents(rawdata);
-
+	aa_put_i_loaddata(rawdata);
 	return PTR_ERR(dent);
 }
 #endif /* CONFIG_SECURITY_APPARMOR_EXPORT_BINARY */
security/apparmor/include/policy_unpack.h +43 -28

112 lines not on this page · raw message

security/apparmor/policy.c +6 -6
@@ -336,7 +336,7 @@ void aa_free_profile(struct aa_profile *
 	}
 
 	kfree_sensitive(profile->hash);
-	aa_put_loaddata(profile->rawdata);
+	aa_put_profile_loaddata(profile->rawdata);
 	aa_label_destroy(&profile->label);
 
 	kfree_sensitive(profile);

@@ -1154,7 +1154,7 @@ ssize_t aa_replace_profiles(struct aa_ns
 	LIST_HEAD(lh);
 
 	op = mask & AA_MAY_REPLACE_POLICY ? OP_PROF_REPL : OP_PROF_LOAD;
-	aa_get_loaddata(udata);
+	aa_get_profile_loaddata(udata);
 	/* released below */
 	error = aa_unpack(udata, &lh, &ns_name);
 	if (error)

@@ -1206,10 +1206,10 @@ ssize_t aa_replace_profiles(struct aa_ns
 			if (aa_rawdata_eq(rawdata_ent, udata)) {
 				struct aa_loaddata *tmp;
 
-				tmp = __aa_get_loaddata(rawdata_ent);
+				tmp = aa_get_profile_loaddata(rawdata_ent);
 				/* check we didn't fail the race */
 				if (tmp) {
-					aa_put_loaddata(udata);
+					aa_put_profile_loaddata(udata);
 					udata = tmp;
 					break;
 				}

@@ -1222,7 +1222,7 @@ ssize_t aa_replace_profiles(struct aa_ns
 		struct aa_profile *p;
 
 		if (aa_g_export_binary)
-			ent->new->rawdata = aa_get_loaddata(udata);
+			ent->new->rawdata = aa_get_profile_loaddata(udata);
 		error = __lookup_replace(ns, ent->new->base.hname,
 					 !(mask & AA_MAY_REPLACE_POLICY),
 					 &ent->old, &info);

@@ -1355,7 +1355,7 @@ ssize_t aa_replace_profiles(struct aa_ns
 
 out:
 	aa_put_ns(ns);
-	aa_put_loaddata(udata);
+	aa_put_profile_loaddata(udata);
 	kfree(ns_name);
 
 	if (error)
security/apparmor/policy_unpack.c +23 -9
@@ -109,34 +109,47 @@ bool aa_rawdata_eq(struct aa_loaddata *l
 	return memcmp(l->data, r->data, r->compressed_size ?: r->size) == 0;
 }
 
+static void do_loaddata_free(struct aa_loaddata *d)
+{
+	kfree_sensitive(d->hash);
+	kfree_sensitive(d->name);
+	kvfree(d->data);
+	kfree_sensitive(d);
+}
+
+void aa_loaddata_kref(struct kref *kref)
+{
+	struct aa_loaddata *d = container_of(kref, struct aa_loaddata, count);
+
+	do_loaddata_free(d);
+}
+
 /*
  * need to take the ns mutex lock which is NOT safe most places that
  * put_loaddata is called, so we have to delay freeing it
  */
-static void do_loaddata_free(struct work_struct *work)
+static void do_ploaddata_rmfs(struct work_struct *work)
 {
 	struct aa_loaddata *d = container_of(work, struct aa_loaddata, work);
 	struct aa_ns *ns = aa_get_ns(d->ns);
 
 	if (ns) {
 		mutex_lock_nested(&ns->lock, ns->level);
+		/* remove fs ref to loaddata */
 		__aa_fs_remove_rawdata(d);
 		mutex_unlock(&ns->lock);
 		aa_put_ns(ns);
 	}
-
-	kfree_sensitive(d->hash);
-	kfree_sensitive(d->name);
-	kvfree(d->data);
-	kfree_sensitive(d);
+	/* called by dropping last pcount, so drop its associated icount */
+	aa_put_i_loaddata(d);
 }
 
-void aa_loaddata_kref(struct kref *kref)
+void aa_ploaddata_kref(struct kref *kref)
 {
-	struct aa_loaddata *d = container_of(kref, struct aa_loaddata, count);
+	struct aa_loaddata *d = container_of(kref, struct aa_loaddata, pcount);
 
 	if (d) {
-		INIT_WORK(&d->work, do_loaddata_free);
+		INIT_WORK(&d->work, do_ploaddata_rmfs);
 		schedule_work(&d->work);
 	}
 }

@@ -154,6 +167,7 @@ struct aa_loaddata *aa_loaddata_alloc(si
 		return ERR_PTR(-ENOMEM);
 	}
 	kref_init(&d->count);
+	kref_init(&d->pcount);
 	INIT_LIST_HEAD(&d->list);
 
 	return d;
author maintainer
#13 lore raw

PATCH 13/13 apparmor: fix race between freeing data and fs accessing it

In reply to Greg Kroah-Hartman

6.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: John Johansen <[email protected]>

commit 8e135b8aee5a06c52a4347a5a6d51223c6f36ba3 upstream.

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 <[email protected]>
Reviewed-by Georgia Garcia <[email protected]>
Reviewed-by Maxime Bélair <[email protected]>
Reviewed-by Cengiz Can <[email protected]>
Signed-off-by John Johansen <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 security/apparmor/apparmorfs.c            |  194 +++++++++++++++++-------------
 security/apparmor/include/label.h         |   16 +-
 security/apparmor/include/lib.h           |   12 +
 security/apparmor/include/policy.h        |    8 -
 security/apparmor/include/policy_unpack.h |    6 
 security/apparmor/label.c                 |   12 +
 security/apparmor/policy_unpack.c         |    6 
 7 files changed, 153 insertions(+), 101 deletions(-)
security/apparmor/apparmorfs.c +114 -80
@@ -32,6 +32,7 @@
 #include "include/crypto.h"
 #include "include/ipc.h"
 #include "include/label.h"
+#include "include/lib.h"
 #include "include/policy.h"
 #include "include/policy_ns.h"
 #include "include/resource.h"

@@ -62,6 +63,7 @@
  * securityfs and apparmorfs filesystems.
  */
 
+#define IREF_POISON 101
 
 /*
  * support fns

@@ -153,6 +155,71 @@ static int aafs_show_path(struct seq_fil
 	return 0;
 }
 
+static struct aa_ns *get_ns_common_ref(struct aa_common_ref *ref)
+{
+	if (ref) {
+		struct aa_label *reflabel = container_of(ref, struct aa_label,
+							 count);
+		return aa_get_ns(labels_ns(reflabel));
+	}
+
+	return NULL;
+}
+
+static struct aa_proxy *get_proxy_common_ref(struct aa_common_ref *ref)
+{
+	if (ref)
+		return aa_get_proxy(container_of(ref, struct aa_proxy, count));
+
+	return NULL;
+}
+
+static struct aa_loaddata *get_loaddata_common_ref(struct aa_common_ref *ref)
+{
+	if (ref)
+		return aa_get_i_loaddata(container_of(ref, struct aa_loaddata,
+						      count));
+	return NULL;
+}
+
+static void aa_put_common_ref(struct aa_common_ref *ref)
+{
+	if (!ref)
+		return;
+
+	switch (ref->reftype) {
+	case REF_RAWDATA:
+		aa_put_i_loaddata(container_of(ref, struct aa_loaddata,
+					       count));
+		break;
+	case REF_PROXY:
+		aa_put_proxy(container_of(ref, struct aa_proxy,
+					  count));
+		break;
+	case REF_NS:
+		/* ns count is held on its unconfined label */
+		aa_put_ns(labels_ns(container_of(ref, struct aa_label, count)));
+		break;
+	default:
+		AA_BUG(true, "unknown refcount type");
+		break;
+	}
+}
+
+static void aa_get_common_ref(struct aa_common_ref *ref)
+{
+	kref_get(&ref->count);
+}
+
+static void aafs_evict(struct inode *inode)
+{
+	struct aa_common_ref *ref = inode->i_private;
+
+	clear_inode(inode);
+	aa_put_common_ref(ref);
+	inode->i_private = (void *) IREF_POISON;
+}
+
 static void aafs_free_inode(struct inode *inode)
 {
 	if (S_ISLNK(inode->i_mode))

@@ -162,6 +229,7 @@ static void aafs_free_inode(struct inode
 
 static const struct super_operations aafs_super_ops = {
 	.statfs = simple_statfs,
+	.evict_inode = aafs_evict,
 	.free_inode = aafs_free_inode,
 	.show_path = aafs_show_path,
 };

@@ -262,7 +330,8 @@ static int __aafs_setup_d_inode(struct i
  * aafs_remove(). Will return ERR_PTR on failure.
  */
 static struct dentry *aafs_create(const char *name, umode_t mode,
-				  struct dentry *parent, void *data, void *link,
+				  struct dentry *parent,
+				  struct aa_common_ref *data, void *link,
 				  const struct file_operations *fops,
 				  const struct inode_operations *iops)
 {

@@ -299,6 +368,9 @@ static struct dentry *aafs_create(const
 		goto fail_dentry;
 	inode_unlock(dir);
 
+	if (data)
+		aa_get_common_ref(data);
+
 	return dentry;
 
 fail_dentry:

@@ -323,7 +395,8 @@ fail_lock:
  * see aafs_create
  */
 static struct dentry *aafs_create_file(const char *name, umode_t mode,
-				       struct dentry *parent, void *data,
+				       struct dentry *parent,
+				       struct aa_common_ref *data,
 				       const struct file_operations *fops)
 {
 	return aafs_create(name, mode, parent, data, NULL, fops, NULL);

@@ -453,7 +526,7 @@ end_section:
 static ssize_t profile_load(struct file *f, const char __user *buf, size_t size,
 			    loff_t *pos)
 {
-	struct aa_ns *ns = aa_get_ns(f->f_inode->i_private);
+	struct aa_ns *ns = get_ns_common_ref(f->f_inode->i_private);
 	int error = policy_update(AA_MAY_LOAD_POLICY, buf, size, pos, ns,
 				  f->f_cred);
 

@@ -471,7 +544,7 @@ static const struct file_operations aa_f
 static ssize_t profile_replace(struct file *f, const char __user *buf,
 			       size_t size, loff_t *pos)
 {
-	struct aa_ns *ns = aa_get_ns(f->f_inode->i_private);
+	struct aa_ns *ns = get_ns_common_ref(f->f_inode->i_private);
 	int error = policy_update(AA_MAY_LOAD_POLICY | AA_MAY_REPLACE_POLICY,
 				  buf, size, pos, ns, f->f_cred);
 	aa_put_ns(ns);

@@ -491,7 +564,7 @@ static ssize_t profile_remove(struct fil
 	struct aa_loaddata *data;
 	struct aa_label *label;
 	ssize_t error;
-	struct aa_ns *ns = aa_get_ns(f->f_inode->i_private);
+	struct aa_ns *ns = get_ns_common_ref(f->f_inode->i_private);
 
 	label = begin_current_label_crit_section();
 	/* high level check about policy management - fine grained in

@@ -581,7 +654,7 @@ static int ns_revision_open(struct inode
 	if (!rev)
 		return -ENOMEM;
 
-	rev->ns = aa_get_ns(inode->i_private);
+	rev->ns = get_ns_common_ref(inode->i_private);
 	if (!rev->ns)
 		rev->ns = aa_get_current_ns();
 	file->private_data = rev;

@@ -1067,7 +1140,7 @@ static const struct file_operations seq_
 static int seq_profile_open(struct inode *inode, struct file *file,
 			    int (*show)(struct seq_file *, void *))
 {
-	struct aa_proxy *proxy = aa_get_proxy(inode->i_private);
+	struct aa_proxy *proxy = get_proxy_common_ref(inode->i_private);
 	int error = single_open(file, show, proxy);
 
 	if (error) {

@@ -1259,7 +1332,7 @@ static const struct file_operations seq_
 static int seq_rawdata_open(struct inode *inode, struct file *file,
 			    int (*show)(struct seq_file *, void *))
 {
-	struct aa_loaddata *data = aa_get_i_loaddata(inode->i_private);
+	struct aa_loaddata *data = get_loaddata_common_ref(inode->i_private);
 	int error;
 
 	if (!data)

@@ -1392,7 +1465,7 @@ static int rawdata_open(struct inode *in
 	if (!aa_current_policy_view_capable(NULL))
 		return -EACCES;
 
-	loaddata = aa_get_i_loaddata(inode->i_private);
+	loaddata = get_loaddata_common_ref(inode->i_private);
 	if (!loaddata)
 		return -ENOENT;
 

@@ -1437,7 +1510,6 @@ static void remove_rawdata_dents(struct
 		if (!IS_ERR_OR_NULL(rawdata->dents[i])) {
 			aafs_remove(rawdata->dents[i]);
 			rawdata->dents[i] = NULL;
-			aa_put_i_loaddata(rawdata);
 		}
 	}
 }

@@ -1476,45 +1548,41 @@ int __aa_fs_create_rawdata(struct aa_ns
 	if (IS_ERR(dir))
 		/* ->name freed when rawdata freed */
 		return PTR_ERR(dir);
-	aa_get_i_loaddata(rawdata);
 	rawdata->dents[AAFS_LOADDATA_DIR] = dir;
 
-	dent = aafs_create_file("abi", S_IFREG | 0444, dir, rawdata,
+	dent = aafs_create_file("abi", S_IFREG | 0444, dir, &rawdata->count,
 				      &seq_rawdata_abi_fops);
 	if (IS_ERR(dent))
 		goto fail;
-	aa_get_i_loaddata(rawdata);
 	rawdata->dents[AAFS_LOADDATA_ABI] = dent;
 
-	dent = aafs_create_file("revision", S_IFREG | 0444, dir, rawdata,
-				      &seq_rawdata_revision_fops);
+	dent = aafs_create_file("revision", S_IFREG | 0444, dir,
+				&rawdata->count,
+				&seq_rawdata_revision_fops);
 	if (IS_ERR(dent))
 		goto fail;
-	aa_get_i_loaddata(rawdata);
 	rawdata->dents[AAFS_LOADDATA_REVISION] = dent;
 
 	if (aa_g_hash_policy) {
 		dent = aafs_create_file("sha256", S_IFREG | 0444, dir,
-					      rawdata, &seq_rawdata_hash_fops);
+					&rawdata->count,
+					&seq_rawdata_hash_fops);
 		if (IS_ERR(dent))
 			goto fail;
-		aa_get_i_loaddata(rawdata);
 		rawdata->dents[AAFS_LOADDATA_HASH] = dent;
 	}
 
 	dent = aafs_create_file("compressed_size", S_IFREG | 0444, dir,
-				rawdata,
+				&rawdata->count,
 				&seq_rawdata_compressed_size_fops);
 	if (IS_ERR(dent))
 		goto fail;
-	aa_get_i_loaddata(rawdata);
 	rawdata->dents[AAFS_LOADDATA_COMPRESSED_SIZE] = dent;
 
-	dent = aafs_create_file("raw_data", S_IFREG | 0444,
-				      dir, rawdata, &rawdata_fops);
+	dent = aafs_create_file("raw_data", S_IFREG | 0444, dir,
+				&rawdata->count, &rawdata_fops);
 	if (IS_ERR(dent))
 		goto fail;
-	aa_get_i_loaddata(rawdata);
 	rawdata->dents[AAFS_LOADDATA_DATA] = dent;
 	d_inode(dent)->i_size = rawdata->size;
 

@@ -1525,7 +1593,6 @@ int __aa_fs_create_rawdata(struct aa_ns
 
 fail:
 	remove_rawdata_dents(rawdata);
-	aa_put_i_loaddata(rawdata);
 	return PTR_ERR(dent);
 }
 #endif /* CONFIG_SECURITY_APPARMOR_EXPORT_BINARY */

@@ -1549,13 +1616,10 @@ void __aafs_profile_rmdir(struct aa_prof
 		__aafs_profile_rmdir(child);
 
 	for (i = AAFS_PROF_SIZEOF - 1; i >= 0; --i) {
-		struct aa_proxy *proxy;
 		if (!profile->dents[i])
 			continue;
 
-		proxy = d_inode(profile->dents[i])->i_private;
 		aafs_remove(profile->dents[i]);
-		aa_put_proxy(proxy);
 		profile->dents[i] = NULL;
 	}
 }

@@ -1589,14 +1653,7 @@ static struct dentry *create_profile_fil
 					  struct aa_profile *profile,
 					  const struct file_operations *fops)
 {
-	struct aa_proxy *proxy = aa_get_proxy(profile->label.proxy);
-	struct dentry *dent;
-
-	dent = aafs_create_file(name, S_IFREG | 0444, dir, proxy, fops);
-	if (IS_ERR(dent))
-		aa_put_proxy(proxy);
-
-	return dent;
+	return aafs_create_file(name, S_IFREG | 0444, dir, &profile->label.proxy->count, fops);
 }
 
 #ifdef CONFIG_SECURITY_APPARMOR_EXPORT_BINARY

@@ -1642,7 +1699,8 @@ static const char *rawdata_get_link_base
 					 struct delayed_call *done,
 					 const char *name)
 {
-	struct aa_proxy *proxy = inode->i_private;
+	struct aa_common_ref *ref = inode->i_private;
+	struct aa_proxy *proxy = container_of(ref, struct aa_proxy, count);
 	struct aa_label *label;
 	struct aa_profile *profile;
 	char *target;

@@ -1784,27 +1842,24 @@ int __aafs_profile_mkdir(struct aa_profi
 	if (profile->rawdata) {
 		if (aa_g_hash_policy) {
 			dent = aafs_create("raw_sha256", S_IFLNK | 0444, dir,
-					   profile->label.proxy, NULL, NULL,
-					   &rawdata_link_sha256_iops);
+					   &profile->label.proxy->count, NULL,
+					   NULL, &rawdata_link_sha256_iops);
 			if (IS_ERR(dent))
 				goto fail;
-			aa_get_proxy(profile->label.proxy);
 			profile->dents[AAFS_PROF_RAW_HASH] = dent;
 		}
 		dent = aafs_create("raw_abi", S_IFLNK | 0444, dir,
-				   profile->label.proxy, NULL, NULL,
+				   &profile->label.proxy->count, NULL, NULL,
 				   &rawdata_link_abi_iops);
 		if (IS_ERR(dent))
 			goto fail;
-		aa_get_proxy(profile->label.proxy);
 		profile->dents[AAFS_PROF_RAW_ABI] = dent;
 
 		dent = aafs_create("raw_data", S_IFLNK | 0444, dir,
-				   profile->label.proxy, NULL, NULL,
+				   &profile->label.proxy->count, NULL, NULL,
 				   &rawdata_link_data_iops);
 		if (IS_ERR(dent))
 			goto fail;
-		aa_get_proxy(profile->label.proxy);
 		profile->dents[AAFS_PROF_RAW_DATA] = dent;
 	}
 #endif /*CONFIG_SECURITY_APPARMOR_EXPORT_BINARY */

@@ -1841,7 +1896,7 @@ static struct dentry *ns_mkdir_op(struct
 	if (error)
 		return ERR_PTR(error);
 
-	parent = aa_get_ns(dir->i_private);
+	parent = get_ns_common_ref(dir->i_private);
 	AA_BUG(d_inode(ns_subns_dir(parent)) != dir);
 
 	/* we have to unlock and then relock to get locking order right

@@ -1891,7 +1946,7 @@ static int ns_rmdir_op(struct inode *dir
 	if (error)
 		return error;
 
-	parent = aa_get_ns(dir->i_private);
+	parent = get_ns_common_ref(dir->i_private);
 	/* rmdir calls the generic securityfs functions to remove files
 	 * from the apparmor dir. It is up to the apparmor ns locking
 	 * to avoid races.

@@ -1961,27 +2016,6 @@ void __aafs_ns_rmdir(struct aa_ns *ns)
 
 	__aa_fs_list_remove_rawdata(ns);
 
-	if (ns_subns_dir(ns)) {
-		sub = d_inode(ns_subns_dir(ns))->i_private;
-		aa_put_ns(sub);
-	}
-	if (ns_subload(ns)) {
-		sub = d_inode(ns_subload(ns))->i_private;
-		aa_put_ns(sub);
-	}
-	if (ns_subreplace(ns)) {
-		sub = d_inode(ns_subreplace(ns))->i_private;
-		aa_put_ns(sub);
-	}
-	if (ns_subremove(ns)) {
-		sub = d_inode(ns_subremove(ns))->i_private;
-		aa_put_ns(sub);
-	}
-	if (ns_subrevision(ns)) {
-		sub = d_inode(ns_subrevision(ns))->i_private;
-		aa_put_ns(sub);
-	}
-
 	for (i = AAFS_NS_SIZEOF - 1; i >= 0; --i) {
 		aafs_remove(ns->dents[i]);
 		ns->dents[i] = NULL;

@@ -2006,40 +2040,40 @@ static int __aafs_ns_mkdir_entries(struc
 		return PTR_ERR(dent);
 	ns_subdata_dir(ns) = dent;
 
-	dent = aafs_create_file("revision", 0444, dir, ns,
+	dent = aafs_create_file("revision", 0444, dir,
+				&ns->unconfined->label.count,
 				&aa_fs_ns_revision_fops);
 	if (IS_ERR(dent))
 		return PTR_ERR(dent);
-	aa_get_ns(ns);
 	ns_subrevision(ns) = dent;
 
-	dent = aafs_create_file(".load", 0640, dir, ns,
-				      &aa_fs_profile_load);
+	dent = aafs_create_file(".load", 0640, dir,
+				&ns->unconfined->label.count,
+				&aa_fs_profile_load);
 	if (IS_ERR(dent))
 		return PTR_ERR(dent);
-	aa_get_ns(ns);
 	ns_subload(ns) = dent;
 
-	dent = aafs_create_file(".replace", 0640, dir, ns,
-				      &aa_fs_profile_replace);
+	dent = aafs_create_file(".replace", 0640, dir,
+				&ns->unconfined->label.count,
+				&aa_fs_profile_replace);
 	if (IS_ERR(dent))
 		return PTR_ERR(dent);
-	aa_get_ns(ns);
 	ns_subreplace(ns) = dent;
 
-	dent = aafs_create_file(".remove", 0640, dir, ns,
-				      &aa_fs_profile_remove);
+	dent = aafs_create_file(".remove", 0640, dir,
+				&ns->unconfined->label.count,
+				&aa_fs_profile_remove);
 	if (IS_ERR(dent))
 		return PTR_ERR(dent);
-	aa_get_ns(ns);
 	ns_subremove(ns) = dent;
 
 	  /* use create_dentry so we can supply private data */
-	dent = aafs_create("namespaces", S_IFDIR | 0755, dir, ns, NULL, NULL,
-			   &ns_dir_inode_operations);
+	dent = aafs_create("namespaces", S_IFDIR | 0755, dir,
+			   &ns->unconfined->label.count,
+			   NULL, NULL, &ns_dir_inode_operations);
 	if (IS_ERR(dent))
 		return PTR_ERR(dent);
-	aa_get_ns(ns);
 	ns_subns_dir(ns) = dent;
 
 	return 0;
security/apparmor/include/label.h +8 -8
@@ -102,7 +102,7 @@ enum label_flags {
 
 struct aa_label;
 struct aa_proxy {
-	struct kref count;
+	struct aa_common_ref count;
 	struct aa_label __rcu *label;
 };
 

@@ -125,7 +125,7 @@ struct label_it {
  * vec: vector of profiles comprising the compound label
  */
 struct aa_label {
-	struct kref count;
+	struct aa_common_ref count;
 	struct rb_node node;
 	struct rcu_head rcu;
 	struct aa_proxy *proxy;

@@ -357,7 +357,7 @@ int aa_label_match(struct aa_profile *pr
  */
 static inline struct aa_label *__aa_get_label(struct aa_label *l)
 {
-	if (l && kref_get_unless_zero(&l->count))
+	if (l && kref_get_unless_zero(&l->count.count))
 		return l;
 
 	return NULL;

@@ -366,7 +366,7 @@ static inline struct aa_label *__aa_get_
 static inline struct aa_label *aa_get_label(struct aa_label *l)
 {
 	if (l)
-		kref_get(&(l->count));
+		kref_get(&(l->count.count));
 
 	return l;
 }

@@ -386,7 +386,7 @@ static inline struct aa_label *aa_get_la
 	rcu_read_lock();
 	do {
 		c = rcu_dereference(*l);
-	} while (c && !kref_get_unless_zero(&c->count));
+	} while (c && !kref_get_unless_zero(&c->count.count));
 	rcu_read_unlock();
 
 	return c;

@@ -426,7 +426,7 @@ static inline struct aa_label *aa_get_ne
 static inline void aa_put_label(struct aa_label *l)
 {
 	if (l)
-		kref_put(&l->count, aa_label_kref);
+		kref_put(&l->count.count, aa_label_kref);
 }
 
 /* wrapper fn to indicate semantics of the check */

@@ -443,7 +443,7 @@ void aa_proxy_kref(struct kref *kref);
 static inline struct aa_proxy *aa_get_proxy(struct aa_proxy *proxy)
 {
 	if (proxy)
-		kref_get(&(proxy->count));
+		kref_get(&(proxy->count.count));
 
 	return proxy;
 }

@@ -451,7 +451,7 @@ static inline struct aa_proxy *aa_get_pr
 static inline void aa_put_proxy(struct aa_proxy *proxy)
 {
 	if (proxy)
-		kref_put(&proxy->count, aa_proxy_kref);
+		kref_put(&proxy->count.count, aa_proxy_kref);
 }
 
 void __aa_proxy_redirect(struct aa_label *orig, struct aa_label *new);
security/apparmor/include/lib.h +12 -0
@@ -85,6 +85,18 @@ void aa_info_message(const char *str);
 /* Security blob offsets */
 extern struct lsm_blob_sizes apparmor_blob_sizes;
 
+enum reftype {
+	REF_NS,
+	REF_PROXY,
+	REF_RAWDATA,
+};
+
+/* common reference count used by data the shows up in aafs */
+struct aa_common_ref {
+	struct kref count;
+	enum reftype reftype;
+};
+
 /**
  * aa_strneq - compare null terminated @str to a non null terminated substring
  * @str: a null terminated string
security/apparmor/include/policy.h +4 -4
@@ -355,7 +355,7 @@ static inline bool profile_mediates_safe
 static inline struct aa_profile *aa_get_profile(struct aa_profile *p)
 {
 	if (p)
-		kref_get(&(p->label.count));
+		kref_get(&(p->label.count.count));
 
 	return p;
 }

@@ -369,7 +369,7 @@ static inline struct aa_profile *aa_get_
  */
 static inline struct aa_profile *aa_get_profile_not0(struct aa_profile *p)
 {
-	if (p && kref_get_unless_zero(&p->label.count))
+	if (p && kref_get_unless_zero(&p->label.count.count))
 		return p;
 
 	return NULL;

@@ -389,7 +389,7 @@ static inline struct aa_profile *aa_get_
 	rcu_read_lock();
 	do {
 		c = rcu_dereference(*p);
-	} while (c && !kref_get_unless_zero(&c->label.count));
+	} while (c && !kref_get_unless_zero(&c->label.count.count));
 	rcu_read_unlock();
 
 	return c;

@@ -402,7 +402,7 @@ static inline struct aa_profile *aa_get_
 static inline void aa_put_profile(struct aa_profile *p)
 {
 	if (p)
-		kref_put(&p->label.count, aa_label_kref);
+		kref_put(&p->label.count.count, aa_label_kref);
 }
 
 static inline int AUDIT_MODE(struct aa_profile *profile)
security/apparmor/include/policy_unpack.h +3 -3
@@ -108,7 +108,7 @@ struct aa_ext {
  * fs entries and drops the associated @count ref.
  */
 struct aa_loaddata {
-	struct kref count;
+	struct aa_common_ref count;
 	struct kref pcount;
 	struct list_head list;
 	struct work_struct work;

@@ -143,7 +143,7 @@ aa_get_i_loaddata(struct aa_loaddata *da
 {
 
 	if (data)
-		kref_get(&(data->count));
+		kref_get(&(data->count.count));
 	return data;
 }
 

@@ -171,7 +171,7 @@ struct aa_loaddata *aa_loaddata_alloc(si
 static inline void aa_put_i_loaddata(struct aa_loaddata *data)
 {
 	if (data)
-		kref_put(&data->count, aa_loaddata_kref);
+		kref_put(&data->count.count, aa_loaddata_kref);
 }
 
 static inline void aa_put_profile_loaddata(struct aa_loaddata *data)
security/apparmor/label.c +8 -4
@@ -52,7 +52,8 @@ static void free_proxy(struct aa_proxy *
 
 void aa_proxy_kref(struct kref *kref)
 {
-	struct aa_proxy *proxy = container_of(kref, struct aa_proxy, count);
+	struct aa_proxy *proxy = container_of(kref, struct aa_proxy,
+					      count.count);
 
 	free_proxy(proxy);
 }

@@ -63,7 +64,8 @@ struct aa_proxy *aa_alloc_proxy(struct a
 
 	new = kzalloc(sizeof(struct aa_proxy), gfp);
 	if (new) {
-		kref_init(&new->count);
+		kref_init(&new->count.count);
+		new->count.reftype = REF_PROXY;
 		rcu_assign_pointer(new->label, aa_get_label(label));
 	}
 	return new;

@@ -375,7 +377,8 @@ static void label_free_rcu(struct rcu_he
 
 void aa_label_kref(struct kref *kref)
 {
-	struct aa_label *label = container_of(kref, struct aa_label, count);
+	struct aa_label *label = container_of(kref, struct aa_label,
+					      count.count);
 	struct aa_ns *ns = labels_ns(label);
 
 	if (!ns) {

@@ -412,7 +415,8 @@ bool aa_label_init(struct aa_label *labe
 
 	label->size = size;			/* doesn't include null */
 	label->vec[size] = NULL;		/* null terminate */
-	kref_init(&label->count);
+	kref_init(&label->count.count);
+	label->count.reftype = REF_NS;		/* for aafs purposes */
 	RB_CLEAR_NODE(&label->node);
 
 	return true;
security/apparmor/policy_unpack.c +4 -2
@@ -119,7 +119,8 @@ static void do_loaddata_free(struct aa_l
 
 void aa_loaddata_kref(struct kref *kref)
 {
-	struct aa_loaddata *d = container_of(kref, struct aa_loaddata, count);
+	struct aa_loaddata *d = container_of(kref, struct aa_loaddata,
+					     count.count);
 
 	do_loaddata_free(d);
 }

@@ -166,7 +167,8 @@ struct aa_loaddata *aa_loaddata_alloc(si
 		kfree(d);
 		return ERR_PTR(-ENOMEM);
 	}
-	kref_init(&d->count);
+	kref_init(&d->count.count);
+	d->count.reftype = REF_RAWDATA;
 	kref_init(&d->pcount);
 	INIT_LIST_HEAD(&d->list);
author maintainer
#2 lore raw

PATCH 2/13 net/sched: Only allow act_ct to bind to clsact/ingress qdiscs and shared blocks

In reply to Greg Kroah-Hartman

6.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: Victor Nogueira <[email protected]>

commit 11cb63b0d1a0685e0831ae3c77223e002ef18189 upstream.

As Paolo said earlier [1]:

"Since the blamed commit below, classify can return TC_ACT_CONSUMED while the current skb being held by the defragmentation engine. As reported by GangMin Kim, if such packet is that may cause a UaF when the defrag engine later on tries to tuch again such packet."

act_ct was never meant to be used in the egress path, however some users are attaching it to egress today [2]. Attempting to reach a middle ground, we noticed that, while most qdiscs are not handling TC_ACT_CONSUMED, clsact/ingress qdiscs are. With that in mind, we address the issue by only allowing act_ct to bind to clsact/ingress qdiscs and shared blocks. That way it's still possible to attach act_ct to egress (albeit only with clsact).

[1] https://lore.kernel.org/netdev/674b8cbfc385c6f37fb29a1de08d8fe5c2b0fbee.1771321118.git.pabeni@redhat.com/
[2] https://lore.kernel.org/netdev/[email protected]/

Reported-by GangMin Kim <[email protected]>
Fixes 3f14b377d01d ("net/sched: act_ct: fix skb leak and crash on ooo frags")
CC [email protected]
Signed-off-by Victor Nogueira <[email protected]>
Acked-by Jamal Hadi Salim <[email protected]>
Link https://patch.msgid.link/[email protected] open here
Signed-off-by Jakub Kicinski <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 include/net/act_api.h |    1 +
 net/sched/act_ct.c    |    6 ++++++
 net/sched/cls_api.c   |    7 +++++++
 3 files changed, 14 insertions(+)
include/net/act_api.h +1 -0
@@ -70,6 +70,7 @@ struct tc_action {
 #define TCA_ACT_FLAGS_REPLACE	(1U << (TCA_ACT_FLAGS_USER_BITS + 2))
 #define TCA_ACT_FLAGS_NO_RTNL	(1U << (TCA_ACT_FLAGS_USER_BITS + 3))
 #define TCA_ACT_FLAGS_AT_INGRESS	(1U << (TCA_ACT_FLAGS_USER_BITS + 4))
+#define TCA_ACT_FLAGS_AT_INGRESS_OR_CLSACT	(1U << (TCA_ACT_FLAGS_USER_BITS + 5))
 
 /* Update lastuse only if needed, to avoid dirtying a cache line.
  * We use a temp variable to avoid fetching jiffies twice.
net/sched/act_ct.c +6 -0
@@ -1358,6 +1358,12 @@ static int tcf_ct_init(struct net *net,
 		return -EINVAL;
 	}
 
+	if (bind && !(flags & TCA_ACT_FLAGS_AT_INGRESS_OR_CLSACT)) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Attaching ct to a non ingress/clsact qdisc is unsupported");
+		return -EOPNOTSUPP;
+	}
+
 	err = nla_parse_nested(tb, TCA_CT_MAX, nla, ct_policy, extack);
 	if (err < 0)
 		return err;
net/sched/cls_api.c +7 -0
@@ -2228,6 +2228,11 @@ static bool is_qdisc_ingress(__u32 class
 	return (TC_H_MIN(classid) == TC_H_MIN(TC_H_MIN_INGRESS));
 }
 
+static bool is_ingress_or_clsact(struct tcf_block *block, struct Qdisc *q)
+{
+	return tcf_block_shared(block) || (q && !!(q->flags & TCQ_F_INGRESS));
+}
+
 static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 			  struct netlink_ext_ack *extack)
 {

@@ -2420,6 +2425,8 @@ replay:
 		flags |= TCA_ACT_FLAGS_NO_RTNL;
 	if (is_qdisc_ingress(parent))
 		flags |= TCA_ACT_FLAGS_AT_INGRESS;
+	if (is_ingress_or_clsact(block, q))
+		flags |= TCA_ACT_FLAGS_AT_INGRESS_OR_CLSACT;
 	err = tp->ops->change(net, skb, tp, cl, t->tcm_handle, tca, &fh,
 			      flags, extack);
 	if (err == 0) {
author maintainer
#3 lore raw

PATCH 3/13 apparmor: validate DFA start states are in bounds in unpack_pdb

In reply to Greg Kroah-Hartman

6.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: Massimiliano Pellizzer <[email protected]>

commit 9063d7e2615f4a7ab321de6b520e23d370e58816 upstream.

Start states are read from untrusted data and used as indexes into the DFA state tables. The aa_dfa_next() function call in unpack_pdb() will access dfa->tables[YYTD_ID_BASE][start], and if the start state exceeds the number of states in the DFA, this results in an out-of-bound read.

==================================================================

 BUG: KASAN: slab-out-of-bounds in aa_dfa_next+0x2a1/0x360
 Read of size 4 at addr ffff88811956fb90 by task su/1097
 ...

Reject policies with out-of-bounds start states during unpacking to prevent the issue.

Fixes ad5ff3db53c6 ("AppArmor: Add ability to load extended policy")
Reported-by Qualys Security Advisory <[email protected]>
Tested-by Salvatore Bonaccorso <[email protected]>
Reviewed-by Georgia Garcia <[email protected]>
Reviewed-by Cengiz Can <[email protected]>
Signed-off-by Massimiliano Pellizzer <[email protected]>
Signed-off-by John Johansen <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 security/apparmor/policy_unpack.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
security/apparmor/policy_unpack.c +11 -1
@@ -770,7 +770,17 @@ static int unpack_pdb(struct aa_ext *e,
 		if (!aa_unpack_u32(e, &pdb->start[AA_CLASS_FILE], "dfa_start")) {
 			/* default start state for xmatch and file dfa */
 			pdb->start[AA_CLASS_FILE] = DFA_START;
-		}	/* setup class index */
+		}
+
+		size_t state_count = pdb->dfa->tables[YYTD_ID_BASE]->td_lolen;
+
+		if (pdb->start[0] >= state_count ||
+		    pdb->start[AA_CLASS_FILE] >= state_count) {
+			*info = "invalid dfa start state";
+			goto fail;
+		}
+
+		/* setup class index */
 		for (i = AA_CLASS_FILE + 1; i <= AA_CLASS_LAST; i++) {
 			pdb->start[i] = aa_dfa_next(pdb->dfa, pdb->start[0],
 						    i);
author maintainer
#4 lore raw

PATCH 4/13 apparmor: fix memory leak in verify_header

In reply to Greg Kroah-Hartman

6.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: Massimiliano Pellizzer <[email protected]>

commit e38c55d9f834e5b848bfed0f5c586aaf45acb825 upstream.

The function sets *ns = NULL on every call, leaking the namespace string allocated in previous iterations when multiple profiles are unpacked. This also breaks namespace consistency checking since *ns is always NULL when the comparison is made.

Remove the incorrect assignment.
The caller (aa_unpack) initializes *ns to NULL once before the loop, which is sufficient.

Fixes dd51c8485763 ("apparmor: provide base for multiple profiles to be replaced at once")
Reported-by Qualys Security Advisory <[email protected]>
Tested-by Salvatore Bonaccorso <[email protected]>
Reviewed-by Georgia Garcia <[email protected]>
Reviewed-by Cengiz Can <[email protected]>
Signed-off-by Massimiliano Pellizzer <[email protected]>
Signed-off-by John Johansen <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 security/apparmor/policy_unpack.c |    1 -
 1 file changed, 1 deletion(-)
security/apparmor/policy_unpack.c +0 -1
@@ -1177,7 +1177,6 @@ static int verify_header(struct aa_ext *
 {
 	int error = -EPROTONOSUPPORT;
 	const char *name = NULL;
-	*ns = NULL;
 
 	/* get the interface version */
 	if (!aa_unpack_u32(e, &e->version, "version")) {
author maintainer
#5 lore raw

PATCH 5/13 apparmor: replace recursive profile removal with iterative approach

In reply to Greg Kroah-Hartman

6.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: Massimiliano Pellizzer <[email protected]>

commit ab09264660f9de5d05d1ef4e225aa447c63a8747 upstream.

The profile removal code uses recursion when removing nested profiles, which can lead to kernel stack exhaustion and system crashes.

Reproducer:

  $ pf='a'; for ((i=0; i<1024; i++)); do
      echo -e "profile $pf { \n }" | apparmor_parser -K -a;
      pf="$pf//x";
  done
  $ echo -n a > /sys/kernel/security/apparmor/.remove

Replace the recursive __aa_profile_list_release() approach with an iterative approach in __remove_profile(). The function repeatedly finds and removes leaf profiles until the entire subtree is removed, maintaining the same removal semantic without recursion.

Fixes c88d4c7b049e ("AppArmor: core policy routines")
Reported-by Qualys Security Advisory <[email protected]>
Tested-by Salvatore Bonaccorso <[email protected]>
Reviewed-by Georgia Garcia <[email protected]>
Reviewed-by Cengiz Can <[email protected]>
Signed-off-by Massimiliano Pellizzer <[email protected]>
Signed-off-by John Johansen <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 security/apparmor/policy.c |   30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)
security/apparmor/policy.c +27 -3
@@ -183,19 +183,43 @@ static void __list_remove_profile(struct
 }
 
 /**
- * __remove_profile - remove old profile, and children
- * @profile: profile to be replaced  (NOT NULL)
+ * __remove_profile - remove profile, and children
+ * @profile: profile to be removed  (NOT NULL)
  *
  * Requires: namespace list lock be held, or list not be shared
  */
 static void __remove_profile(struct aa_profile *profile)
 {
+	struct aa_profile *curr, *to_remove;
+
 	AA_BUG(!profile);
 	AA_BUG(!profile->ns);
 	AA_BUG(!mutex_is_locked(&profile->ns->lock));
 
 	/* release any children lists first */
-	__aa_profile_list_release(&profile->base.profiles);
+	if (!list_empty(&profile->base.profiles)) {
+		curr = list_first_entry(&profile->base.profiles, struct aa_profile, base.list);
+
+		while (curr != profile) {
+
+			while (!list_empty(&curr->base.profiles))
+				curr = list_first_entry(&curr->base.profiles,
+							struct aa_profile, base.list);
+
+			to_remove = curr;
+			if (!list_is_last(&to_remove->base.list,
+					  &aa_deref_parent(curr)->base.profiles))
+				curr = list_next_entry(to_remove, base.list);
+			else
+				curr = aa_deref_parent(curr);
+
+			/* released by free_profile */
+			aa_label_remove(&to_remove->label);
+			__aafs_profile_rmdir(to_remove);
+			__list_remove_profile(to_remove);
+		}
+	}
+
 	/* released by free_profile */
 	aa_label_remove(&profile->label);
 	__aafs_profile_rmdir(profile);
author maintainer
#6 lore raw

PATCH 6/13 apparmor: fix: limit the number of levels of policy namespaces

In reply to Greg Kroah-Hartman

6.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: John Johansen <[email protected]>

commit 306039414932c80f8420695a24d4fe10c84ccfb2 upstream.

Currently the number of policy namespaces is not bounded relying on the user namespace limit. However policy namespaces aren't strictly tied to user namespaces and it is possible to create them and nest them arbitrarily deep which can be used to exhaust system resource.

Hard cap policy namespaces to the same depth as user namespaces.

Fixes c88d4c7b049e8 ("AppArmor: core policy routines")
Reported-by Qualys Security Advisory <[email protected]>
Reviewed-by Ryan Lee <[email protected]>
Reviewed-by Cengiz Can <[email protected]>
Signed-off-by John Johansen <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 security/apparmor/include/policy_ns.h |    2 ++
 security/apparmor/policy_ns.c         |    2 ++
 2 files changed, 4 insertions(+)
security/apparmor/include/policy_ns.h +2 -0
@@ -18,6 +18,8 @@
 #include "label.h"
 #include "policy.h"
 
+/* Match max depth of user namespaces */
+#define MAX_NS_DEPTH 32
 
 /* struct aa_ns_acct - accounting of profiles in namespace
  * @max_size: maximum space allowed for all profiles in namespace
security/apparmor/policy_ns.c +2 -0
@@ -223,6 +223,8 @@ static struct aa_ns *__aa_create_ns(stru
 	AA_BUG(!name);
 	AA_BUG(!mutex_is_locked(&parent->lock));
 
+	if (parent->level > MAX_NS_DEPTH)
+		return ERR_PTR(-ENOSPC);
 	ns = alloc_ns(parent->base.hname, name);
 	if (!ns)
 		return ERR_PTR(-ENOMEM);
author maintainer
#7 lore raw

PATCH 7/13 apparmor: fix side-effect bug in match_char() macro usage

In reply to Greg Kroah-Hartman

6.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: Massimiliano Pellizzer <[email protected]>

commit 8756b68edae37ff546c02091989a4ceab3f20abd upstream.

The match_char() macro evaluates its character parameter multiple times when traversing differential encoding chains. When invoked with *str++, the string pointer advances on each iteration of the inner do-while loop, causing the DFA to check different characters at each iteration and therefore skip input characters. This results in out-of-bounds reads when the pointer advances past the input buffer boundary.

[   94.984676] ==================================================================
[   94.985301] BUG: KASAN: slab-out-of-bounds in aa_dfa_match+0x5ae/0x760
[   94.985655] Read of size 1 at addr ffff888100342000 by task file/976
[   94.986319] CPU: 7 UID: 1000 PID: 976 Comm: file Not tainted 6.19.0-rc7-next-20260127 #1 PREEMPT(lazy)
[   94.986322] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[   94.986329] Call Trace:
[   94.986341]  <TASK>
[   94.986347]  dump_stack_lvl+0x5e/0x80
[   94.986374]  print_report+0xc8/0x270
[   94.986384]  ? aa_dfa_match+0x5ae/0x760
[   94.986388]  kasan_report+0x118/0x150
[   94.986401]  ? aa_dfa_match+0x5ae/0x760
[   94.986405]  aa_dfa_match+0x5ae/0x760
[   94.986408]  __aa_path_perm+0x131/0x400
[   94.986418]  aa_path_perm+0x219/0x2f0
[   94.986424]  apparmor_file_open+0x345/0x570
[   94.986431]  security_file_open+0x5c/0x140
[   94.986442]  do_dentry_open+0x2f6/0x1120
[   94.986450]  vfs_open+0x38/0x2b0
[   94.986453]  ? may_open+0x1e2/0x2b0
[   94.986466]  path_openat+0x231b/0x2b30
[   94.986469]  ? __x64_sys_openat+0xf8/0x130
[   94.986477]  do_file_open+0x19d/0x360
[   94.986487]  do_sys_openat2+0x98/0x100
[   94.986491]  __x64_sys_openat+0xf8/0x130
[   94.986499]  do_syscall_64+0x8e/0x660
[   94.986515]  ? count_memcg_events+0x15f/0x3c0
[   94.986526]  ? srso_alias_return_thunk+0x5/0xfbef5
[   94.986540]  ? handle_mm_fault+0x1639/0x1ef0
[   94.986551]  ? vma_start_read+0xf0/0x320
[   94.986558]  ? srso_alias_return_thunk+0x5/0xfbef5
[   94.986561]  ? srso_alias_return_thunk+0x5/0xfbef5
[   94.986563]  ? fpregs_assert_state_consistent+0x50/0xe0
[   94.986572]  ? srso_alias_return_thunk+0x5/0xfbef5
[   94.986574]  ? arch_exit_to_user_mode_prepare+0x9/0xb0
[   94.986587]  ? srso_alias_return_thunk+0x5/0xfbef5
[   94.986588]  ? irqentry_exit+0x3c/0x590
[   94.986595]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[   94.986597] RIP: 0033:0x7fda4a79c3ea

Fix by extracting the character value before invoking match_char, ensuring single evaluation per outer loop.

Fixes 074c1cd798cb ("apparmor: dfa move character match into a macro")
Reported-by Qualys Security Advisory <[email protected]>
Tested-by Salvatore Bonaccorso <[email protected]>
Reviewed-by Georgia Garcia <[email protected]>
Reviewed-by Cengiz Can <[email protected]>
Signed-off-by Massimiliano Pellizzer <[email protected]>
Signed-off-by John Johansen <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 security/apparmor/match.c |   30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)
security/apparmor/match.c +20 -10
@@ -463,13 +463,18 @@ aa_state_t aa_dfa_match_len(struct aa_df
 	if (dfa->tables[YYTD_ID_EC]) {
 		/* Equivalence class table defined */
 		u8 *equiv = EQUIV_TABLE(dfa);
-		for (; len; len--)
-			match_char(state, def, base, next, check,
-				   equiv[(u8) *str++]);
+		for (; len; len--) {
+			u8 c = equiv[(u8) *str];
+
+			match_char(state, def, base, next, check, c);
+			str++;
+		}
 	} else {
 		/* default is direct to next state */
-		for (; len; len--)
-			match_char(state, def, base, next, check, (u8) *str++);
+		for (; len; len--) {
+			match_char(state, def, base, next, check, (u8) *str);
+			str++;
+		}
 	}
 
 	return state;

@@ -503,13 +508,18 @@ aa_state_t aa_dfa_match(struct aa_dfa *d
 		/* Equivalence class table defined */
 		u8 *equiv = EQUIV_TABLE(dfa);
 		/* default is direct to next state */
-		while (*str)
-			match_char(state, def, base, next, check,
-				   equiv[(u8) *str++]);
+		while (*str) {
+			u8 c = equiv[(u8) *str];
+
+			match_char(state, def, base, next, check, c);
+			str++;
+		}
 	} else {
 		/* default is direct to next state */
-		while (*str)
-			match_char(state, def, base, next, check, (u8) *str++);
+		while (*str) {
+			match_char(state, def, base, next, check, (u8) *str);
+			str++;
+		}
 	}
 
 	return state;
author maintainer
#8 lore raw

PATCH 8/13 apparmor: fix missing bounds check on DEFAULT table in verify_dfa()

In reply to Greg Kroah-Hartman

6.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: Massimiliano Pellizzer <[email protected]>

commit d352873bbefa7eb39995239d0b44ccdf8aaa79a4 upstream.

The verify_dfa() function only checks DEFAULT_TABLE bounds when the state is not differentially encoded.

When the verification loop traverses the differential encoding chain, it reads k = DEFAULT_TABLE[j] and uses k as an array index without validation. A malformed DFA with DEFAULT_TABLE[j] >= state_count, therefore, causes both out-of-bounds reads and writes.

[   57.179855] ==================================================================
[   57.180549] BUG: KASAN: slab-out-of-bounds in verify_dfa+0x59a/0x660
[   57.180904] Read of size 4 at addr ffff888100eadec4 by task su/993
[   57.181554] CPU: 1 UID: 0 PID: 993 Comm: su Not tainted 6.19.0-rc7-next-20260127 #1 PREEMPT(lazy)
[   57.181558] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[   57.181563] Call Trace:
[   57.181572]  <TASK>
[   57.181577]  dump_stack_lvl+0x5e/0x80
[   57.181596]  print_report+0xc8/0x270
[   57.181605]  ? verify_dfa+0x59a/0x660
[   57.181608]  kasan_report+0x118/0x150
[   57.181620]  ? verify_dfa+0x59a/0x660
[   57.181623]  verify_dfa+0x59a/0x660
[   57.181627]  aa_dfa_unpack+0x1610/0x1740
[   57.181629]  ? __kmalloc_cache_noprof+0x1d0/0x470
[   57.181640]  unpack_pdb+0x86d/0x46b0
[   57.181647]  ? srso_alias_return_thunk+0x5/0xfbef5
[   57.181653]  ? srso_alias_return_thunk+0x5/0xfbef5
[   57.181656]  ? aa_unpack_nameX+0x1a8/0x300
[   57.181659]  aa_unpack+0x20b0/0x4c30
[   57.181662]  ? srso_alias_return_thunk+0x5/0xfbef5
[   57.181664]  ? stack_depot_save_flags+0x33/0x700
[   57.181681]  ? kasan_save_track+0x4f/0x80
[   57.181683]  ? kasan_save_track+0x3e/0x80
[   57.181686]  ? __kasan_kmalloc+0x93/0xb0
[   57.181688]  ? __kvmalloc_node_noprof+0x44a/0x780
[   57.181693]  ? aa_simple_write_to_buffer+0x54/0x130
[   57.181697]  ? policy_update+0x154/0x330
[   57.181704]  aa_replace_profiles+0x15a/0x1dd0
[   57.181707]  ? srso_alias_return_thunk+0x5/0xfbef5
[   57.181710]  ? __kvmalloc_node_noprof+0x44a/0x780
[   57.181712]  ? aa_loaddata_alloc+0x77/0x140
[   57.181715]  ? srso_alias_return_thunk+0x5/0xfbef5
[   57.181717]  ? _copy_from_user+0x2a/0x70
[   57.181730]  policy_update+0x17a/0x330
[   57.181733]  profile_replace+0x153/0x1a0
[   57.181735]  ? rw_verify_area+0x93/0x2d0
[   57.181740]  vfs_write+0x235/0xab0
[   57.181745]  ksys_write+0xb0/0x170
[   57.181748]  do_syscall_64+0x8e/0x660
[   57.181762]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[   57.181765] RIP: 0033:0x7f6192792eb2

Remove the MATCH_FLAG_DIFF_ENCODE condition to validate all DEFAULT_TABLE entries unconditionally.

Fixes 031dcc8f4e84 ("apparmor: dfa add support for state differential encoding")
Reported-by Qualys Security Advisory <[email protected]>
Tested-by Salvatore Bonaccorso <[email protected]>
Reviewed-by Georgia Garcia <[email protected]>
Reviewed-by Cengiz Can <[email protected]>
Signed-off-by Massimiliano Pellizzer <[email protected]>
Signed-off-by John Johansen <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 security/apparmor/match.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
security/apparmor/match.c +3 -2
@@ -160,9 +160,10 @@ static int verify_dfa(struct aa_dfa *dfa
 	if (state_count == 0)
 		goto out;
 	for (i = 0; i < state_count; i++) {
-		if (!(BASE_TABLE(dfa)[i] & MATCH_FLAG_DIFF_ENCODE) &&
-		    (DEFAULT_TABLE(dfa)[i] >= state_count))
+		if (DEFAULT_TABLE(dfa)[i] >= state_count) {
+			pr_err("AppArmor DFA default state out of bounds");
 			goto out;
+		}
 		if (BASE_TABLE(dfa)[i] & MATCH_FLAGS_INVALID) {
 			pr_err("AppArmor DFA state with invalid match flags");
 			goto out;
author maintainer
#9 lore raw

PATCH 9/13 apparmor: Fix double free of ns_name in aa_replace_profiles()

In reply to Greg Kroah-Hartman

6.19-stable review patch. If anyone has any objections, please let me know.

------------------

From: John Johansen <[email protected]>

commit 5df0c44e8f5f619d3beb871207aded7c78414502 upstream.

if ns_name is NULL after
1071 error = aa_unpack(udata, &lh, &ns_name);

and if ent->ns_name contains an ns_name in
1089 } else if (ent->ns_name) {

then ns_name is assigned the ent->ns_name
1095 ns_name = ent->ns_name;

however ent->ns_name is freed at
1262 aa_load_ent_free(ent);

and then again when freeing ns_name at
1270 kfree(ns_name);

Fix this by NULLing out ent->ns_name after it is transferred to ns_name

Fixes 145a0ef21c8e9 ("apparmor: fix blob compression when ns is forced on a policy load

")

Reported-by Qualys Security Advisory <[email protected]>
Tested-by Salvatore Bonaccorso <[email protected]>
Reviewed-by Georgia Garcia <[email protected]>
Reviewed-by Cengiz Can <[email protected]>
Signed-off-by John Johansen <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 security/apparmor/policy.c |    1 +
 1 file changed, 1 insertion(+)
security/apparmor/policy.c +1 -0
@@ -1149,6 +1149,7 @@ ssize_t aa_replace_profiles(struct aa_ns
 				goto fail;
 			}
 			ns_name = ent->ns_name;
+			ent->ns_name = NULL;
 		} else
 			count++;
 	}
Tested-by on the v1 series
#15 lore raw
In reply to Greg Kroah-Hartman
Quoted 9 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 6.19.8 release. There are 13 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Fri, 13 Mar 2026 20:03:10 +0000. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

Quoted 9 lines
	https://www.kernel.org/pub/linux/kernel/v6.x/stable-review/patch-6.19.8-rc1.gz

or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.19.y
and the diffstat can be found below.

thanks,

greg k-h

Compiled and booted on my test system. No dmesg regressions.

Tested-by Shuah Khan <[email protected]>

thanks,
-- Shuah

Tested-by on the v1 series
#17 lore raw
In reply to Greg Kroah-Hartman
Quoted 9 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 6.19.8 release. There are 13 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Fri, 13 Mar 2026 20:03:10 +0000. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

Quoted 8 lines
	https://www.kernel.org/pub/linux/kernel/v6.x/stable-review/patch-6.19.8-rc1.gz

or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.19.y
and the diffstat can be found below.

thanks,

greg k-h

Built and booted successfully on RISC-V RV64 (HiFive Unmatched).

Tested-by Ron Economos <[email protected]>
Tested-by on the v1 series
#18 lore raw
In reply to Greg Kroah-Hartman
Quoted 4 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 6.19.8 release. There are 13 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Tested-by Mark Brown <[email protected]>
Tested-by on the v1 series
#19 lore raw
In reply to Greg Kroah-Hartman

Hi Greg

Quoted 10 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 6.19.8 release. There are 13 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Fri, 13 Mar 2026 20:03:10 +0000. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

Quoted 9 lines
        https://www.kernel.org/pub/linux/kernel/v6.x/stable-review/patch-6.19.8-rc1.gz

or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.19.y
and the diffstat can be found below.

thanks,

greg k-h

6.19.8-rc1 tested.

Build successfully completed.
Boot successfully completed.
No dmesg regressions.
Video output normal.
Sound output normal.

Lenovo ThinkPad X1 Carbon Gen10(Intel i7-1260P(x86_64) arch linux)

[    0.000000] Linux version 6.19.8-rc1rv-gc1996363ec4b

(takeshi@ThinkPadX1Gen10J0764) (gcc (GCC) 15.2.1 20260209, GNU ld (GNU Binutils) 2.46) #1 SMP PREEMPT_DYNAMIC Fri Mar 13 22:20:12 JST 2026

Thanks

Tested-by Takeshi Ogasawara <[email protected]>
maintainer Tested-by on the v1 series
#20 lore raw
In reply to Greg Kroah-Hartman
Quoted 9 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 6.19.8 release. There are 13 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Fri, 13 Mar 2026 20:03:10 +0000. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

Quoted 8 lines
	https://www.kernel.org/pub/linux/kernel/v6.x/stable-review/patch-6.19.8-rc1.gz

or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.19.y
and the diffstat can be found below.

thanks,

greg k-h

All tests passing for Tegra ...

Test results for stable-v6.19:

    11 builds:	11 pass, 0 fail
    28 boots:	28 pass, 0 fail
    133 tests:	133 pass, 0 fail

Linux version: 6.19.8-rc1-gc1996363ec4b
Boards tested: tegra124-jetson-tk1, tegra186-p2771-0000,

                tegra186-p3509-0000+p3636-0001, tegra194-p2972-0000,
                tegra194-p3509-0000+p3668-0000, tegra20-ventana,
                tegra210-p2371-2180, tegra210-p3450-0000,
                tegra234-p3737-0000+p3701-0000,
                tegra234-p3768-0000+p3767-0005, tegra30-cardhu-a04
Tested-by Jon Hunter <[email protected]>

Jon

Tested-by on the v1 series
#21 lore raw
In reply to Greg Kroah-Hartman
Quoted 9 lines · Greg Kroah-Hartman

This is the start of the stable review cycle for the 6.19.8 release. There are 13 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Fri, 13 Mar 2026 20:03:10 +0000. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

Quoted 8 lines
	https://www.kernel.org/pub/linux/kernel/v6.x/stable-review/patch-6.19.8-rc1.gz

or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.19.y
and the diffstat can be found below.

thanks,

greg k-h

On ARCH_BRCMSTB using 32-bit and 64-bit ARM kernels, build tested on BMIPS_GENERIC:

Tested-by Florian Fainelli <[email protected]>
Signature · 1 line
Florian
Tested-by on the v1 series
#22 lore raw
In reply to Greg Kroah-Hartman
Quoted 8 lines · On Thu, 12 Mar 2026 21:03:32 +0100 Greg Kroah-Hartman…

This is the start of the stable review cycle for the 6.19.8 release. There are 13 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Fri, 13 Mar 2026 20:03:10 +0000. Anything received after that time might be too late.

Boot-tested under QEMU for Rust x86_64, arm64 and riscv64; built-tested for loongarch64:

Tested-by Miguel Ojeda <[email protected]>

Thanks!

Cheers,
Miguel