linux/security/apparmor, branch v5.8

Merge tag 'linux-kselftest-kunit-5.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest

2020-06-09T17:04:47Z

Pull Kunit updates from Shuah Khan: "This consists of: - Several config fragment fixes from Anders Roxell to improve test coverage. - Improvements to kunit run script to use defconfig as default and restructure the code for config/build/exec/parse from Vitor Massaru Iha and David Gow. - Miscellaneous documentation warn fix" * tag 'linux-kselftest-kunit-5.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest: security: apparmor: default KUNIT_* fragments to KUNIT_ALL_TESTS fs: ext4: default KUNIT_* fragments to KUNIT_ALL_TESTS drivers: base: default KUNIT_* fragments to KUNIT_ALL_TESTS lib: Kconfig.debug: default KUNIT_* fragments to KUNIT_ALL_TESTS kunit: default KUNIT_* fragments to KUNIT_ALL_TESTS kunit: Kconfig: enable a KUNIT_ALL_TESTS fragment kunit: Fix TabError, remove defconfig code and handle when there is no kunitconfig kunit: use KUnit defconfig by default kunit: use --build_dir=.kunit as default Documentation: test.h - fix warnings kunit: kunit_tool: Separate out config/build/exec/parse

Merge tag 'apparmor-pr-2020-06-07' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor

2020-06-07T23:04:49Z

Pull apparmor updates from John Johansen: "Features: - Replace zero-length array with flexible-array - add a valid state flags check - add consistency check between state and dfa diff encode flags - add apparmor subdir to proc attr interface - fail unpack if profile mode is unknown - add outofband transition and use it in xattr match - ensure that dfa state tables have entries Cleanups: - Use true and false for bool variable - Remove semicolon - Clean code by removing redundant instructions - Replace two seq_printf() calls by seq_puts() in aa_label_seq_xprint() - remove duplicate check of xattrs on profile attachment - remove useless aafs_create_symlink Bug fixes: - Fix memory leak of profile proxy - fix introspection of of task mode for unconfined tasks - fix nnp subset test for unconfined - check/put label on apparmor_sk_clone_security()" * tag 'apparmor-pr-2020-06-07' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor: apparmor: Fix memory leak of profile proxy apparmor: fix introspection of of task mode for unconfined tasks apparmor: check/put label on apparmor_sk_clone_security() apparmor: Use true and false for bool variable security/apparmor/label.c: Clean code by removing redundant instructions apparmor: Replace zero-length array with flexible-array apparmor: ensure that dfa state tables have entries apparmor: remove duplicate check of xattrs on profile attachment. apparmor: add outofband transition and use it in xattr match apparmor: fail unpack if profile mode is unknown apparmor: fix nnp subset test for unconfined apparmor: remove useless aafs_create_symlink apparmor: add proc subdir to attrs apparmor: add consistency check between state and dfa diff encode flags apparmor: add a valid state flags check AppArmor: Remove semicolon apparmor: Replace two seq_printf() calls by seq_puts() in aa_label_seq_xprint()

apparmor: Fix memory leak of profile proxy

2020-06-07T20:38:55Z

When the proxy isn't replaced and the profile is removed, the proxy is being leaked resulting in a kmemleak check message of unreferenced object 0xffff888077a3a490 (size 16): comm "apparmor_parser", pid 128041, jiffies 4322684109 (age 1097.028s) hex dump (first 16 bytes): 03 00 00 00 00 00 00 00 b0 92 fd 4b 81 88 ff ff ...........K.... backtrace: [<0000000084d5daf2>] aa_alloc_proxy+0x58/0xe0 [<00000000ecc0e21a>] aa_alloc_profile+0x159/0x1a0 [<000000004cc9ce15>] unpack_profile+0x275/0x1c40 [<000000007332b3ca>] aa_unpack+0x1e7/0x7e0 [<00000000e25e31bd>] aa_replace_profiles+0x18a/0x1d10 [<00000000350d9415>] policy_update+0x237/0x650 [<000000003fbf934e>] profile_load+0x122/0x160 [<0000000047f7b781>] vfs_write+0x139/0x290 [<000000008ad12358>] ksys_write+0xcd/0x170 [<000000001a9daa7b>] do_syscall_64+0x70/0x310 [<00000000b9efb0cf>] entry_SYSCALL_64_after_hwframe+0x49/0xb3 Make sure to cleanup the profile's embedded label which will result on the proxy being properly freed. Fixes: 637f688dc3dc ("apparmor: switch from profiles to using labels on contexts") Signed-off-by: John Johansen

apparmor: fix introspection of of task mode for unconfined tasks

