Forum | Documentation | Website | Blog

Skip to content
Snippets Groups Projects
  1. Dec 16, 2021
    • Scott Mayhew's avatar
      selinux: fix sleeping function called from invalid context · cc274ae7
      Scott Mayhew authored
      selinux_sb_mnt_opts_compat() is called via sget_fc() under the sb_lock
      spinlock, so it can't use GFP_KERNEL allocations:
      
      [  868.565200] BUG: sleeping function called from invalid context at
                     include/linux/sched/mm.h:230
      [  868.568246] in_atomic(): 1, irqs_disabled(): 0,
                     non_block: 0, pid: 4914, name: mount.nfs
      [  868.569626] preempt_count: 1, expected: 0
      [  868.570215] RCU nest depth: 0, expected: 0
      [  868.570809] Preemption disabled at:
      [  868.570810] [<0000000000000000>] 0x0
      [  868.571848] CPU: 1 PID: 4914 Comm: mount.nfs Kdump: loaded
                     Tainted: G        W         5.16.0-rc5.2585cf9d #1
      [  868.573273] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009),
                     BIOS 1.14.0-4.fc34 04/01/2014
      [  868.574478] Call Trace:
      [  868.574844]  <TASK>
      [  868.575156]  dump_stack_lvl+0x34/0x44
      [  868.575692]  __might_resched.cold+0xd6/0x10f
      [  868.576308]  slab_pre_alloc_hook.constprop.0+0x89/0xf0
      [  868.577046]  __kmalloc_track_caller+0x72/0x420
      [  868.577684]  ? security_context_to_sid_core+0x48/0x2b0
      [  868.578569]  kmemdup_nul+0x22/0x50
      [  868.579108]  security_context_to_sid_core+0x48/0x2b0
      [  868.579854]  ? _nfs4_proc_pathconf+0xff/0x110 [nfsv4]
      [  868.580742]  ? nfs_reconfigure+0x80/0x80 [nfs]
      [  868.581355]  security_context_str_to_sid+0x36/0x40
      [  868.581960]  selinux_sb_mnt_opts_compat+0xb5/0x1e0
      [  868.582550]  ? nfs_reconfigure+0x80/0x80 [nfs]
      [  868.583098]  security_sb_mnt_opts_compat+0x2a/0x40
      [  868.583676]  nfs_compare_super+0x113/0x220 [nfs]
      [  868.584249]  ? nfs_try_mount_request+0x210/0x210 [nfs]
      [  868.584879]  sget_fc+0xb5/0x2f0
      [  868.585267]  nfs_get_tree_common+0x91/0x4a0 [nfs]
      [  868.585834]  vfs_get_tree+0x25/0xb0
      [  868.586241]  fc_mount+0xe/0x30
      [  868.586605]  do_nfs4_mount+0x130/0x380 [nfsv4]
      [  868.587160]  nfs4_try_get_tree+0x47/0xb0 [nfsv4]
      [  868.587724]  vfs_get_tree+0x25/0xb0
      [  868.588193]  do_new_mount+0x176/0x310
      [  868.588782]  __x64_sys_mount+0x103/0x140
      [  868.589388]  do_syscall_64+0x3b/0x90
      [  868.589935]  entry_SYSCALL_64_after_hwframe+0x44/0xae
      [  868.590699] RIP: 0033:0x7f2b371c6c4e
      [  868.591239] Code: 48 8b 0d dd 71 0e 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e
                           0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 a5 00
                           00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d aa 71
                           0e 00 f7 d8 64 89 01 48
      [  868.593810] RSP: 002b:00007ffc83775d88 EFLAGS: 00000246
                     ORIG_RAX: 00000000000000a5
      [  868.594691] RAX: ffffffffffffffda RBX: 00007ffc83775f10 RCX: 00007f2b371c6c4e
      [  868.595504] RDX: 0000555d517247a0 RSI: 0000555d51724700 RDI: 0000555d51724540
      [  868.596317] RBP: 00007ffc83775f10 R08: 0000555d51726890 R09: 0000555d51726890
      [  868.597162] R10: 0000000000000000 R11: 0000000000000246 R12: 0000555d51726890
      [  868.598005] R13: 0000000000000003 R14: 0000555d517246e0 R15: 0000555d511ac925
      [  868.598826]  </TASK>
      
      Cc: stable@vger.kernel.org
      Fixes: 69c4a42d
      
       ("lsm,selinux: add new hook to compare new mount to an existing mount")
      Signed-off-by: default avatarScott Mayhew <smayhew@redhat.com>
      [PM: cleanup/line-wrap the backtrace]
      Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
      cc274ae7
  2. Nov 19, 2021
    • Ondrej Mosnacek's avatar
      selinux: fix NULL-pointer dereference when hashtab allocation fails · dc27f3c5
      Ondrej Mosnacek authored
      When the hash table slot array allocation fails in hashtab_init(),
      h->size is left initialized with a non-zero value, but the h->htable
      pointer is NULL. This may then cause a NULL pointer dereference, since
      the policydb code relies on the assumption that even after a failed
      hashtab_init(), hashtab_map() and hashtab_destroy() can be safely called
      on it. Yet, these detect an empty hashtab only by looking at the size.
      
      Fix this by making sure that hashtab_init() always leaves behind a valid
      empty hashtab when the allocation fails.
      
      Cc: stable@vger.kernel.org
      Fixes: 03414a49
      
       ("selinux: do not allocate hashtabs dynamically")
      Signed-off-by: default avatarOndrej Mosnacek <omosnace@redhat.com>
      Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
      dc27f3c5
  3. Nov 14, 2021
    • Paul Moore's avatar
      net,lsm,selinux: revert the security_sctp_assoc_established() hook · 1aa3b220
      Paul Moore authored
      This patch reverts two prior patches, e7310c94
      ("security: implement sctp_assoc_established hook in selinux") and
      7c2ef024
      
       ("security: add sctp_assoc_established hook"), which
      create the security_sctp_assoc_established() LSM hook and provide a
      SELinux implementation.  Unfortunately these two patches were merged
      without proper review (the Reviewed-by and Tested-by tags from
      Richard Haines were for previous revisions of these patches that
      were significantly different) and there are outstanding objections
      from the SELinux maintainers regarding these patches.
      
      Work is currently ongoing to correct the problems identified in the
      reverted patches, as well as others that have come up during review,
      but it is unclear at this point in time when that work will be ready
      for inclusion in the mainline kernel.  In the interest of not keeping
      objectionable code in the kernel for multiple weeks, and potentially
      a kernel release, we are reverting the two problematic patches.
      
      Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      1aa3b220
  4. Nov 12, 2021
    • Paul Moore's avatar
      net,lsm,selinux: revert the security_sctp_assoc_established() hook · 32a370ab
      Paul Moore authored
      This patch reverts two prior patches, e7310c94
      ("security: implement sctp_assoc_established hook in selinux") and
      7c2ef024 ("security: add sctp_assoc_established hook"), which
      create the security_sctp_assoc_established() LSM hook and provide a
      SELinux implementation.  Unfortunately these two patches were merged
      without proper review (the Reviewed-by and Tested-by tags from
      Richard Haines were for previous revisions of these patches that
      were significantly different) and there are outstanding objections
      from the SELinux maintainers regarding these patches.
      
      Work is currently ongoing to correct the problems identified in the
      reverted patches, as well as others that have come up during review,
      but it is unclear at this point in time when that work will be ready
      for inclusion in the mainline kernel.  In the interest of not keeping
      objectionable code in the kernel for multiple weeks, and potentially
      a kernel release, we are reverting th...
      32a370ab
  5. Nov 06, 2021
  6. Nov 03, 2021
  7. Nov 01, 2021
  8. Oct 28, 2021
  9. Oct 22, 2021
  10. Oct 21, 2021
    • Kees Cook's avatar
      gcc-plugins: Explicitly document purpose and deprecation schedule · 8bd51a2b
      Kees Cook authored
      
      GCC plugins should only exist when some compiler feature needs to be
      proven but does not exist in either GCC nor Clang. For example, if a
      desired feature is already in Clang, it should be added to GCC upstream.
      Document this explicitly.
      
      Additionally, mark the plugins with matching upstream GCC features as
      removable past their respective GCC versions.
      
      Cc: Masahiro Yamada <masahiroy@kernel.org>
      Cc: Michal Marek <michal.lkml@markovi.net>
      Cc: Nick Desaulniers <ndesaulniers@google.com>
      Cc: Jonathan Corbet <corbet@lwn.net>
      Cc: James Morris <jmorris@namei.org>
      Cc: "Serge E. Hallyn" <serge@hallyn.com>
      Cc: Nathan Chancellor <nathan@kernel.org>
      Cc: linux-hardening@vger.kernel.org
      Cc: linux-kbuild@vger.kernel.org
      Cc: linux-doc@vger.kernel.org
      Cc: linux-security-module@vger.kernel.org
      Cc: llvm@lists.linux.dev
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Reviewed-by: default avatarNathan Chancellor <nathan@kernel.org>
      Reviewed-by: default avatarMiguel Ojeda <ojeda@kernel.org...>
      8bd51a2b
  11. Oct 20, 2021
    • Eric W. Biederman's avatar
      ucounts: Move get_ucounts from cred_alloc_blank to key_change_session_keyring · 5ebcbe34
      Eric W. Biederman authored
      Setting cred->ucounts in cred_alloc_blank does not make sense.  The
      uid and user_ns are deliberately not set in cred_alloc_blank but
      instead the setting is delayed until key_change_session_keyring.
      
      So move dealing with ucounts into key_change_session_keyring as well.
      
      Unfortunately that movement of get_ucounts adds a new failure mode to
      key_change_session_keyring.  I do not see anything stopping the parent
      process from calling setuid and changing the relevant part of it's
      cred while keyctl_session_to_parent is running making it fundamentally
      necessary to call get_ucounts in key_change_session_keyring.  Which
      means that the new failure mode cannot be avoided.
      
      A failure of key_change_session_keyring results in a single threaded
      parent keeping it's existing credentials.  Which results in the parent
      process not being able to access the session keyring and whichever
      keys are in the new keyring.
      
      Further get_ucounts is only expected to fail if the number of bits in
      the refernece count for the structure is too few.
      
      Since the code has no other way to report the failure of get_ucounts
      and because such failures are not expected to be common add a WARN_ONCE
      to report this problem to userspace.
      
      Between the WARN_ONCE and the parent process not having access to
      the keys in the new session keyring I expect any failure of get_ucounts
      will be noticed and reported and we can find another way to handle this
      condition.  (Possibly by just making ucounts->count an atomic_long_t).
      
      Cc: stable@vger.kernel.org
      Fixes: 905ae01c ("Add a reference to ucounts for each cred")
      Link: https://lkml.kernel.org/r/7k0ias0uf.fsf_-_@disp2133
      
      
      Tested-by: default avatarYu Zhao <yuzhao@google.com>
      Reviewed-by: default avatarAlexey Gladkov <legion@kernel.org>
      Signed-off-by: default avatar"Eric W. Biederman" <ebiederm@xmission.com>
      5ebcbe34
    • Vivek Goyal's avatar
      security: Return xattr name from security_dentry_init_security() · 15bf3239
      Vivek Goyal authored
      
      Right now security_dentry_init_security() only supports single security
      label and is used by SELinux only. There are two users of this hook,
      namely ceph and nfs.
      
      NFS does not care about xattr name. Ceph hardcodes the xattr name to
      security.selinux (XATTR_NAME_SELINUX).
      
      I am making changes to fuse/virtiofs to send security label to virtiofsd
      and I need to send xattr name as well. I also hardcoded the name of
      xattr to security.selinux.
      
      Stephen Smalley suggested that it probably is a good idea to modify
      security_dentry_init_security() to also return name of xattr so that
      we can avoid this hardcoding in the callers.
      
      This patch adds a new parameter "const char **xattr_name" to
      security_dentry_init_security() and LSM puts the name of xattr
      too if caller asked for it (xattr_name != NULL).
      
      Signed-off-by: default avatarVivek Goyal <vgoyal@redhat.com>
      Reviewed-by: default avatarJeff Layton <jlayton@kernel.org>
      Reviewed-by: default avatarChristian Brauner <christian.brauner@ubuntu.com>
      Acked-by: default avatarJames Morris <jamorris@linux.microsoft.com>
      [PM: fixed typos in the commit description]
      Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
      15bf3239
  12. Oct 19, 2021
  13. Oct 14, 2021
    • Todd Kjos's avatar
      binder: use cred instead of task for selinux checks · 52f88693
      Todd Kjos authored
      Since binder was integrated with selinux, it has passed
      'struct task_struct' associated with the binder_proc
      to represent the source and target of transactions.
      The conversion of task to SID was then done in the hook
      implementations. It turns out that there are race conditions
      which can result in an incorrect security context being used.
      
      Fix by using the 'struct cred' saved during binder_open and pass
      it to the selinux subsystem.
      
      Cc: stable@vger.kernel.org # 5.14 (need backport for earlier stables)
      Fixes: 79af7307
      
       ("Add security hooks to binder and implement the hooks for SELinux.")
      Suggested-by: default avatarJann Horn <jannh@google.com>
      Signed-off-by: default avatarTodd Kjos <tkjos@google.com>
      Acked-by: default avatarCasey Schaufler <casey@schaufler-ca.com>
      Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
      52f88693
    • Kees Cook's avatar
      LSM: Avoid warnings about potentially unused hook variables · 86dd9fd5
      Kees Cook authored
      
      Building with W=1 shows many unused const variable warnings. These can
      be silenced, as we're well aware of their being potentially unused:
      
      ./include/linux/lsm_hook_defs.h:36:18: error: 'ptrace_access_check_default' defined but not used [-Werror=unused-const-variable=]
         36 | LSM_HOOK(int, 0, ptrace_access_check, struct task_struct *child,
            |                  ^~~~~~~~~~~~~~~~~~~
      security/security.c:706:32: note: in definition of macro 'LSM_RET_DEFAULT'
        706 | #define LSM_RET_DEFAULT(NAME) (NAME##_default)
            |                                ^~~~
      security/security.c:711:9: note: in expansion of macro 'DECLARE_LSM_RET_DEFAULT_int'
        711 |         DECLARE_LSM_RET_DEFAULT_##RET(DEFAULT, NAME)
            |         ^~~~~~~~~~~~~~~~~~~~~~~~
      ./include/linux/lsm_hook_defs.h:36:1: note: in expansion of macro 'LSM_HOOK'
         36 | LSM_HOOK(int, 0, ptrace_access_check, struct task_struct *child,
            | ^~~~~~~~
      
      Cc: James Morris <jmorris@namei.org>
      Cc: "Serge E. Hallyn" <serge@hallyn.com>
      Cc: Paul Moore <paul@paul-moore.com>
      Cc: Casey Schaufler <casey@schaufler-ca.com>
      Cc: KP Singh <kpsingh@chromium.org>
      Cc: linux-security-module@vger.kernel.org
      Reported-by: default avatarkernel test robot <lkp@intel.com>
      Link: https://lore.kernel.org/linux-mm/202110131608.zms53FPR-lkp@intel.com/
      Fixes: 98e828a0
      
       ("security: Refactor declaration of LSM hooks")
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Acked-by: default avatarJames Morris <jamorris@linux.microsoft.com>
      Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
      86dd9fd5
  14. Oct 13, 2021
  15. Oct 12, 2021
  16. Oct 11, 2021
    • Ondrej Mosnacek's avatar
      selinux: fix race condition when computing ocontext SIDs · cbfcd13b
      Ondrej Mosnacek authored
      Current code contains a lot of racy patterns when converting an
      ocontext's context structure to an SID. This is being done in a "lazy"
      fashion, such that the SID is looked up in the SID table only when it's
      first needed and then cached in the "sid" field of the ocontext
      structure. However, this is done without any locking or memory barriers
      and is thus unsafe.
      
      Between commits 24ed7fda ("selinux: use separate table for initial
      SID lookup") and 66f8e2f0 ("selinux: sidtab reverse lookup hash
      table"), this race condition lead to an actual observable bug, because a
      pointer to the shared sid field was passed directly to
      sidtab_context_to_sid(), which was using this location to also store an
      intermediate value, which could have been read by other threads and
      interpreted as an SID. In practice this caused e.g. new mounts to get a
      wrong (seemingly random) filesystem context, leading to strange denials.
      This bug has been spotted in the wild at least twice, see [1] and [2].
      
      Fix the race condition by making all the racy functions use a common
      helper that ensures the ocontext::sid accesses are made safely using the
      appropriate SMP constructs.
      
      Note that security_netif_sid() was populating the sid field of both
      contexts stored in the ocontext, but only the first one was actually
      used. The SELinux wiki's documentation on the "netifcon" policy
      statement [3] suggests that using only the first context is intentional.
      I kept only the handling of the first context here, as there is really
      no point in doing the SID lookup for the unused one.
      
      I wasn't able to reproduce the bug mentioned above on any kernel that
      includes commit 66f8e2f0, even though it has been reported that the
      issue occurs with that commit, too, just less frequently. Thus, I wasn't
      able to verify that this patch fixes the issue, but it makes sense to
      avoid the race condition regardless.
      
      [1] https://github.com/containers/container-selinux/issues/89
      [2] https://lists.fedoraproject.org/archives/list/selinux@lists.fedoraproject.org/thread/6DMTAMHIOAOEMUAVTULJD45JZU7IBAFM/
      [3] https://selinuxproject.org/page/NetworkStatements#netifcon
      
      
      
      Cc: stable@vger.kernel.org
      Cc: Xinjie Zheng <xinjie@google.com>
      Reported-by: default avatarSujithra Periasamy <sujithra@google.com>
      Fixes: 1da177e4
      
       ("Linux-2.6.12-rc2")
      Signed-off-by: default avatarOndrej Mosnacek <omosnace@redhat.com>
      Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
      cbfcd13b
    • Florian Westphal's avatar
      selinux: remove unneeded ipv6 hook wrappers · 4342f705
      Florian Westphal authored
      
      Netfilter places the protocol number the hook function is getting called
      from in state->pf, so we can use that instead of an extra wrapper.
      
      While at it, remove one-line wrappers too and make
      selinux_ip_{out,forward,postroute} useable as hook function.
      
      Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
      Message-Id: <20211011202229.28289-1-fw@strlen.de>
      Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
      4342f705
  17. Oct 09, 2021
  18. Sep 30, 2021
    • Paul Moore's avatar
      selinux: remove the SELinux lockdown implementation · f5d0e5e9
      Paul Moore authored
      NOTE: This patch intentionally omits any "Fixes:" metadata or stable
      tagging since it removes a SELinux access control check; while
      removing the control point is the right thing to do moving forward,
      removing it in stable kernels could be seen as a regression.
      
      The original SELinux lockdown implementation in 59438b46
      
      
      ("security,lockdown,selinux: implement SELinux lockdown") used the
      current task's credentials as both the subject and object in the
      SELinux lockdown hook, selinux_lockdown().  Unfortunately that
      proved to be incorrect in a number of cases as the core kernel was
      calling the LSM lockdown hook in places where the credentials from
      the "current" task_struct were not the correct credentials to use
      in the SELinux access check.
      
      Attempts were made to resolve this by adding a credential pointer
      to the LSM lockdown hook as well as suggesting that the single hook
      be split into two: one for user tasks, one for kernel tasks; however
      neither approach was deemed acceptable by Linus.  Faced with the
      prospect of either changing the subj/obj in the access check to a
      constant context (likely the kernel's label) or removing the SELinux
      lockdown check entirely, the SELinux community decided that removing
      the lockdown check was preferable.
      
      The supporting changes to the general LSM layer are left intact, this
      patch only removes the SELinux implementation.
      
      Acked-by: default avatarOndrej Mosnacek <omosnace@redhat.com>
      Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
      f5d0e5e9
  19. Sep 28, 2021