<feed xmlns='http://www.w3.org/2005/Atom'>
<title>BMC/Intel-BMC/linux.git/security/apparmor/lsm.c, branch dev-5.4</title>
<subtitle>Intel OpenBMC Linux kernel source tree (mirror)</subtitle>
<id>https://git.radix-linux.su/BMC/Intel-BMC/linux.git/atom?h=dev-5.4</id>
<link rel='self' href='https://git.radix-linux.su/BMC/Intel-BMC/linux.git/atom?h=dev-5.4'/>
<link rel='alternate' type='text/html' href='https://git.radix-linux.su/BMC/Intel-BMC/linux.git/'/>
<updated>2020-06-24T15:50:19+00:00</updated>
<entry>
<title>apparmor: check/put label on apparmor_sk_clone_security()</title>
<updated>2020-06-24T15:50:19+00:00</updated>
<author>
<name>Mauricio Faria de Oliveira</name>
<email>mfo@canonical.com</email>
</author>
<published>2020-06-02T21:15:16+00:00</published>
<link rel='alternate' type='text/html' href='https://git.radix-linux.su/BMC/Intel-BMC/linux.git/commit/?id=039a79ef0b1eb81ab060f29e14d3f9df0e6bc447'/>
<id>urn:sha1:039a79ef0b1eb81ab060f29e14d3f9df0e6bc447</id>
<content type='text'>
[ Upstream commit 3b646abc5bc6c0df649daea4c2c976bd4d47e4c8 ]

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-&gt;label = aa_get_label(ctx-&gt;label);
            new-&gt;peer = aa_get_label(ctx-&gt;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-&gt;sk) {
                    struct aa_sk_ctx *ctx = SK_CTX(sock-&gt;sk);

                    aa_put_label(ctx-&gt;label);
                    ctx-&gt;label = aa_get_label(label);
            }
    ...

Context:
-------

