kernel/linux.git/security/apparmor, branch v4.14.263

apparmor: fix error check

2021-11-26T10:40:34+00:00

[ Upstream commit d108370c644b153382632b3e5511ade575c91c86 ] clang static analysis reports this representative problem: label.c:1463:16: warning: Assigned value is garbage or undefined label->hname = name; ^ ~~~~ In aa_update_label_name(), this the problem block of code if (aa_label_acntsxprint(&name, ...) == -1) return res; On failure, aa_label_acntsxprint() has a more complicated return that just -1. So check for a negative return. It was also noted that the aa_label_acntsxprint() main comment refers to a nonexistent parameter, so clean up the comment. Fixes: f1bd904175e8 ("apparmor: add the base fns() for domain labels") Signed-off-by: Tom Rix Reviewed-by: Nick Desaulniers Signed-off-by: John Johansen Signed-off-by: Sasha Levin

apparmor: remove duplicate macro list_entry_is_head()

2021-09-26T11:37:28+00:00

commit 9801ca279ad37f72f71234fa81722afd95a3f997 upstream. Strangely I hadn't had noticed the existence of the list_entry_is_head() in apparmor code when added the same one in the list.h. Luckily it's fully identical and didn't break builds. In any case we don't need a duplicate anymore, thus remove it from apparmor code. Link: https://lkml.kernel.org/r/20201208100639.88182-1-andriy.shevchenko@linux.intel.com Fixes: e130816164e244 ("include/linux/list.h: add a macro to test if entry is pointing to the head") Signed-off-by: Andy Shevchenko Acked-by: John Johansen Cc: James Morris Cc: "Serge E . Hallyn " Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds Signed-off-by: Nobuhiro Iwamatsu (CIP) Signed-off-by: Greg Kroah-Hartman

apparmor: don't try to replace stale label in ptraceme check

2020-06-30T19:37:57+00:00

[ Upstream commit ca3fde5214e1d24f78269b337d3f22afd6bf445e ] begin_current_label_crit_section() must run in sleepable context because when label_is_stale() is true, aa_replace_current_label() runs, which uses prepare_creds(), which can sleep. Until now, the ptraceme access check (which runs with tasklist_lock held) violated this rule. Fixes: b2d09ae449ced ("apparmor: move ptrace checks to using labels") Reported-by: Cyrill Gorcunov Reported-by: kernel test robot Signed-off-by: Jann Horn Signed-off-by: John Johansen Signed-off-by: Sasha Levin

apparmor: fix introspection of of task mode for unconfined tasks

2020-06-25T13:41:50+00:00

[ Upstream commit dd2569fbb053719f7df7ef8fdbb45cf47156a701 ] Fix two issues with introspecting the task mode. 1. If a task is attached to a unconfined profile that is not the ns->unconfined profile then. Mode the mode is always reported as - $ ps -Z LABEL PID TTY TIME CMD unconfined 1287 pts/0 00:00:01 bash test (-) 1892 pts/0 00:00:00 ps instead of the correct value of (unconfined) as shown below $ ps -Z LABEL PID TTY TIME CMD unconfined 2483 pts/0 00:00:01 bash test (unconfined) 3591 pts/0 00:00:00 ps 2. if a task is confined by a stack of profiles that are unconfined the output of label mode is again the incorrect value of (-) like above, instead of (unconfined). This is because the visibile profile count increment is skipped by the special casing of unconfined. Fixes: f1bd904175e8 ("apparmor: add the base fns() for domain labels") Signed-off-by: John Johansen Signed-off-by: Sasha Levin

apparmor: Fix aa_label refcnt leak in policy_update

2020-05-27T14:43:07+00:00

