diff options
| author | Linus Torvalds <torvalds@linux-foundation.org> | 2026-10-30 21:50:13 -0700 |
|---|---|---|
| committer | Linus Torvalds <torvalds@linux-foundation.org> | 2026-10-30 21:50:13 -0700 |
| commit | 60aba79a50127e92d8c4a663d35b855e52a9da01 (patch) | |
| tree | 2224ab02a04ab210ed25eb794425ffa7a34bd218 | |
| parent | 2a500a04d6f969318c05f46aedb6e73fc1963a00 (diff) | |
| parent | ed35bbe7d06e6505401eff3f3c7a36bd1f167a17 (diff) | |
Merge tag 'apparmor-pr-2026-10-29' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
Pull AppArmor fixes from John Johansen:
"Leaks:
- fix a leak on a rejected string table entry
- fix refcount leak in aa_get_newest_profile()
- fix profile leak when a policy load fails
- remove fs entries created by a failed policy load
- on failed load ensure profile files are removed before parent
- Fix namespace reference leak in __aa_find_or_create_ns()
Use after free/double free:
- fix a use-after-free in aa_lookupn_ns()
- avoid potential double free when remapping DFA tables
Bounds checking by-passes:
- fix table_size() integer overflow on 32 bit
- check accept index + 1 is in bounds
Mediation fixes:
- Fix AF_UNSPEC mediation
- check address length before reading AF_UNSPEC bind address
- mediate AF_UNSPEC socket disconnects
- Fix sctp connect/bind by-passess
- advertise that sctp is being mediated
- fix check SCTP bind, and connect permission checks
- check connect permission for SCTP
- incorrect state machine traversal
- convert duplicated dfa transitions to match_char()
- locking issues
- don't revalidate inherited files under files->file_lock
- fix uninitialized variable warning in aa_inherit_files
- Bad dereferences
- fix NULL ctx->peer derefs in unix socket ctx updates
Unnecessary warnings:
- remove unconfined_profile() check in inet socket mediation
Kernel Doc:
- Fix the return value in split_token_from_name() kernel-doc
Misc:
- fix query_data() output overlapping the lookup key
- report a label allocation failure as -ENOMEM"
* tag 'apparmor-pr-2026-10-29' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor: (23 commits)
apparmor: fix uninitialized variable warning in aa_inherit_files
apparmor: fix profile leak when a policy load fails
apparmor: on failed load ensure profile files are removed before parent
apparmor: remove fs entries created by a failed policy load
apparmor: fix query_data() output overlapping the lookup key
apparmor: advertise that sctp is being mediated
apparmor: fix check SCTP bind, and connect permission checks
apparmor: check connect permission for SCTP
apparmor: fix a use-after-free in aa_lookupn_ns()
apparmor: Fix namespace reference leak in __aa_find_or_create_ns()
apparmor: check accept index + 1 is in bounds
apparmor: Fix the return value in split_token_from_name() kernel-doc
apparmor: convert duplicated dfa transitions to match_char()
apparmor: fix table_size() integer overflow on 32 bit
apparmor: avoid potential double free when remapping DFA tables
apparmor: report a label allocation failure as -ENOMEM
apparmor: fix a leak on a rejected string table entry
apparmor: remove unconfined_profile() check in inet socket mediation
apparmor: fix refcount leak in aa_get_newest_profile()
apparmor: check address length before reading AF_UNSPEC bind address
...
| -rw-r--r-- | security/apparmor/af_inet.c | 5 | ||||
| -rw-r--r-- | security/apparmor/af_unix.c | 20 | ||||
| -rw-r--r-- | security/apparmor/apparmorfs.c | 15 | ||||
| -rw-r--r-- | security/apparmor/file.c | 61 | ||||
| -rw-r--r-- | security/apparmor/include/label.h | 6 | ||||
| -rw-r--r-- | security/apparmor/include/match.h | 11 | ||||
| -rw-r--r-- | security/apparmor/include/policy.h | 20 | ||||
| -rw-r--r-- | security/apparmor/include/policy_ns.h | 16 | ||||
| -rw-r--r-- | security/apparmor/label.c | 15 | ||||
| -rw-r--r-- | security/apparmor/lsm.c | 19 | ||||
| -rw-r--r-- | security/apparmor/match.c | 61 | ||||
| -rw-r--r-- | security/apparmor/net.c | 4 | ||||
| -rw-r--r-- | security/apparmor/policy.c | 19 | ||||
| -rw-r--r-- | security/apparmor/policy_ns.c | 8 | ||||
| -rw-r--r-- | security/apparmor/policy_unpack.c | 23 | ||||
| -rw-r--r-- | security/apparmor/procattr.c | 3 |
16 files changed, 212 insertions, 94 deletions
diff --git a/security/apparmor/af_inet.c b/security/apparmor/af_inet.c index 516c9016537c..2a933572fa95 100644 --- a/security/apparmor/af_inet.c +++ b/security/apparmor/af_inet.c @@ -196,6 +196,9 @@ static int bind_map_addr(const struct sock *sk, struct sockaddr *addr, * AF_UNSPEC if the address is INADDR_ANY */ addr4 = (struct sockaddr_in *)addr; + /* Length check from inet_bind_sk() */ + if (addrlen < sizeof(struct sockaddr_in)) + return -EINVAL; if (addr4->sin_addr.s_addr != htonl(INADDR_ANY)) return -EAFNOSUPPORT; family = AF_INET; @@ -425,7 +428,7 @@ int aa_inet_connect_perm(struct socket *sock, struct sockaddr *addr, if (addrlen < offsetofend(struct sockaddr, sa_family)) return -EINVAL; if (addr->sa_family == AF_UNSPEC) - return 0; + return aa_sk_perm(OP_CONNECT, AA_MAY_CONNECT, sock->sk); /* do we need early bailout for !family ... */ return sk_has_perm2(sock->sk, OP_CONNECT, AA_MAY_CONNECT, profile, ad, diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c index b908e744818c..58c638830afc 100644 --- a/security/apparmor/af_unix.c +++ b/security/apparmor/af_unix.c @@ -682,7 +682,7 @@ static void update_sk_ctx(struct sock *sk, struct aa_label *label, if (old == plabel) { rcu_assign_pointer(ctx->peer_lastupdate, aa_get_label(plabel)); - } else if (aa_label_is_subset(plabel, old)) { + } else if (!old || aa_label_is_subset(plabel, old)) { rcu_assign_pointer(ctx->peer_lastupdate, aa_get_label(plabel)); rcu_assign_pointer(ctx->peer, aa_get_label(plabel)); @@ -700,13 +700,17 @@ static void update_peer_ctx(struct sock *sk, struct aa_sk_ctx *ctx, spin_lock(&unix_sk(sk)->lock); old = rcu_dereference_protected(ctx->peer, lockdep_is_held(&unix_sk(sk)->lock)); - l = aa_label_merge(old, label, GFP_ATOMIC); - if (l) { - if (l != old) { - rcu_assign_pointer(ctx->peer, l); - aa_put_label(old); - } else - aa_put_label(l); + if (!old) { + rcu_assign_pointer(ctx->peer, aa_get_label(label)); + } else { + l = aa_label_merge(old, label, GFP_ATOMIC); + if (l) { + if (l != old) { + rcu_assign_pointer(ctx->peer, l); + aa_put_label(old); + } else + aa_put_label(l); + } } spin_unlock(&unix_sk(sk)->lock); } diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index d1386722537d..a99366467217 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -919,7 +919,7 @@ static void profile_query_cb(const struct aa_profile *profile, static ssize_t query_data(char *buf, size_t buf_len, char *query, size_t query_len) { - char *out; + char *out, *keydup; const char *key; struct label_it i; struct aa_label *label, *curr; @@ -941,11 +941,18 @@ static ssize_t query_data(char *buf, size_t buf_len, if (buf_len < sizeof(bytes) + sizeof(blocks)) return -EINVAL; /* not enough space */ + /* buf overlaps query, keep a copy of the key for the later lookups */ + keydup = kstrdup(key, GFP_KERNEL); + if (!keydup) + return -ENOMEM; + curr = begin_current_label_crit_section(&needput); label = aa_label_parse(curr, query, GFP_KERNEL, false, false); end_current_label_crit_section(curr, needput); - if (IS_ERR(label)) + if (IS_ERR(label)) { + kfree(keydup); return PTR_ERR(label); + } /* We are going to leave space for two numbers. The first is the total * number of bytes we are writing after the first number. This is so @@ -963,13 +970,14 @@ static ssize_t query_data(char *buf, size_t buf_len, if (!profile->data) continue; - data = rhashtable_lookup_fast(profile->data, &key, + data = rhashtable_lookup_fast(profile->data, &keydup, profile->data->p); if (data) { if (out + sizeof(outle32) + data->size > buf + buf_len) { aa_put_label(label); + kfree(keydup); return -EINVAL; /* not enough space */ } outle32 = __cpu_to_le32(data->size); @@ -981,6 +989,7 @@ static ssize_t query_data(char *buf, size_t buf_len, } } aa_put_label(label); + kfree(keydup); outle32 = __cpu_to_le32(out - buf - sizeof(bytes)); memcpy(buf, &outle32, sizeof(outle32)); diff --git a/security/apparmor/file.c b/security/apparmor/file.c index 3e74b613db32..baca370ddfc0 100644 --- a/security/apparmor/file.c +++ b/security/apparmor/file.c @@ -706,49 +706,50 @@ static void revalidate_tty(const struct cred *subj_cred, struct aa_label *label) no_tty(); } -struct cred_label { - const struct cred *cred; - struct aa_label *label; -}; - -static int match_file(const void *p, struct file *file, unsigned int fd) +/* + * iterate_fd() callback, called with files->file_lock held: grab a ref + * on the next file so it can be revalidated without the lock held. + */ +static int get_next_file(const void *p, struct file *file, unsigned int fd) { - struct cred_label *cl = (struct cred_label *)p; + *(struct file **)p = get_file(file); - if (aa_file_perm(OP_INHERIT, cl->cred, cl->label, file, - aa_map_file_to_perms(file), IN_ATOMIC)) - return fd + 1; - return 0; + return fd + 1; } - /* based on selinux's flush_unauthorized_files */ void aa_inherit_files(const struct cred *cred, struct files_struct *files) { struct aa_label *label = aa_get_newest_cred_label(cred); - struct cred_label cl = { - .cred = cred, - .label = label, - }; - struct file *devnull = NULL; - unsigned int n; + struct file *devnull = NULL, *file = NULL; + bool devnull_tried = false; + unsigned int n = 0; revalidate_tty(cred, label); - /* Revalidate access to inherited open files. */ - n = iterate_fd(files, 0, match_file, &cl); - if (!n) /* none found? */ - goto out; - - devnull = dentry_open(&aa_null, O_RDWR, cred); - if (IS_ERR(devnull)) - devnull = NULL; - /* replace all the matching ones with this */ - do { + /* + * Revalidate access to inherited open files. Each file is + * revalidated holding only a reference to it, with no locks + * held, as revalidation may sleep (e.g. unix socket mediation + * dropping path refs, or allocating path buffers). + */ + while ((n = iterate_fd(files, n, get_next_file, &file)) != 0) { + int error = aa_file_perm(OP_INHERIT, cred, label, file, + aa_map_file_to_perms(file), false); + + fput(file); + if (!error) + continue; + if (!devnull_tried) { + devnull_tried = true; + devnull = dentry_open(&aa_null, O_RDWR, cred); + if (IS_ERR(devnull)) + devnull = NULL; + } + /* replace the denied fd (n - 1) with devnull, or close it */ replace_fd(n - 1, devnull, 0); - } while ((n = iterate_fd(files, n, match_file, &cl)) != 0); + } if (devnull) fput(devnull); -out: aa_put_label(label); } diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h index 2c869b7dec97..61794b9cd6be 100644 --- a/security/apparmor/include/label.h +++ b/security/apparmor/include/label.h @@ -162,7 +162,11 @@ do { \ #define labels_ns(X) (vec_ns(&((X)->vec[0]), (X)->size)) #define labels_set(X) (&labels_ns(X)->labels) #define labels_view(X) labels_ns(X) -#define labels_profile(X) ((X)->vec[(X)->size - 1]) + +static inline struct aa_profile *labels_profile(const struct aa_label *label) +{ + return label->vec[label->size - 1]; +} int aa_label_next_confined(const struct aa_label *l, int i); diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h index f7bd7855f1bd..66485f76d331 100644 --- a/security/apparmor/include/match.h +++ b/security/apparmor/include/match.h @@ -12,6 +12,7 @@ #define __AA_MATCH_H #include <linux/kref.h> +#include <linux/overflow.h> #define DFA_NOMATCH 0 #define DFA_START 1 @@ -120,7 +121,15 @@ struct aa_dfa { static inline size_t table_size(size_t len, size_t el_size) { - return ALIGN(sizeof(struct table_header) + len * el_size, 8); + size_t size; + + /* Need check_*_overflow for 32 bit machines */ + if (check_mul_overflow(len, el_size, &size) || + check_add_overflow(size, sizeof(struct table_header), &size) || + size > SIZE_MAX - 7) + return 0; + + return ALIGN(size, 8); } #define aa_state_t unsigned int diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index 0dcbf8cf1029..bb851654cb80 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -436,6 +436,26 @@ static inline void aa_put_profile(struct aa_profile *p) kref_put(&p->label.count.count, aa_label_kref); } +/** + * aa_put_unpublished_profile - put a profile that was never made visible + * @p: profile (MAYBE NULL) + * + * A newly allocated profile and its proxy reference each other + * (proxy->label holds a label refcount). That cycle is normally broken + * when the profile is replaced or removed, which redirects the proxy. + * A profile that is discarded before being added to the policy tree + * never goes through either path, so break the cycle here, otherwise + * the profile, its proxy and its rawdata are leaked. + */ +static inline void aa_put_unpublished_profile(struct aa_profile *p) +{ + if (p && p->label.proxy) { + aa_put_proxy(p->label.proxy); + p->label.proxy = NULL; + } + aa_put_profile(p); +} + static inline int AUDIT_MODE(const struct aa_profile *profile) { if (aa_g_audit != AUDIT_NORMAL) diff --git a/security/apparmor/include/policy_ns.h b/security/apparmor/include/policy_ns.h index cc6e84151812..4f8359875476 100644 --- a/security/apparmor/include/policy_ns.h +++ b/security/apparmor/include/policy_ns.h @@ -118,6 +118,22 @@ static inline struct aa_ns *aa_get_ns(struct aa_ns *ns) } /** + * aa_get_ns_not0 - increment references count on @ns found via lookup + * @ns: namespace to increment reference count of (MAYBE NULL) + * + * Returns: pointer to @ns, if @ns is NULL returns NULL, + * NULL if @ns is being freed + * Requires: @ns must be held with valid refcount when called + */ +static inline struct aa_ns *aa_get_ns_not0(struct aa_ns *ns) +{ + if (ns && aa_get_profile_not0(ns->unconfined)) + return ns; + + return NULL; +} + +/** * aa_put_ns - decrement refcount on @ns * @ns: namespace to put reference of * diff --git a/security/apparmor/label.c b/security/apparmor/label.c index 82742a471055..fd67262330f9 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -387,7 +387,6 @@ void aa_label_kref(struct kref *kref) label_free_switch(label); return; } - /* TODO: update labels_profile macro so it works here */ AA_BUG(label_isprofile(label) && on_list_rcu(&label->vec[0]->base.profiles)); AA_BUG(label_isprofile(label) && @@ -1955,12 +1954,18 @@ struct aa_label *aa_label_strn_parse(struct aa_label *base, const char *str, goto out; } - if (create) + if (create) { label = aa_vec_find_or_create_label(vec, len, gfp); - else + if (!label) { + /* not absence: creating only fails on allocation */ + label = ERR_PTR(-ENOMEM); + goto out; + } + } else { label = vec_find(vec, len); - if (!label) - goto fail; + if (!label) + goto fail; + } out: /* use adjusted len from after vec_unique, not original */ diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index c73681d820a0..eaebc3c60c5a 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -15,6 +15,7 @@ #include <linux/mount.h> #include <linux/namei.h> #include <linux/ptrace.h> +#include <linux/sctp.h> #include <linux/ctype.h> #include <linux/sysctl.h> #include <linux/sysfs.h> @@ -1405,6 +1406,23 @@ static int apparmor_socket_connect(struct socket *sock, return aa_sk_perm(OP_CONNECT, AA_MAY_CONNECT, sock->sk); } +static int apparmor_sctp_bind_connect(struct sock *sk, int optname, + struct sockaddr *address, int addrlen) +{ + switch (optname) { + /* Bind checks */ + case SCTP_SET_PEER_PRIMARY_ADDR: + case SCTP_SOCKOPT_BINDX_ADD: + return aa_sk_perm(OP_BIND, AA_MAY_BIND, sk); + /* Connect checks */ + case SCTP_SOCKOPT_CONNECTX: + case SCTP_SENDMSG_CONNECT: + return aa_sk_perm(OP_CONNECT, AA_MAY_CONNECT, sk); + } + + return 0; +} + static int apparmor_socket_listen(struct socket *sock, int backlog) { AA_BUG(!sock); @@ -1823,6 +1841,7 @@ static struct security_hook_list apparmor_hooks[] __ro_after_init = { LSM_HOOK_INIT(socket_getsockopt, apparmor_socket_getsockopt), LSM_HOOK_INIT(socket_setsockopt, apparmor_socket_setsockopt), LSM_HOOK_INIT(socket_shutdown, apparmor_socket_shutdown), + LSM_HOOK_INIT(sctp_bind_connect, apparmor_sctp_bind_connect), #ifdef CONFIG_NETWORK_SECMARK LSM_HOOK_INIT(socket_sock_rcv_skb, apparmor_socket_sock_rcv_skb), #endif diff --git a/security/apparmor/match.c b/security/apparmor/match.c index 7713484f6a36..cf3b35a6b1cd 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -58,7 +58,7 @@ static struct table_header *unpack_table(const char *blob, size_t bsize) if (th.td_lolen == 0) goto out; tsize = table_size(th.td_lolen, th.td_flags); - if (bsize < tsize) + if (!tsize || bsize < tsize) goto out; table = kvzalloc(tsize, GFP_KERNEL); @@ -281,6 +281,10 @@ static struct table_header *remap_data16_to_data32(struct table_header *old) u32 i; tsize = table_size(old->td_lolen, YYTD_DATA32); + if (!tsize) { + kvfree(old); + return NULL; + } new = kvzalloc(tsize, GFP_KERNEL); if (!new) { kvfree(old); @@ -397,7 +401,6 @@ struct aa_dfa *aa_dfa_unpack(const void *blob, size_t size, int flags) /* check for duplicate table entry */ if (dfa->tables[table->td_id]) goto fail; - dfa->tables[table->td_id] = table; data += table_size(table->td_lolen, table->td_flags); size -= table_size(table->td_lolen, table->td_flags); @@ -414,9 +417,9 @@ struct aa_dfa *aa_dfa_unpack(const void *blob, size_t size, int flags) if (!table) goto fail; } - dfa->tables[table->td_id] = table; break; } + dfa->tables[table->td_id] = table; table = NULL; } error = verify_table_headers(dfa->tables, flags); @@ -614,7 +617,7 @@ aa_state_t aa_dfa_match_until(const struct aa_dfa *dfa, aa_state_t start, u32 *next = NEXT_TABLE(dfa); u32 *check = CHECK_TABLE(dfa); u32 *accept = ACCEPT_TABLE(dfa); - aa_state_t state = start, pos; + aa_state_t state = start; if (state == DFA_NOMATCH) return DFA_NOMATCH; @@ -625,22 +628,18 @@ aa_state_t aa_dfa_match_until(const struct aa_dfa *dfa, aa_state_t start, u8 *equiv = EQUIV_TABLE(dfa); /* default is direct to next state */ while (*str) { - pos = base_idx(base[state]) + equiv[(u8) *str++]; - if (check[pos] == state) - state = next[pos]; - else - state = def[state]; + u8 c = equiv[(u8) *str++]; + + match_char(state, def, base, next, check, c); if (accept[state]) break; } } else { /* default is direct to next state */ while (*str) { - pos = base_idx(base[state]) + (u8) *str++; - if (check[pos] == state) - state = next[pos]; - else - state = def[state]; + u8 c = (u8) *str++; + + match_char(state, def, base, next, check, c); if (accept[state]) break; } @@ -675,7 +674,7 @@ aa_state_t aa_dfa_matchn_until(const struct aa_dfa *dfa, aa_state_t start, u32 *next = NEXT_TABLE(dfa); u32 *check = CHECK_TABLE(dfa); u32 *accept = ACCEPT_TABLE(dfa); - aa_state_t state = start, pos; + aa_state_t state = start; *retpos = NULL; if (state == DFA_NOMATCH) @@ -687,22 +686,18 @@ aa_state_t aa_dfa_matchn_until(const struct aa_dfa *dfa, aa_state_t start, u8 *equiv = EQUIV_TABLE(dfa); /* default is direct to next state */ for (; n; n--) { - pos = base_idx(base[state]) + equiv[(u8) *str++]; - if (check[pos] == state) - state = next[pos]; - else - state = def[state]; + u8 c = equiv[(u8) *str++]; + + match_char(state, def, base, next, check, c); if (accept[state]) break; } } else { /* default is direct to next state */ for (; n; n--) { - pos = base_idx(base[state]) + (u8) *str++; - if (check[pos] == state) - state = next[pos]; - else - state = def[state]; + u8 c = (u8) *str++; + + match_char(state, def, base, next, check, c); if (accept[state]) break; } @@ -751,7 +746,7 @@ static aa_state_t leftmatch_fb(const struct aa_dfa *dfa, aa_state_t start, u32 *base = BASE_TABLE(dfa); u32 *next = NEXT_TABLE(dfa); u32 *check = CHECK_TABLE(dfa); - aa_state_t state = start, pos; + aa_state_t state = start; AA_BUG(!dfa); AA_BUG(!str); @@ -769,13 +764,10 @@ static aa_state_t leftmatch_fb(const struct aa_dfa *dfa, aa_state_t start, /* default is direct to next state */ while (*str) { unsigned int adjust; + u8 c = equiv[(u8) *str++]; wb->history[wb->pos] = state; - pos = base_idx(base[state]) + equiv[(u8) *str++]; - if (check[pos] == state) - state = next[pos]; - else - state = def[state]; + match_char(state, def, base, next, check, c); if (is_loop(wb, state, &adjust)) { state = aa_dfa_match(dfa, state, str); *count -= adjust; @@ -788,13 +780,10 @@ static aa_state_t leftmatch_fb(const struct aa_dfa *dfa, aa_state_t start, /* default is direct to next state */ while (*str) { unsigned int adjust; + u8 c = (u8) *str++; wb->history[wb->pos] = state; - pos = base_idx(base[state]) + (u8) *str++; - if (check[pos] == state) - state = next[pos]; - else - state = def[state]; + match_char(state, def, base, next, check, c); if (is_loop(wb, state, &adjust)) { state = aa_dfa_match(dfa, state, str); *count -= adjust; diff --git a/security/apparmor/net.c b/security/apparmor/net.c index a333e6aff926..136d14226b97 100644 --- a/security/apparmor/net.c +++ b/security/apparmor/net.c @@ -24,6 +24,7 @@ struct aa_sfs_entry aa_sfs_entry_network[] = { AA_SFS_FILE_STRING("af_mask", AA_SFS_AF_MASK), AA_SFS_FILE_BOOLEAN("tcp-fast-open", 1), + AA_SFS_FILE_BOOLEAN("sctp", 1), { } }; @@ -260,7 +261,6 @@ int aa_profile_af_perm(struct aa_profile *profile, AA_BUG(family >= AF_MAX); AA_BUG(type < 0 || type >= SOCK_MAX); - AA_BUG(profile_unconfined(profile)); state = RULE_MEDIATES_NET(rules); if (state) { @@ -268,7 +268,7 @@ int aa_profile_af_perm(struct aa_profile *profile, type, protocol, &p, &ad->info); return aa_do_perms(profile, rules->policy, state, request, p, ad); - } /* else */ + } /* else - not mediated this covers the profile_unconfined() case */ return 0; } diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 397f9ff78aeb..63f7821b4dff 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -1191,6 +1191,7 @@ ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label, struct aa_loaddata *rawdata_ent; const char *op; ssize_t count, error; + bool rawdata_created = false; LIST_HEAD(lh); op = mask & AA_MAY_REPLACE_POLICY ? OP_PROF_REPL : OP_PROF_LOAD; @@ -1330,6 +1331,7 @@ ssize_t aa_replace_profiles(struct aa_ns *policy_ns, struct aa_label *label, ent = NULL; goto fail_lock; } + rawdata_created = true; } list_for_each_entry(ent, &lh, list) { if (!ent->old) { @@ -1425,6 +1427,20 @@ out: return udata_sz; fail_lock: + /* + * Remove fs entries created by this load. Nothing else references + * them yet, and leaving them behind would leak the new profiles + * (their dents pin the proxy) and make the next load at the same, + * unbumped ns revision fail with -EEXIST on its raw_data dir. + * ensure children (which are at the end of the list are removed + * before their parent + */ + list_for_each_entry_reverse(tmp, &lh, list) { + if (!tmp->old) + __aafs_profile_rmdir(tmp->new); + } + if (rawdata_created) + __aa_fs_remove_rawdata(udata); mutex_unlock(&ns->lock); /* audit cause of failure */ @@ -1446,6 +1462,9 @@ fail: } list_for_each_entry_safe(ent, tmp, &lh, list) { list_del_init(&ent->list); + /* new was never added to the policy tree */ + aa_put_unpublished_profile(ent->new); + ent->new = NULL; aa_load_ent_free(ent); } diff --git a/security/apparmor/policy_ns.c b/security/apparmor/policy_ns.c index 5a907a875d8f..7d5c768493e8 100644 --- a/security/apparmor/policy_ns.c +++ b/security/apparmor/policy_ns.c @@ -207,7 +207,7 @@ struct aa_ns *aa_lookupn_ns(struct aa_ns *view, const char *name, size_t n) struct aa_ns *ns = NULL; rcu_read_lock(); - ns = aa_get_ns(__aa_lookupn_ns(view, name, n)); + ns = aa_get_ns_not0(__aa_lookupn_ns(view, name, n)); rcu_read_unlock(); return ns; @@ -265,10 +265,12 @@ struct aa_ns *__aa_find_or_create_ns(struct aa_ns *parent, const char *name, /* try and find the specified ns */ /* released by caller */ ns = aa_get_ns(__aa_find_ns(&parent->sub_ns, name)); - if (!ns) + if (!ns) { ns = __aa_create_ns(parent, name, dir); - else + } else { + aa_put_ns(ns); ns = ERR_PTR(-EEXIST); + } /* return ref */ return ns; diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index f1fc48e72d0e..35dfa3b451f3 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -558,6 +558,9 @@ static int unpack_strs_table(struct aa_ext *e, const char *name, bool multi, /* aa_unpack_strdup verifies that the last character is * null termination byte. */ + /* publish before validating; the error path frees it */ + table[i].strs = str; + table[i].size = size2; c = process_strs_entry(str, size2, multi); if (c <= 0) { AA_DEBUG(DEBUG_UNPACK, "process_strs %d i %d pos %ld", @@ -570,9 +573,7 @@ static int unpack_strs_table(struct aa_ext *e, const char *name, bool multi, /* fail - all other cases with embedded \0 */ goto fail; } - table[i].strs = str; table[i].count = c; - table[i].size = size2; } if (!aa_unpack_nameX(e, AA_ARRAYEND, NULL)) goto fail; @@ -1052,6 +1053,11 @@ static int unpack_pdb(struct aa_ext *e, struct aa_policydb **policy, u16 tdflags = pdb->dfa->tables[YYTD_ID_ACCEPT]->td_flags; size_t tsize = table_size(noents, tdflags); + if (!tsize) { + *info = "invalid dfa flags table size"; + error = -EPROTO; + goto fail; + } pdb->dfa->tables[YYTD_ID_ACCEPT2] = kvzalloc(tsize, GFP_KERNEL); if (!pdb->dfa->tables[YYTD_ID_ACCEPT2]) { *info = "failed to alloc dfa flags table"; @@ -1489,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; } @@ -1807,13 +1822,15 @@ int aa_unpack(struct aa_loaddata *udata, struct list_head *lh, fail_profile: kfree(ns_name); - aa_put_profile(profile); + aa_put_unpublished_profile(profile); fail: if (compressed_data) kvfree(compressed_data); list_for_each_entry_safe(ent, tmp, lh, list) { list_del_init(&ent->list); + aa_put_unpublished_profile(ent->new); + ent->new = NULL; aa_load_ent_free(ent); } diff --git a/security/apparmor/procattr.c b/security/apparmor/procattr.c index c07b6e8fd9c9..cfc45db43aa0 100644 --- a/security/apparmor/procattr.c +++ b/security/apparmor/procattr.c @@ -74,7 +74,8 @@ int aa_getprocattr(struct aa_label *label, char **string, bool newline) * @args: string to parse (NOT NULL) * @token: stores returned parsed token value (NOT NULL) * - * Returns: start position of name after token else NULL on failure + * Returns: the name following the token, %NULL if the name is empty, or + * ERR_PTR(-EINVAL) if the input is malformed. */ static char *split_token_from_name(const char *op, char *args, u64 *token) { |