The label reference count leak is observed if apparmor_sock_graft()
is called previously: this sets the 'ctx-&gt;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-&gt;label)
                    ctx-&gt;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 &lt;stdio.h&gt;
    #include &lt;string.h&gt;
    #include &lt;unistd.h&gt;
    #include &lt;sys/socket.h&gt;
    #include &lt;linux/if_alg.h&gt;

    int main() {
            int sockfd;
            struct sockaddr_alg sa;

            /* Setup the crypto API socket */
            sockfd = socket(AF_ALG, SOCK_SEQPACKET, 0);
            if (sockfd &lt; 0) {
                    perror("socket");
                    return 1;
            }

            memset(&amp;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 *) &amp;sa, sizeof(sa)) &lt; 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
    &lt;a few hours later&gt;

    [ 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 -&gt; sk_security -&gt; label -&gt; 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 &lt;bmoyles@netflix.com&gt;
Signed-off-by: Mauricio Faria de Oliveira &lt;mfo@canonical.com&gt;
Signed-off-by: John Johansen &lt;john.johansen@canonical.com&gt;
Signed-off-by: Sasha Levin &lt;sashal@kernel.org&gt;
</content>
</entry>
<entry>
<title>treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 441</title>
<updated>2019-06-05T15:37:17+00:00</updated>
<author>
<name>Thomas Gleixner</name>
<email>tglx@linutronix.de</email>
</author>
<published>2019-06-01T08:08:55+00:00</published>
<link rel='alternate' type='text/html' href='https://git.radix-linux.su/BMC/Intel-BMC/linux.git/commit/?id=b886d83c5b621abc84ff9616f14c529be3f6b147'/>
<id>urn:sha1:b886d83c5b621abc84ff9616f14c529be3f6b147</id>
<content type='text'>
Based on 1 normalized pattern(s):

  this program is free software you can redistribute it and or modify
  it under the terms of the gnu general public license as published by
  the free software foundation version 2 of the license

extracted by the scancode license scanner the SPDX license identifier

  GPL-2.0-only

has been chosen to replace the boilerplate/reference in 315 file(s).

Signed-off-by: Thomas Gleixner &lt;tglx@linutronix.de&gt;
Reviewed-by: Allison Randal &lt;allison@lohutok.net&gt;
Reviewed-by: Armijn Hemel &lt;armijn@tjaldur.nl&gt;
Cc: linux-spdx@vger.kernel.org
Link: https://lkml.kernel.org/r/20190531190115.503150771@linutronix.de
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
</entry>
<entry>
<title>apparmor: Restore Y/N in /sys for apparmor's "enabled"</title>
<updated>2019-04-10T11:24:48+00:00</updated>
<author>
<name>Kees Cook</name>
<email>keescook@chromium.org</email>
</author>
<published>2019-04-08T16:07:06+00:00</published>
<link rel='alternate' type='text/html' href='https://git.radix-linux.su/BMC/Intel-BMC/linux.git/commit/?id=e33c1b9923775d17ad246946fe67fcb9be288677'/>
<id>urn:sha1:e33c1b9923775d17ad246946fe67fcb9be288677</id>
<content type='text'>
Before commit c5459b829b71 ("LSM: Plumb visibility into optional "enabled"
state"), /sys/module/apparmor/parameters/enabled would show "Y" or "N"
since it was using the "bool" handler. After being changed to "int",
this switched to "1" or "0", breaking the userspace AppArmor detection
of dbus-broker. This restores the Y/N output while keeping the LSM
infrastructure happy.

Before:
	$ cat /sys/module/apparmor/parameters/enabled
	1

After:
	$ cat /sys/module/apparmor/parameters/enabled
	Y

Reported-by: David Rheinsberg &lt;david.rheinsberg@gmail.com&gt;
Reviewed-by: David Rheinsberg &lt;david.rheinsberg@gmail.com&gt;
Link: https://lkml.kernel.org/r/CADyDSO6k8vYb1eryT4g6+EHrLCvb68GAbHVWuULkYjcZcYNhhw@mail.gmail.com
Fixes: c5459b829b71 ("LSM: Plumb visibility into optional "enabled" state")
Signed-off-by: Kees Cook &lt;keescook@chromium.org&gt;
Signed-off-by: John Johansen &lt;john.johansen@canonical.com&gt;
</content>
</entry>
<entry>
<title>Merge branch 'next-general' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security</title>
<updated>2019-03-07T19:44:01+00:00</updated>
<author>
<name>Linus Torvalds</name>
<email>torvalds@linux-foundation.org</email>
</author>
<published>2019-03-07T19:44:01+00:00</published>
<link rel='alternate' type='text/html' href='https://git.radix-linux.su/BMC/Intel-BMC/linux.git/commit/?id=ae5906ceee038ea29ff5162d1bcd18fb50af8b94'/>
<id>urn:sha1:ae5906ceee038ea29ff5162d1bcd18fb50af8b94</id>
<content type='text'>
Pull security subsystem updates from James Morris:

 - Extend LSM stacking to allow sharing of cred, file, ipc, inode, and
   task blobs. This paves the way for more full-featured LSMs to be
   merged, and is specifically aimed at LandLock and SARA LSMs. This
   work is from Casey and Kees.

 - There's a new LSM from Micah Morton: "SafeSetID gates the setid
   family of syscalls to restrict UID/GID transitions from a given
   UID/GID to only those approved by a system-wide whitelist." This
   feature is currently shipping in ChromeOS.

* 'next-general' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security: (62 commits)
  keys: fix missing __user in KEYCTL_PKEY_QUERY
  LSM: Update list of SECURITYFS users in Kconfig
  LSM: Ignore "security=" when "lsm=" is specified
  LSM: Update function documentation for cap_capable
  security: mark expected switch fall-throughs and add a missing break
  tomoyo: Bump version.
  LSM: fix return value check in safesetid_init_securityfs()
  LSM: SafeSetID: add selftest
  LSM: SafeSetID: remove unused include
  LSM: SafeSetID: 'depend' on CONFIG_SECURITY
  LSM: Add 'name' field for SafeSetID in DEFINE_LSM
  LSM: add SafeSetID module that gates setid calls
  LSM: add SafeSetID module that gates setid calls
  tomoyo: Allow multiple use_group lines.
  tomoyo: Coding style fix.
  tomoyo: Swicth from cred-&gt;security to task_struct-&gt;security.
  security: keys: annotate implicit fall throughs
  security: keys: annotate implicit fall throughs
  security: keys: annotate implicit fall through
  capabilities:: annotate implicit fall through
  ...
</content>
</entry>
<entry>
<title>apparmor: Fix warning about unused function apparmor_ipv6_postroute</title>
<updated>2019-02-01T12:12:57+00:00</updated>
<author>
<name>Petr Vorel</name>
<email>pvorel@suse.cz</email>
</author>
<published>2018-11-12T10:59:12+00:00</published>
<link rel='alternate' type='text/html' href='https://git.radix-linux.su/BMC/Intel-BMC/linux.git/commit/?id=a1a02062ad466052a34a8c4323143ccf9726eb52'/>
<id>urn:sha1:a1a02062ad466052a34a8c4323143ccf9726eb52</id>
<content type='text'>
when compiled without CONFIG_IPV6:
security/apparmor/lsm.c:1601:21: warning: ‘apparmor_ipv6_postroute’ defined but not used [-Wunused-function]
 static unsigned int apparmor_ipv6_postroute(void *priv,
                     ^~~~~~~~~~~~~~~~~~~~~~~

Reported-by: Jordan Glover &lt;Golden_Miller83@protonmail.ch&gt;
Tested-by: Jordan Glover &lt;Golden_Miller83@protonmail.ch&gt;
Signed-off-by: Petr Vorel &lt;pvorel@suse.cz&gt;
Signed-off-by: John Johansen &lt;john.johansen@canonical.com&gt;
</content>
</entry>
<entry>
<title>LSM: Make lsm_early_cred() and lsm_early_task() local functions.</title>
<updated>2019-01-18T19:44:02+00:00</updated>
<author>
<name>Tetsuo Handa</name>
<email>penguin-kernel@I-love.SAKURA.ne.jp</email>
</author>
<published>2019-01-18T10:15:59+00:00</published>
<link rel='alternate' type='text/html' href='https://git.radix-linux.su/BMC/Intel-BMC/linux.git/commit/?id=1cfb2a512e74e577bb0ed7c8d76df90a41a83f6a'/>
<id>urn:sha1:1cfb2a512e74e577bb0ed7c8d76df90a41a83f6a</id>
<content type='text'>
Since current-&gt;cred == current-&gt;real_cred when ordered_lsm_init()
is called, and lsm_early_cred()/lsm_early_task() need to be called
between the amount of required bytes is determined and module specific
initialization function is called, we can move these calls from
individual modules to ordered_lsm_init().

Signed-off-by: Tetsuo Handa &lt;penguin-kernel@I-love.SAKURA.ne.jp&gt;
Acked-by: Casey Schaufler &lt;casey@schaufler-ca.com&gt;
Signed-off-by: James Morris &lt;james.morris@microsoft.com&gt;
</content>
</entry>
<entry>
<title>LSM: generalize flag passing to security_capable</title>
<updated>2019-01-10T22:16:06+00:00</updated>
<author>
<name>Micah Morton</name>
<email>mortonm@chromium.org</email>
</author>
<published>2019-01-08T00:10:53+00:00</published>
<link rel='alternate' type='text/html' href='https://git.radix-linux.su/BMC/Intel-BMC/linux.git/commit/?id=c1a85a00ea66cb6f0bd0f14e47c28c2b0999799f'/>
<id>urn:sha1:c1a85a00ea66cb6f0bd0f14e47c28c2b0999799f</id>
<content type='text'>
This patch provides a general mechanism for passing flags to the
security_capable LSM hook. It replaces the specific 'audit' flag that is
used to tell security_capable whether it should log an audit message for
the given capability check. The reason for generalizing this flag
passing is so we can add an additional flag that signifies whether
security_capable is being called by a setid syscall (which is needed by
the proposed SafeSetID LSM).

Signed-off-by: Micah Morton &lt;mortonm@chromium.org&gt;
Reviewed-by: Kees Cook &lt;keescook@chromium.org&gt;
Signed-off-by: James Morris &lt;james.morris@microsoft.com&gt;
</content>
</entry>
<entry>
<title>LSM: Infrastructure management of the task security</title>
<updated>2019-01-08T21:18:45+00:00</updated>
<author>
<name>Casey Schaufler</name>
<email>casey@schaufler-ca.com</email>
</author>
<published>2018-09-22T00:19:37+00:00</published>
<link rel='alternate' type='text/html' href='https://git.radix-linux.su/BMC/Intel-BMC/linux.git/commit/?id=f4ad8f2c40769b3cc9497ba0883bbaf823f7752f'/>
<id>urn:sha1:f4ad8f2c40769b3cc9497ba0883bbaf823f7752f</id>
<content type='text'>
Move management of the task_struct-&gt;security blob out
of the individual security modules and into the security
infrastructure. Instead of allocating the blobs from within
the modules the modules tell the infrastructure how much
space is required, and the space is allocated there.
The only user of this blob is AppArmor. The AppArmor use
is abstracted to avoid future conflict.

Signed-off-by: Casey Schaufler &lt;casey@schaufler-ca.com&gt;
Reviewed-by: Kees Cook &lt;keescook@chromium.org&gt;
[kees: adjusted for ordered init series]
Signed-off-by: Kees Cook &lt;keescook@chromium.org&gt;
</content>
</entry>
<entry>
<title>LSM: Infrastructure management of the file security</title>
<updated>2019-01-08T21:18:44+00:00</updated>
<author>
<name>Casey Schaufler</name>
<email>casey@schaufler-ca.com</email>
</author>
<published>2018-11-12T20:02:49+00:00</published>
<link rel='alternate' type='text/html' href='https://git.radix-linux.su/BMC/Intel-BMC/linux.git/commit/?id=33bf60cabcc7687b194a689b068b65e9ecd556be'/>
<id>urn:sha1:33bf60cabcc7687b194a689b068b65e9ecd556be</id>
<content type='text'>
Move management of the file-&gt;f_security blob out of the
individual security modules and into the infrastructure.
The modules no longer allocate or free the data, instead
they tell the infrastructure how much space they require.

Signed-off-by: Casey Schaufler &lt;casey@schaufler-ca.com&gt;
Reviewed-by: Kees Cook &lt;keescook@chromium.org&gt;
[kees: adjusted for ordered init series]
Signed-off-by: Kees Cook &lt;keescook@chromium.org&gt;
</content>
</entry>
<entry>
<title>Infrastructure management of the cred security blob</title>
<updated>2019-01-08T21:18:44+00:00</updated>
<author>
<name>Casey Schaufler</name>
<email>casey@schaufler-ca.com</email>
</author>
<published>2018-11-12T17:30:56+00:00</published>
<link rel='alternate' type='text/html' href='https://git.radix-linux.su/BMC/Intel-BMC/linux.git/commit/?id=bbd3662a834813730912a58efb44dd6df6d952e6'/>
<id>urn:sha1:bbd3662a834813730912a58efb44dd6df6d952e6</id>
<content type='text'>
Move management of the cred security blob out of the
security modules and into the security infrastructre.
Instead of allocating and freeing space the security
modules tell the infrastructure how much space they
require.

Signed-off-by: Casey Schaufler &lt;casey@schaufler-ca.com&gt;
Reviewed-by: Kees Cook &lt;keescook@chromium.org&gt;
[kees: adjusted for ordered init series]
Signed-off-by: Kees Cook &lt;keescook@chromium.org&gt;
</content>
</entry>
</feed>