commit c6b39f070722ea9963ffe756bfe94e89218c5e63 upstream. policy_update() invokes begin_current_label_crit_section(), which returns a reference of the updated aa_label object to "label" with increased refcount. When policy_update() returns, "label" becomes invalid, so the refcount should be decreased to keep refcount balanced. The reference counting issue happens in one exception handling path of policy_update(). When aa_may_manage_policy() returns not NULL, the refcnt increased by begin_current_label_crit_section() is not decreased, causing a refcnt leak. Fix this issue by jumping to "end_section" label when aa_may_manage_policy() returns not NULL. Fixes: 5ac8c355ae00 ("apparmor: allow introspecting the loaded policy pre internal transform") Signed-off-by: Xiyu Yang Signed-off-by: Xin Tan Signed-off-by: John Johansen Signed-off-by: Greg Kroah-Hartman

apparmor: don't try to replace stale label in ptrace access check

2020-01-27T13:46:04+00:00

[ Upstream commit 1f8266ff58840d698a1e96d2274189de1bdf7969 ] As a comment above begin_current_label_crit_section() explains, begin_current_label_crit_section() must run in sleepable context because when label_is_stale() is true, aa_replace_current_label() runs, which uses prepare_creds(), which can sleep. Until now, the ptrace access check (which runs with a task lock held) violated this rule. Also add a might_sleep() assertion to begin_current_label_crit_section(), because asserts are less likely to be ignored than comments. Fixes: b2d09ae449ced ("apparmor: move ptrace checks to using labels") Signed-off-by: Jann Horn Signed-off-by: John Johansen Signed-off-by: Sasha Levin

apparmor: fix unsigned len comparison with less than zero

2020-01-04T12:59:51+00:00

[ Upstream commit 00e0590dbaec6f1bcaa36a85467d7e3497ced522 ] The sanity check in macro update_for_len checks to see if len is less than zero, however, len is a size_t so it can never be less than zero, so this sanity check is a no-op. Fix this by making len a ssize_t so the comparison will work and add ulen that is a size_t copy of len so that the min() macro won't throw warnings about comparing different types. Addresses-Coverity: ("Macro compares unsigned to 0") Fixes: f1bd904175e8 ("apparmor: add the base fns() for domain labels") Signed-off-by: Colin Ian King Signed-off-by: John Johansen Signed-off-by: Sasha Levin

apparmor: delete the dentry in aafs_remove() to avoid a leak

2019-12-05T14:38:01+00:00

[ Upstream commit 201218e4d3dfa1346e30997f48725acce3f26d01 ] Although the apparmorfs dentries are always dropped from the dentry cache when the usage count drops to zero, there is no guarantee that this will happen in aafs_remove(), as another thread might still be using it. In this scenario, this means that the dentry will temporarily continue to appear in the results of lookups, even after the call to aafs_remove(). In the case of removal of a profile - it also causes simple_rmdir() on the profile directory to fail, as the directory won't be empty until the usage counts of all child dentries have decreased to zero. This results in the dentry for the profile directory leaking and appearing empty in the file system tree forever. Signed-off-by: Chris Coulson Signed-off-by: John Johansen Signed-off-by: Sasha Levin

apparmor: enforce nullbyte at end of tag string

2019-06-25T03:36:51+00:00

commit 8404d7a674c49278607d19726e0acc0cae299357 upstream. A packed AppArmor policy contains null-terminated tag strings that are read by unpack_nameX(). However, unpack_nameX() uses string functions on them without ensuring that they are actually null-terminated, potentially leading to out-of-bounds accesses. Make sure that the tag string is null-terminated before passing it to strcmp(). Cc: stable@vger.kernel.org Fixes: 736ec752d95e ("AppArmor: policy routines for loading and unpacking policy") Signed-off-by: Jann Horn Signed-off-by: John Johansen Signed-off-by: Greg Kroah-Hartman

apparmorfs: fix use-after-free on symlink traversal

2019-05-25T16:25:35+00:00

[ Upstream commit f51dcd0f621caac5380ce90fbbeafc32ce4517ae ] symlink body shouldn't be freed without an RCU delay. Switch apparmorfs to ->destroy_inode() and use of call_rcu(); free both the inode and symlink body in the callback. Signed-off-by: Al Viro Signed-off-by: Sasha Levin