2020-06-07T20:38:55Z

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

apparmor: check/put label on apparmor_sk_clone_security()

2020-06-07T20:38:56Z

Currently apparmor_sk_clone_security() does not check for existing label/peer in the 'new' struct sock; it just overwrites it, if any (with another reference to the label of the source sock.) static void apparmor_sk_clone_security(const struct sock *sk, struct sock *newsk) { struct aa_sk_ctx *ctx = SK_CTX(sk); struct aa_sk_ctx *new = SK_CTX(newsk); new->label = aa_get_label(ctx->label); new->peer = aa_get_label(ctx->peer); } This might leak label references, which might overflow under load. Thus, check for and put labels, to prevent such errors. Note this is similarly done on: static int apparmor_socket_post_create(struct socket *sock, ...) ... if (sock->sk) { struct aa_sk_ctx *ctx = SK_CTX(sock->sk); aa_put_label(ctx->label); ctx->label = aa_get_label(label); } ... Context: ------- The label reference count leak is observed if apparmor_sock_graft() is called previously: this sets the 'ctx->label' field by getting a reference to the current label (later overwritten, without put.) static void apparmor_sock_graft(struct sock *sk, ...) { struct aa_sk_ctx *ctx = SK_CTX(sk); if (!ctx->label) ctx->label = aa_get_current_label(); } And that is the case on crypto/af_alg.c:af_alg_accept(): int af_alg_accept(struct sock *sk, struct socket *newsock, ...) ... struct sock *sk2; ... sk2 = sk_alloc(...); ... security_sock_graft(sk2, newsock); security_sk_clone(sk, sk2); ... Apparently both calls are done on their own right, especially for other LSMs, being introduced in 2010/2014, before apparmor socket mediation in 2017 (see commits [1,2,3,4]). So, it looks OK there! Let's fix the reference leak in apparmor. Test-case: --------- Exercise that code path enough to overflow label reference count. $ cat aa-refcnt-af_alg.c #include #include #include #include #include int main() { int sockfd; struct sockaddr_alg sa; /* Setup the crypto API socket */ sockfd = socket(AF_ALG, SOCK_SEQPACKET, 0); if (sockfd < 0) { perror("socket"); return 1; } memset(&sa, 0, sizeof(sa)); sa.salg_family = AF_ALG; strcpy((char *) sa.salg_type, "rng"); strcpy((char *) sa.salg_name, "stdrng"); if (bind(sockfd, (struct sockaddr *) &sa, sizeof(sa)) < 0) { perror("bind"); return 1; } /* Accept a "connection" and close it; repeat. */ while (!close(accept(sockfd, NULL, 0))); return 0; } $ gcc -o aa-refcnt-af_alg aa-refcnt-af_alg.c $ ./aa-refcnt-af_alg [ 9928.475953] refcount_t overflow at apparmor_sk_clone_security+0x37/0x70 in aa-refcnt-af_alg[1322], uid/euid: 1000/1000 ... [ 9928.507443] RIP: 0010:apparmor_sk_clone_security+0x37/0x70 ... [ 9928.514286] security_sk_clone+0x33/0x50 [ 9928.514807] af_alg_accept+0x81/0x1c0 [af_alg] [ 9928.516091] alg_accept+0x15/0x20 [af_alg] [ 9928.516682] SYSC_accept4+0xff/0x210 [ 9928.519609] SyS_accept+0x10/0x20 [ 9928.520190] do_syscall_64+0x73/0x130 [ 9928.520808] entry_SYSCALL_64_after_hwframe+0x3d/0xa2 Note that other messages may be seen, not just overflow, depending on the value being incremented by kref_get(); on another run: [ 7273.182666] refcount_t: saturated; leaking memory. ... [ 7273.185789] refcount_t: underflow; use-after-free. Kprobes: ------- Using kprobe events to monitor sk -> sk_security -> label -> count (kref): Original v5.7 (one reference leak every iteration) ... (af_alg_accept+0x0/0x1c0) label=0xffff8a0f36c25eb0 label_refcnt=0x11fd2 ... (af_alg_release_parent+0x0/0xd0) label=0xffff8a0f36c25eb0 label_refcnt=0x11fd4 ... (af_alg_accept+0x0/0x1c0) label=0xffff8a0f36c25eb0 label_refcnt=0x11fd3 ... (af_alg_release_parent+0x0/0xd0) label=0xffff8a0f36c25eb0 label_refcnt=0x11fd5 ... (af_alg_accept+0x0/0x1c0) label=0xffff8a0f36c25eb0 label_refcnt=0x11fd4 ... (af_alg_release_parent+0x0/0xd0) label=0xffff8a0f36c25eb0 label_refcnt=0x11fd6 Patched v5.7 (zero reference leak per iteration) ... (af_alg_accept+0x0/0x1c0) label=0xffff9ff376c25eb0 label_refcnt=0x593 ... (af_alg_release_parent+0x0/0xd0) label=0xffff9ff376c25eb0 label_refcnt=0x594 ... (af_alg_accept+0x0/0x1c0) label=0xffff9ff376c25eb0 label_refcnt=0x593 ... (af_alg_release_parent+0x0/0xd0) label=0xffff9ff376c25eb0 label_refcnt=0x594 ... (af_alg_accept+0x0/0x1c0) label=0xffff9ff376c25eb0 label_refcnt=0x593 ... (af_alg_release_parent+0x0/0xd0) label=0xffff9ff376c25eb0 label_refcnt=0x594 Commits: ------- [1] commit 507cad355fc9 ("crypto: af_alg - Make sure sk_security is initialized on accept()ed sockets") [2] commit 4c63f83c2c2e ("crypto: af_alg - properly label AF_ALG socket") [3] commit 2acce6aa9f65 ("Networking") a.k.a ("crypto: af_alg - Avoid sock_graft call warning) [4] commit 56974a6fcfef ("apparmor: add base infastructure for socket mediation") Fixes: 56974a6fcfef ("apparmor: add base infastructure for socket mediation") Reported-by: Brian Moyles Signed-off-by: Mauricio Faria de Oliveira Signed-off-by: John Johansen

