Skip to content
Snippets Groups Projects
  1. Jun 13, 2024
    • GUO Zihua's avatar
      ima: Avoid blocking in RCU read-side critical section · 9a95c5bf
      GUO Zihua authored
      
      A panic happens in ima_match_policy:
      
      BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
      PGD 42f873067 P4D 0
      Oops: 0000 [#1] SMP NOPTI
      CPU: 5 PID: 1286325 Comm: kubeletmonit.sh
      Kdump: loaded Tainted: P
      Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
                     BIOS 0.0.0 02/06/2015
      RIP: 0010:ima_match_policy+0x84/0x450
      Code: 49 89 fc 41 89 cf 31 ed 89 44 24 14 eb 1c 44 39
            7b 18 74 26 41 83 ff 05 74 20 48 8b 1b 48 3b 1d
            f2 b9 f4 00 0f 84 9c 01 00 00 <44> 85 73 10 74 ea
            44 8b 6b 14 41 f6 c5 01 75 d4 41 f6 c5 02 74 0f
      RSP: 0018:ff71570009e07a80 EFLAGS: 00010207
      RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000200
      RDX: ffffffffad8dc7c0 RSI: 0000000024924925 RDI: ff3e27850dea2000
      RBP: 0000000000000000 R08: 0000000000000000 R09: ffffffffabfce739
      R10: ff3e27810cc42400 R11: 0000000000000000 R12: ff3e2781825ef970
      R13: 00000000ff3e2785 R14: 000000000000000c R15: 0000000000000001
      FS:  00007f5195b51740(0000)
      GS:ff3e278b12d40000(0000) knlGS:0000000000000000
      CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
      CR2: 0000000000000010 CR3: 0000000626d24002 CR4: 0000000000361ee0
      DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
      DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
      Call Trace:
       ima_get_action+0x22/0x30
       process_measurement+0xb0/0x830
       ? page_add_file_rmap+0x15/0x170
       ? alloc_set_pte+0x269/0x4c0
       ? prep_new_page+0x81/0x140
       ? simple_xattr_get+0x75/0xa0
       ? selinux_file_open+0x9d/0xf0
       ima_file_check+0x64/0x90
       path_openat+0x571/0x1720
       do_filp_open+0x9b/0x110
       ? page_counter_try_charge+0x57/0xc0
       ? files_cgroup_alloc_fd+0x38/0x60
       ? __alloc_fd+0xd4/0x250
       ? do_sys_open+0x1bd/0x250
       do_sys_open+0x1bd/0x250
       do_syscall_64+0x5d/0x1d0
       entry_SYSCALL_64_after_hwframe+0x65/0xca
      
      Commit c7423dbd ("ima: Handle -ESTALE returned by
      ima_filter_rule_match()") introduced call to ima_lsm_copy_rule within a
      RCU read-side critical section which contains kmalloc with GFP_KERNEL.
      This implies a possible sleep and violates limitations of RCU read-side
      critical sections on non-PREEMPT systems.
      
      Sleeping within RCU read-side critical section might cause
      synchronize_rcu() returning early and break RCU protection, allowing a
      UAF to happen.
      
      The root cause of this issue could be described as follows:
      |	Thread A	|	Thread B	|
      |			|ima_match_policy	|
      |			|  rcu_read_lock	|
      |ima_lsm_update_rule	|			|
      |  synchronize_rcu	|			|
      |			|    kmalloc(GFP_KERNEL)|
      |			|      sleep		|
      ==> synchronize_rcu returns early
      |  kfree(entry)		|			|
      |			|    entry = entry->next|
      ==> UAF happens and entry now becomes NULL (or could be anything).
      |			|    entry->action	|
      ==> Accessing entry might cause panic.
      
      To fix this issue, we are converting all kmalloc that is called within
      RCU read-side critical section to use GFP_ATOMIC.
      
      Fixes: c7423dbd ("ima: Handle -ESTALE returned by ima_filter_rule_match()")
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarGUO Zihua <guozihua@huawei.com>
      Acked-by: default avatarJohn Johansen <john.johansen@canonical.com>
      Reviewed-by: default avatarMimi Zohar <zohar@linux.ibm.com>
      Reviewed-by: default avatarCasey Schaufler <casey@schaufler-ca.com>
      [PM: fixed missing comment, long lines, !CONFIG_IMA_LSM_RULES case]
      Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
      9a95c5bf
  2. Jun 03, 2024
    • Paul Moore's avatar
      lsm: fixup the inode xattr capability handling · 61df7b82
      Paul Moore authored
      
      The current security_inode_setxattr() and security_inode_removexattr()
      hooks rely on individual LSMs to either call into the associated
      capability hooks (cap_inode_setxattr() or cap_inode_removexattr()), or
      return a magic value of 1 to indicate that the LSM layer itself should
      perform the capability checks.  Unfortunately, with the default return
      value for these LSM hooks being 0, an individual LSM hook returning a
      1 will cause the LSM hook processing to exit early, potentially
      skipping a LSM.  Thankfully, with the exception of the BPF LSM, none
      of the LSMs which currently register inode xattr hooks should end up
      returning a value of 1, and in the BPF LSM case, with the BPF LSM hooks
      executing last there should be no real harm in stopping processing of
      the LSM hooks.  However, the reliance on the individual LSMs to either
      call the capability hooks themselves, or signal the LSM with a return
      value of 1, is fragile and relies on a specific set of LSMs being
      enabled.  This patch is an effort to resolve, or minimize, these
      issues.
      
      Before we discuss the solution, there are a few observations and
      considerations that we need to take into account:
      * BPF LSM registers an implementation for every LSM hook, and that
        implementation simply returns the hook's default return value, a
        0 in this case.  We want to ensure that the default BPF LSM behavior
        results in the capability checks being called.
      * SELinux and Smack do not expect the traditional capability checks
        to be applied to the xattrs that they "own".
      * SELinux and Smack are currently written in such a way that the
        xattr capability checks happen before any additional LSM specific
        access control checks.  SELinux does apply SELinux specific access
        controls to all xattrs, even those not "owned" by SELinux.
      * IMA and EVM also register xattr hooks but assume that the LSM layer
        and specific LSMs have already authorized the basic xattr operation.
      
      In order to ensure we perform the capability based access controls
      before the individual LSM access controls, perform only one capability
      access control check for each operation, and clarify the logic around
      applying the capability controls, we need a mechanism to determine if
      any of the enabled LSMs "own" a particular xattr and want to take
      responsibility for controlling access to that xattr.  The solution in
      this patch is to create a new LSM hook, 'inode_xattr_skipcap', that is
      not exported to the rest of the kernel via a security_XXX() function,
      but is used by the LSM layer to determine if a LSM wants to control
      access to a given xattr and avoid the traditional capability controls.
      Registering an inode_xattr_skipcap hook is optional, if a LSM declines
      to register an implementation, or uses an implementation that simply
      returns the default value (0), there is no effect as the LSM continues
      to enforce the capability based controls (unless another LSM takes
      ownership of the xattr).  If none of the LSMs signal that the
      capability checks should be skipped, the capability check is performed
      and if access is granted the individual LSM xattr access control hooks
      are executed, keeping with the DAC-before-LSM convention.
      
      Cc: stable@vger.kernel.org
      Acked-by: default avatarCasey Schaufler <casey@schaufler-ca.com>
      Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
      61df7b82
  3. Apr 09, 2024
  4. Apr 03, 2024
  5. Mar 14, 2024
  6. Feb 22, 2024
  7. Feb 16, 2024
  8. Feb 14, 2024
    • Jann Horn's avatar
      lsm: fix integer overflow in lsm_set_self_attr() syscall · d8bdd795
      Jann Horn authored
      
      security_setselfattr() has an integer overflow bug that leads to
      out-of-bounds access when userspace provides bogus input:
      `lctx->ctx_len + sizeof(*lctx)` is checked against `lctx->len` (and,
      redundantly, also against `size`), but there are no checks on
      `lctx->ctx_len`.
      Therefore, userspace can provide an `lsm_ctx` with `->ctx_len` set to a
      value between `-sizeof(struct lsm_ctx)` and -1, and this bogus `->ctx_len`
      will then be passed to an LSM module as a buffer length, causing LSM
      modules to perform out-of-bounds accesses.
      
      The following reproducer will demonstrate this under ASAN (if AppArmor is
      loaded as an LSM):
      
      ```
      
      struct lsm_ctx {
        uint64_t id;
        uint64_t flags;
        uint64_t len;
        uint64_t ctx_len;
        char ctx[];
      };
      
      int main(void) {
        size_t size = sizeof(struct lsm_ctx);
        struct lsm_ctx *ctx = malloc(size);
        ctx->id = 104/*LSM_ID_APPARMOR*/;
        ctx->flags = 0;
        ctx->len = size;
        ctx->ctx_len = -sizeof(struct lsm_ctx);
        syscall(
          460/*__NR_lsm_set_self_attr*/,
          /*attr=*/  100/*LSM_ATTR_CURRENT*/,
          /*ctx=*/   ctx,
          /*size=*/  size,
          /*flags=*/ 0
        );
      }
      ```
      
      Fixes: a04a1198 ("LSM: syscalls for current process attributes")
      Signed-off-by: default avatarJann Horn <jannh@google.com>
      Acked-by: default avatarCasey Schaufler <casey@schaufler-ca.com>
      [PM: subj tweak, removed ref to ASAN splat that isn't included]
      Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
      d8bdd795
  9. Jan 30, 2024
  10. Jan 26, 2024
  11. Jan 25, 2024
    • Andrii Nakryiko's avatar
      bpf,lsm: Add BPF token LSM hooks · f568a3d4
      Andrii Nakryiko authored
      
      Wire up bpf_token_create and bpf_token_free LSM hooks, which allow to
      allocate LSM security blob (we add `void *security` field to struct
      bpf_token for that), but also control who can instantiate BPF token.
      This follows existing pattern for BPF map and BPF prog.
      
      Also add security_bpf_token_allow_cmd() and security_bpf_token_capable()
      LSM hooks that allow LSM implementation to control and negate (if
      necessary) BPF token's delegation of a specific bpf_cmd and capability,
      respectively.
      
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      Acked-by: default avatarPaul Moore <paul@paul-moore.com>
      Link: https://lore.kernel.org/bpf/20240124022127.2379740-12-andrii@kernel.org
      f568a3d4
    • Andrii Nakryiko's avatar
      bpf,lsm: Refactor bpf_map_alloc/bpf_map_free LSM hooks · a2431c7e
      Andrii Nakryiko authored
      
      Similarly to bpf_prog_alloc LSM hook, rename and extend bpf_map_alloc
      hook into bpf_map_create, taking not just struct bpf_map, but also
      bpf_attr and bpf_token, to give a fuller context to LSMs.
      
      Unlike bpf_prog_alloc, there is no need to move the hook around, as it
      currently is firing right before allocating BPF map ID and FD, which
      seems to be a sweet spot.
      
      But like bpf_prog_alloc/bpf_prog_free combo, make sure that bpf_map_free
      LSM hook is called even if bpf_map_create hook returned error, as if few
      LSMs are combined together it could be that one LSM successfully
      allocated security blob for its needs, while subsequent LSM rejected BPF
      map creation. The former LSM would still need to free up LSM blob, so we
      need to ensure security_bpf_map_free() is called regardless of the
      outcome.
      
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      Acked-by: default avatarPaul Moore <paul@paul-moore.com>
      Link: https://lore.kernel.org/bpf/20240124022127.2379740-11-andrii@kernel.org
      a2431c7e
    • Andrii Nakryiko's avatar
      bpf,lsm: Refactor bpf_prog_alloc/bpf_prog_free LSM hooks · 1b67772e
      Andrii Nakryiko authored
      Based on upstream discussion ([0]), rework existing
      bpf_prog_alloc_security LSM hook. Rename it to bpf_prog_load and instead
      of passing bpf_prog_aux, pass proper bpf_prog pointer for a full BPF
      program struct. Also, we pass bpf_attr union with all the user-provided
      arguments for BPF_PROG_LOAD command.  This will give LSMs as much
      information as we can basically provide.
      
      The hook is also BPF token-aware now, and optional bpf_token struct is
      passed as a third argument. bpf_prog_load LSM hook is called after
      a bunch of sanity checks were performed, bpf_prog and bpf_prog_aux were
      allocated and filled out, but right before performing full-fledged BPF
      verification step.
      
      bpf_prog_free LSM hook is now accepting struct bpf_prog argument, for
      consistency. SELinux code is adjusted to all new names, types, and
      signatures.
      
      Note, given that bpf_prog_load (previously bpf_prog_alloc) hook can be
      used by some LSMs to allocate extra security blob, but also by other
      LSMs to reject BPF program loading, we need to make sure that
      bpf_prog_free LSM hook is called after bpf_prog_load/bpf_prog_alloc one
      *even* if the hook itself returned error. If we don't do that, we run
      the risk of leaking memory. This seems to be possible today when
      combining SELinux and BPF LSM, as one example, depending on their
      relative ordering.
      
      Also, for BPF LSM setup, add bpf_prog_load and bpf_prog_free to
      sleepable LSM hooks list, as they are both executed in sleepable
      context. Also drop bpf_prog_load hook from untrusted, as there is no
      issue with refcount or anything else anymore, that originally forced us
      to add it to untrusted list in c0c852dd ("bpf: Do not mark certain LSM
      hook arguments as trusted"). We now trigger this hook much later and it
      should not be an issue anymore.
      
        [0] https://lore.kernel.org/bpf/9fe88aef7deabbe87d3fc38c4aea3c69.paul@paul-moore.com/
      
      
      
      Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
      Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
      Acked-by: default avatarPaul Moore <paul@paul-moore.com>
      Link: https://lore.kernel.org/bpf/20240124022127.2379740-10-andrii@kernel.org
      1b67772e
  12. Dec 24, 2023
    • Alfred Piccioni's avatar
      lsm: new security_file_ioctl_compat() hook · f1bb47a3
      Alfred Piccioni authored
      
      Some ioctl commands do not require ioctl permission, but are routed to
      other permissions such as FILE_GETATTR or FILE_SETATTR. This routing is
      done by comparing the ioctl cmd to a set of 64-bit flags (FS_IOC_*).
      
      However, if a 32-bit process is running on a 64-bit kernel, it emits
      32-bit flags (FS_IOC32_*) for certain ioctl operations. These flags are
      being checked erroneously, which leads to these ioctl operations being
      routed to the ioctl permission, rather than the correct file
      permissions.
      
      This was also noted in a RED-PEN finding from a while back -
      "/* RED-PEN how should LSM module know it's handling 32bit? */".
      
      This patch introduces a new hook, security_file_ioctl_compat(), that is
      called from the compat ioctl syscall. All current LSMs have been changed
      to support this hook.
      
      Reviewing the three places where we are currently using
      security_file_ioctl(), it appears that only SELinux needs a dedicated
      compat change; TOMOYO and SMACK appear to be functional without any
      change.
      
      Cc: stable@vger.kernel.org
      Fixes: 0b24dcb7 ("Revert "selinux: simplify ioctl checking"")
      Signed-off-by: default avatarAlfred Piccioni <alpic@google.com>
      Reviewed-by: default avatarStephen Smalley <stephen.smalley.work@gmail.com>
      [PM: subject tweak, line length fixes, and alignment corrections]
      Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
      f1bb47a3
  13. Dec 20, 2023
  14. Dec 19, 2023
  15. Dec 12, 2023
Loading