Merge branch 'exec-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace

2020-06-04T21:07:08Z

Pull execve updates from Eric Biederman: "Last cycle for the Nth time I ran into bugs and quality of implementation issues related to exec that could not be easily be fixed because of the way exec is implemented. So I have been digging into exec and cleanup up what I can. I don't think I have exec sorted out enough to fix the issues I started with but I have made some headway this cycle with 4 sets of changes. - promised cleanups after introducing exec_update_mutex - trivial cleanups for exec - control flow simplifications - remove the recomputation of bprm->cred The net result is code that is a bit easier to understand and work with and a decrease in the number of lines of code (if you don't count the added tests)" * 'exec-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace: (24 commits) exec: Compute file based creds only once exec: Add a per bprm->file version of per_clear binfmt_elf_fdpic: fix execfd build regression selftests/exec: Add binfmt_script regression test exec: Remove recursion from search_binary_handler exec: Generic execfd support exec/binfmt_script: Don't modify bprm->buf and then return -ENOEXEC exec: Move the call of prepare_binprm into search_binary_handler exec: Allow load_misc_binary to call prepare_binprm unconditionally exec: Convert security_bprm_set_creds into security_bprm_repopulate_creds exec: Factor security_bprm_creds_for_exec out of security_bprm_set_creds exec: Teach prepare_exec_creds how exec treats uids & gids exec: Set the point of no return sooner exec: Move handling of the point of no return to the top level exec: Run sync_mm_rss before taking exec_update_mutex exec: Fix spelling of search_binary_handler in a comment exec: Move the comment from above de_thread to above unshare_sighand exec: Rename flush_old_exec begin_new_exec exec: Move most of setup_new_exec into flush_old_exec exec: In setup_new_exec cache current in the local variable me ...

security: apparmor: default KUNIT_* fragments to KUNIT_ALL_TESTS

2020-06-01T20:25:15Z

This makes it easier to enable all KUnit fragments. Adding 'if !KUNIT_ALL_TESTS' so individual tests can not be turned off. Therefore if KUNIT_ALL_TESTS is enabled that will hide the prompt in menuconfig. Reviewed-by: David Gow Signed-off-by: Anders Roxell Acked-by: John Johansen Signed-off-by: Shuah Khan

Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net

2020-05-24T20:47:27Z

The MSCC bug fix in 'net' had to be slightly adjusted because the register accesses are done slightly differently in net-next. Signed-off-by: David S. Miller

apparmor: Fix use-after-free in aa_audit_rule_init

2020-05-21T22:25:51Z

In the implementation of aa_audit_rule_init(), when aa_label_parse() fails the allocated memory for rule is released using aa_audit_rule_free(). But after this release, the return statement tries to access the label field of the rule which results in use-after-free. Before releasing the rule, copy errNo and return it after release. Fixes: 52e8c38001d8 ("apparmor: Fix memory leak of rule on error exit path") Signed-off-by: Navid Emamdoost Signed-off-by: John Johansen

apparmor: Fix aa_label refcnt leak in policy_update

2020-05-21T22:25:51Z

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