- Mar 08, 2022
-
-
Eric Snowberg authored
With the introduction of uefi_check_trust_mok_keys, it signifies the end- user wants to trust the machine keyring as trusted keys. If they have chosen to trust the machine keyring, load the qualifying keys into it during boot, then link it to the secondary keyring . If the user has not chosen to trust the machine keyring, it will be empty and not linked to the secondary keyring. Signed-off-by:
Eric Snowberg <eric.snowberg@oracle.com> Reviewed-by:
Jarkko Sakkinen <jarkko@kernel.org> Signed-off-by:
Jarkko Sakkinen <jarkko@kernel.org>
-
Eric Snowberg authored
A new Machine Owner Key (MOK) variable called MokListTrustedRT has been introduced in shim. When this UEFI variable is set, it indicates the end-user has made the decision themselves that they wish to trust MOK keys within the Linux trust boundary. It is not an error if this variable does not exist. If it does not exist, the MOK keys should not be trusted within the kernel. Signed-off-by:
Eric Snowberg <eric.snowberg@oracle.com> Reviewed-by:
Jarkko Sakkinen <jarkko@kernel.org> Signed-off-by:
Jarkko Sakkinen <jarkko@kernel.org>
-
Eric Snowberg authored
Expose the .machine keyring created in integrity code by adding a reference. Store a reference to the machine keyring in system keyring code. The system keyring code needs this to complete the keyring link to the machine keyring. Signed-off-by:
Eric Snowberg <eric.snowberg@oracle.com> Reviewed-by:
Jarkko Sakkinen <jarkko@kernel.org> Tested-by:
Mimi Zohar <zohar@linux.ibm.com> Signed-off-by:
Jarkko Sakkinen <jarkko@kernel.org>
-
Eric Snowberg authored
Currently both Secure Boot DB and Machine Owner Keys (MOK) go through the same keyring handler (get_handler_for_db). With the addition of the new machine keyring, the end-user may choose to trust MOK keys. Introduce a new keyring handler specific for MOK keys. If MOK keys are trusted by the end-user, use the new keyring handler instead. Signed-off-by:
Eric Snowberg <eric.snowberg@oracle.com> Reviewed-by:
Mimi Zohar <zohar@linux.ibm.com> Reviewed-by:
Jarkko Sakkinen <jarkko@kernel.org> Tested-by:
Mimi Zohar <zohar@linux.ibm.com> Signed-off-by:
Jarkko Sakkinen <jarkko@kernel.org>
-
Eric Snowberg authored
Many UEFI Linux distributions boot using shim. The UEFI shim provides what is called Machine Owner Keys (MOK). Shim uses both the UEFI Secure Boot DB and MOK keys to validate the next step in the boot chain. The MOK facility can be used to import user generated keys. These keys can be used to sign an end-users development kernel build. When Linux boots, both UEFI Secure Boot DB and MOK keys get loaded in the Linux .platform keyring. Define a new Linux keyring called machine. This keyring shall contain just MOK keys and not the remaining keys in the platform keyring. This new machine keyring will be used in follow on patches. Unlike keys in the platform keyring, keys contained in the machine keyring will be trusted within the kernel if the end-user has chosen to do so. Signed-off-by:
Eric Snowberg <eric.snowberg@oracle.com> Tested-by:
Jarkko Sakkinen <jarkko@kernel.org> Tested-by:
Mimi Zohar <zohar@linux.ibm.com> Reviewed-by:
Jarkko Sakkinen <jarkko@kernel.org> Signed-off-by:
Jarkko Sakkinen <jarkko@kernel.org>
-
Eric Snowberg authored
make W=1 generates the following warning in keyring_handler.c security/integrity/platform_certs/keyring_handler.c:71:30: warning: no previous prototype for get_handler_for_db [-Wmissing-prototypes] __init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type) ^~~~~~~~~~~~~~~~~~ security/integrity/platform_certs/keyring_handler.c:82:30: warning: no previous prototype for get_handler_for_dbx [-Wmissing-prototypes] __init efi_element_handler_t get_handler_for_dbx(const efi_guid_t *sig_type) ^~~~~~~~~~~~~~~~~~~ Add the missing prototypes by including keyring_handler.h. Reported-by:
kernel test robot <lkp@intel.com> Signed-off-by:
Eric Snowberg <eric.snowberg@oracle.com> Reviewed-by:
Mimi Zohar <zohar@linux.ibm.com> Reviewed-by:
Jarkko Sakkinen <jarkko@kernel.org> Tested-by:
Mimi Zohar <zohar@linux.ibm.com> Signed-off-by:
Jarkko Sakkinen <jarkko@kernel.org>
-
Dave Kleikamp authored
If one loads and unloads the trusted module, trusted_key_exit can be NULL. Call it through static_call_cond() to avoid a kernel trap. Fixes: 5d0682be ("KEYS: trusted: Add generic trusted keys framework") Signed-off-by:
Dave Kleikamp <dave.kleikamp@oracle.com> Cc: Sumit Garg <sumit.garg@linaro.org> Cc: James Bottomley <jejb@linux.ibm.com> Cc: Jarkko Sakkinen <jarkko@kernel.org> Cc: Mimi Zohar <zohar@linux.ibm.com> Cc: David Howells <dhowells@redhat.com> Cc: James Morris <jmorris@namei.org> Cc: "Serge E. Hallyn" <serge@hallyn.com> Cc: linux-integrity@vger.kernel.org Cc: keyrings@vger.kernel.org Cc: linux-security-module@vger.kernel.org Reviewed-by:
Jarkko Sakkinen <jarkko@kernel.org> Signed-off-by:
Jarkko Sakkinen <jarkko@kernel.org>
-
Andreas Rammhold authored
Before this commit the kernel could end up with no trusted key sources even though both of the currently supported backends (TPM and TEE) were compiled as modules. This manifested in the trusted key type not being registered at all. When checking if a CONFIG_… preprocessor variable is defined we only test for the builtin (=y) case and not the module (=m) case. By using the IS_REACHABLE() macro we do test for both cases. Fixes: 5d0682be ("KEYS: trusted: Add generic trusted keys framework") Reviewed-by:
Jarkko Sakkinen <jarkko@kernel.org> Reviewed-by:
Ahmad Fatoum <a.fatoum@pengutronix.de> Reviewed-by:
Sumit Garg <sumit.garg@linaro.org> Signed-off-by:
Andreas Rammhold <andreas@rammhold.de> Tested-by:
Ahmad Fatoum <a.fatoum@pengutronix.de> Signed-off-by:
Ahmad Fatoum <a.fatoum@pengutronix.de> Signed-off-by:
Jarkko Sakkinen <jarkko@kernel.org>
-
Eric Biggers authored
In many cases, keyctl_pkey_params_get_2() is validating the user buffer lengths against the wrong algorithm properties. Fix it to check against the correct properties. Probably this wasn't noticed before because for all asymmetric keys of the "public_key" subtype, max_data_size == max_sig_size == max_enc_size == max_dec_size. However, this isn't necessarily true for the "asym_tpm" subtype (it should be, but it's not strictly validated). Of course, future key types could have different values as well. Fixes: 00d60fd3 ("KEYS: Provide keyctls to drive the new key type ops for asymmetric keys [ver #2]") Cc: <stable@vger.kernel.org> # v4.20+ Signed-off-by:
Eric Biggers <ebiggers@google.com> Reviewed-by:
Jarkko Sakkinen <jarkko@kernel.org> Signed-off-by:
Jarkko Sakkinen <jarkko@kernel.org>
-
- Mar 02, 2022
-
-
Nicolai Stange authored
struct dh contains several pointer members corresponding to DH parameters: ->key, ->p and ->g. A subsequent commit will introduce "dh" wrapping templates of the form "ffdhe2048(dh)", "ffdhe3072(dh)" and so on in order to provide built-in support for the well-known safe-prime ffdhe group parameters specified in RFC 7919. These templates will need to set the group parameter related members of the (serialized) struct dh instance passed to the inner "dh" kpp_alg instance, i.e. ->p and ->g, to some constant, static storage arrays. Turn the struct dh pointer members' types into "pointer to const" in preparation for this. Signed-off-by:
Nicolai Stange <nstange@suse.de> Reviewed-by:
Hannes Reinecke <hare@suse.de> Signed-off-by:
Herbert Xu <herbert@gondor.apana.org.au>
-
Paul Moore authored
The SELinux policy capability enum names are rather long and follow the "POLICYDB_CAPABILITY_XXX format". While the "POLICYDB_" prefix is helpful in tying the enums to other SELinux policy constants, macros, etc. there is no reason why we need to spell out "CAPABILITY" completely. Shorten "CAPABILITY" to "CAP" in order to make things a bit shorter and cleaner. Moving forward, the SELinux policy capability enum names should follow the "POLICYDB_CAP_XXX" format. Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
- Feb 28, 2022
-
-
Casey Schaufler authored
Remove inappropriate use of ntohs() and assign the port value directly. Reported-by:
kernel test robot <lkp@intel.com> Signed-off-by:
Casey Schaufler <casey@schaufler-ca.com>
-
- Feb 25, 2022
-
-
Richard Haines authored
These ioctls are equivalent to fcntl(fd, F_SETFD, flags), which SELinux always allows too. Furthermore, a failed FIOCLEX could result in a file descriptor being leaked to a process that should not have access to it. As this patch removes access controls, a policy capability needs to be enabled in policy to always allow these ioctls. Based-on-patch-by:
Demi Marie Obenour <demiobenour@gmail.com> Signed-off-by:
Richard Haines <richard_c_haines@btinternet.com> [PM: subject line tweak] Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
- Feb 22, 2022
-
-
Randy Dunlap authored
__setup() handlers should return 1 if the parameter is handled. Returning 0 causes the entire string to be added to init's environment strings (limited to 32 strings), unnecessarily polluting it. Using the documented string "evm=fix" causes an Unknown parameter message: Unknown kernel command line parameters "BOOT_IMAGE=/boot/bzImage-517rc5 evm=fix", will be passed to user space. and that string is added to init's environment string space: Run /sbin/init as init process with arguments: /sbin/init with environment: HOME=/ TERM=linux BOOT_IMAGE=/boot/bzImage-517rc5 evm=fix With this change, using "evm=fix" acts as expected and an invalid option ("evm=evm") causes a warning to be printed: evm: invalid "evm" mode but init's environment is not polluted with this string, as expected. Fixes: 7102ebcd ("evm: permit only valid security.evm xattrs to be updated") Signed-off-by:
Randy Dunlap <rdunlap@infradead.org> Reported-by:
Igor Zhbanov <i.zhbanov@omprussia.ru> Link: lore.kernel.org/r/64644a2f-4a20-bab3-1e15-3b2cdd0defe3@omprussia.ru Signed-off-by:
Mimi Zohar <zohar@linux.ibm.com>
-
Ondrej Mosnacek authored
mutex_is_locked() tests whether the mutex is locked *by any task*, while here we want to test if it is held *by the current task*. To avoid false/missed WARNINGs, use lockdep_assert_is_held() and lockdep_assert_is_not_held() instead, which do the right thing (though they are a no-op if CONFIG_LOCKDEP=n). Cc: stable@vger.kernel.org Fixes: 2554a48f ("selinux: measure state and policy capabilities") Signed-off-by:
Ondrej Mosnacek <omosnace@redhat.com> Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
- Feb 21, 2022
-
-
Yael Tzur authored
For availability and performance reasons master keys often need to be released outside of a Key Management Service (KMS) to clients. It would be beneficial to provide a mechanism where the wrapping/unwrapping of data encryption keys (DEKs) is not dependent on a remote call at runtime yet security is not (or only minimally) compromised. Master keys could be securely stored in the Kernel and be used to wrap/unwrap keys from Userspace. The encrypted.c class supports instantiation of encrypted keys with either an already-encrypted key material, or by generating new key material based on random numbers. This patch defines a new datablob format: [<format>] <master-key name> <decrypted data length> <decrypted data> that allows to inject and encrypt user-provided decrypted data. The decrypted data must be hex-ascii encoded. Signed-off-by:
Yael Tzur <yaelt@google.com> Reviewed-by:
Mimi Zohar <zohar@linux.ibm.com> Reviewed-by:
Sumit Garg <sumit.garg@linaro.org> Reviewed-by:
Jarkko Sakkinen <jarkko@kernel.org> Signed-off-by:
Mimi Zohar <zohar@linux.ibm.com>
-
- Feb 18, 2022
-
-
Christian Göttsche authored
security_sid_to_context() expects a pointer to an u32 as the address where to store the length of the computed context. Reported by sparse: security/selinux/xfrm.c:359:39: warning: incorrect type in arg 4 (different signedness) security/selinux/xfrm.c:359:39: expected unsigned int [usertype] *scontext_len security/selinux/xfrm.c:359:39: got int * Signed-off-by:
Christian Göttsche <cgzones@googlemail.com> [PM: wrapped commit description] Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
Christian Göttsche authored
Those return statements at the end of a void function are redundant. Reported by clang-tidy [readability-redundant-control-flow] Signed-off-by:
Christian Göttsche <cgzones@googlemail.com> Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
- Feb 15, 2022
-
-
Ondrej Mosnacek authored
Do this by extracting the peer labeling per-association logic from selinux_sctp_assoc_request() into a new helper selinux_sctp_process_new_assoc() and use this helper in both selinux_sctp_assoc_request() and selinux_sctp_assoc_established(). This ensures that the peer labeling behavior as documented in Documentation/security/SCTP.rst is applied both on the client and server side: """ An SCTP socket will only have one peer label assigned to it. This will be assigned during the establishment of the first association. Any further associations on this socket will have their packet peer label compared to the sockets peer label, and only if they are different will the ``association`` permission be validated. This is validated by checking the socket peer sid against the received packets peer sid to determine whether the association should be allowed or denied. """ At the same time, it also ensures that the peer label of the association is set to the correct value, such that if it is peeled off into a new socket, the socket's peer label will then be set to the association's peer label, same as it already works on the server side. While selinux_inet_conn_established() (which we are replacing by selinux_sctp_assoc_established() for SCTP) only deals with assigning a peer label to the connection (socket), in case of SCTP we need to also copy the (local) socket label to the association, so that selinux_sctp_sk_clone() can then pick it up for the new socket in case of SCTP peeloff. Careful readers will notice that the selinux_sctp_process_new_assoc() helper also includes the "IPv4 packet received over an IPv6 socket" check, even though it hadn't been in selinux_sctp_assoc_request() before. While such check is not necessary in selinux_inet_conn_request() (because struct request_sock's family field is already set according to the skb's family), here it is needed, as we don't have request_sock and we take the initial family from the socket. In selinux_sctp_assoc_established() it is similarly needed as well (and also selinux_inet_conn_established() already has it). Fixes: 72e89f50 ("security: Add support for SCTP security hooks") Reported-by:
Prashanth Prahlad <pprahlad@redhat.com> Based-on-patch-by:
Xin Long <lucien.xin@gmail.com> Reviewed-by:
Xin Long <lucien.xin@gmail.com> Tested-by:
Richard Haines <richard_c_haines@btinternet.com> Signed-off-by:
Ondrej Mosnacek <omosnace@redhat.com> Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
Ondrej Mosnacek authored
security_sctp_assoc_established() is added to replace security_inet_conn_established() called in sctp_sf_do_5_1E_ca(), so that asoc can be accessed in security subsystem and save the peer secid to asoc->peer_secid. Fixes: 72e89f50 ("security: Add support for SCTP security hooks") Reported-by:
Prashanth Prahlad <pprahlad@redhat.com> Based-on-patch-by:
Xin Long <lucien.xin@gmail.com> Reviewed-by:
Xin Long <lucien.xin@gmail.com> Tested-by:
Richard Haines <richard_c_haines@btinternet.com> Signed-off-by:
Ondrej Mosnacek <omosnace@redhat.com> Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
Mimi Zohar authored
To support larger hash digests in the 'iint' cache, instead of defining the 'digest' field as the maximum digest size, the 'digest' field was defined as a flexible array variable. The "ima_digest_data" struct was wrapped inside a local structure with the maximum digest size. But before adding the record to the iint cache, memory for the exact digest size was dynamically allocated. The original reason for defining the 'digest' field as a flexible array variable is still valid for the 'iint' cache use case. Instead of wrapping the 'ima_digest_data' struct in a local structure define 'ima_max_digest_data' struct. Reviewed-by:
Stefan Berger <stefanb@linux.ibm.com> Signed-off-by:
Mimi Zohar <zohar@linux.ibm.com>
-
Mimi Zohar authored
Simple policy rule options, such as fowner, uid, or euid, can be checked immediately, while other policy rule options, such as requiring a file signature, need to be deferred. The 'flags' field in the integrity_iint_cache struct contains the policy action', 'subaction', and non action/subaction. action: measure/measured, appraise/appraised, (collect)/collected, audit/audited subaction: appraise status for each hook (e.g. file, mmap, bprm, read, creds) non action/subaction: deferred policy rule options and state Rename the IMA_ACTION_FLAGS to IMA_NONACTION_FLAGS. Reviewed-by:
Stefan Berger <stefanb@linux.ibm.com> Signed-off-by:
Mimi Zohar <zohar@linux.ibm.com>
-
Stefan Berger authored
If an error occurs when creating a securityfs file, return the exact error code to the caller. Signed-off-by:
Stefan Berger <stefanb@linux.ibm.com> Signed-off-by:
Mimi Zohar <zohar@linux.ibm.com>
-
Austin Kim authored
There are a few minor typos in the comments. Fix these. Signed-off-by:
Austin Kim <austindh.kim@gmail.com> Signed-off-by:
Mimi Zohar <zohar@linux.ibm.com>
-
- Feb 06, 2022
-
-
Kees Cook authored
In order to compare instrumentation between builds, make the verbose mode of the plugin available during the build. This is rarely needed (behind EXPERT) and very noisy (disabled for COMPILE_TEST). Cc: Alexander Popov <alex.popov@linux.com> Signed-off-by:
Kees Cook <keescook@chromium.org>
-
- Feb 04, 2022
-
-
Ondrej Mosnacek authored
Commit b8b87fd9 ("selinux: Fix selinux_sb_mnt_opts_compat()") started to parse mount options into SIDs in selinux_add_opt() if policy has already been loaded. Since it's extremely unlikely that anyone would depend on the ability to set SELinux contexts on fs_context before loading the policy and then mounting that context after simplify the logic by always parsing the options early. Note that the multi-step mounting is only possible with the new fscontext mount API and wasn't possible before its introduction. Signed-off-by:
Ondrej Mosnacek <omosnace@redhat.com> Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
- Feb 02, 2022
-
-
Stefan Berger authored
Before printing a policy rule scan for inactive LSM labels in the policy rule. Inactive LSM labels are identified by args_p != NULL and rule == NULL. Fixes: 483ec26e ("ima: ima/lsm policy rule loading logic bug fixes") Signed-off-by:
Stefan Berger <stefanb@linux.ibm.com> Cc: <stable@vger.kernel.org> # v5.6+ Acked-by:
Christian Brauner <brauner@kernel.org> [zohar@linux.ibm.com: Updated "Fixes" tag] Signed-off-by:
Mimi Zohar <zohar@linux.ibm.com>
-
Roberto Sassu authored
Commit c2426d2a ("ima: added support for new kernel cmdline parameter ima_template_fmt") introduced an additional check on the ima_template variable to avoid multiple template selection. Unfortunately, ima_template could be also set by the setup function of the ima_hash= parameter, when it calls ima_template_desc_current(). This causes attempts to choose a new template with ima_template= or with ima_template_fmt=, after ima_hash=, to be ignored. Achieve the goal of the commit mentioned with the new static variable template_setup_done, so that template selection requests after ima_hash= are not ignored. Finally, call ima_init_template_list(), if not already done, to initialize the list of templates before lookup_template_desc() is called. Reported-by:
Guo Zihua <guozihua@huawei.com> Signed-off-by:
Roberto Sassu <roberto.sassu@huawei.com> Cc: stable@vger.kernel.org Fixes: c2426d2a ("ima: added support for new kernel cmdline parameter ima_template_fmt") Signed-off-by:
Mimi Zohar <zohar@linux.ibm.com>
-
Stefan Berger authored
The removal of ima_dir currently fails since ima_policy still exists, so remove the ima_policy file before removing the directory. Fixes: 4af4662f ("integrity: IMA policy") Signed-off-by:
Stefan Berger <stefanb@linux.ibm.com> Cc: <stable@vger.kernel.org> Acked-by:
Christian Brauner <brauner@kernel.org> Signed-off-by:
Mimi Zohar <zohar@linux.ibm.com>
-
Xiaoke Wang authored
audit_log_start() returns audit_buffer pointer on success or NULL on error, so it is better to check the return value of it. Fixes: 3323eec9 ("integrity: IMA as an integrity service provider") Signed-off-by:
Xiaoke Wang <xkernel.wang@foxmail.com> Cc: <stable@vger.kernel.org> Reviewed-by:
Paul Moore <paul@paul-moore.com> Signed-off-by:
Mimi Zohar <zohar@linux.ibm.com>
-
Vratislav Bendel authored
On error path from cond_read_list() and duplicate_policydb_cond_list() the cond_list_destroy() gets called a second time in caller functions, resulting in NULL pointer deref. Fix this by resetting the cond_list_len to 0 in cond_list_destroy(), making subsequent calls a noop. Also consistently reset the cond_list pointer to NULL after freeing. Cc: stable@vger.kernel.org Signed-off-by:
Vratislav Bendel <vbendel@redhat.com> [PM: fix line lengths in the description] Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
Christoph Hellwig authored
There is no good reason to keep genhd.h separate from the main blkdev.h header that includes it. So fold the contents of genhd.h into blkdev.h and remove genhd.h entirely. Signed-off-by:
Christoph Hellwig <hch@lst.de> Reviewed-by:
Chaitanya Kulkarni <kch@nvidia.com> Reviewed-by:
Martin K. Petersen <martin.petersen@oracle.com> Link: https://lore.kernel.org/r/20220124093913.742411-4-hch@lst.de Signed-off-by:
Jens Axboe <axboe@kernel.dk>
-
- Feb 01, 2022
-
-
Paul Moore authored
When running the SELinux code through sparse, there are a handful of warnings. This patch resolves some of these warnings caused by "__rcu" mismatches. % make W=1 C=1 security/selinux/ Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
Scott Mayhew authored
Avoid unnecessary parsing of sids that have already been parsed via selinux_sb_eat_lsm_opts(). Signed-off-by:
Scott Mayhew <smayhew@redhat.com> Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
Scott Mayhew authored
selinux_sb_mnt_opts_compat() is called under the sb_lock spinlock and shouldn't be performing any memory allocations. Fix this by parsing the sids at the same time we're chopping up the security mount options string and then using the pre-parsed sids when doing the comparison. Fixes: cc274ae7 ("selinux: fix sleeping function called from invalid context") Fixes: 69c4a42d ("lsm,selinux: add new hook to compare new mount to an existing mount") Signed-off-by:
Scott Mayhew <smayhew@redhat.com> Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
- Jan 28, 2022
-
-
Vivek Goyal authored
A ceph user has reported that ceph is crashing with kernel NULL pointer dereference. Following is the backtrace. /proc/version: Linux version 5.16.2-arch1-1 (linux@archlinux) (gcc (GCC) 11.1.0, GNU ld (GNU Binutils) 2.36.1) #1 SMP PREEMPT Thu, 20 Jan 2022 16:18:29 +0000 distro / arch: Arch Linux / x86_64 SELinux is not enabled ceph cluster version: 16.2.7 (dd0603118f56ab514f133c8d2e3adfc983942503) relevant dmesg output: [ 30.947129] BUG: kernel NULL pointer dereference, address: 0000000000000000 [ 30.947206] #PF: supervisor read access in kernel mode [ 30.947258] #PF: error_code(0x0000) - not-present page [ 30.947310] PGD 0 P4D 0 [ 30.947342] Oops: 0000 [#1] PREEMPT SMP PTI [ 30.947388] CPU: 5 PID: 778 Comm: touch Not tainted 5.16.2-arch1-1 #1 86fbf2c313cc37a553d65deb81d98e9dcc2a3659 [ 30.947486] Hardware name: Gigabyte Technology Co., Ltd. B365M DS3H/B365M DS3H, BIOS F5 08/13/2019 [ 30.947569] RIP: 0010:strlen+0x0/0x20 [ 30.947616] Code: b6 07 38 d0 74 16 48 83 c7 01 84 c0 74 05 48 39 f7 75 ec 31 c0 31 d2 89 d6 89 d7 c3 48 89 f8 31 d2 89 d6 89 d7 c3 0 f 1f 40 00 <80> 3f 00 74 12 48 89 f8 48 83 c0 01 80 38 00 75 f7 48 29 f8 31 ff [ 30.947782] RSP: 0018:ffffa4ed80ffbbb8 EFLAGS: 00010246 [ 30.947836] RAX: 0000000000000000 RBX: ffffa4ed80ffbc60 RCX: 0000000000000000 [ 30.947904] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 [ 30.947971] RBP: ffff94b0d15c0ae0 R08: 0000000000000000 R09: 0000000000000000 [ 30.948040] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 [ 30.948106] R13: 0000000000000001 R14: ffffa4ed80ffbc60 R15: 0000000000000000 [ 30.948174] FS: 00007fc7520f0740(0000) GS:ffff94b7ced40000(0000) knlGS:0000000000000000 [ 30.948252] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 30.948308] CR2: 0000000000000000 CR3: 0000000104a40001 CR4: 00000000003706e0 [ 30.948376] Call Trace: [ 30.948404] <TASK> [ 30.948431] ceph_security_init_secctx+0x7b/0x240 [ceph 49f9c4b9bf5be8760f19f1747e26da33920bce4b] [ 30.948582] ceph_atomic_open+0x51e/0x8a0 [ceph 49f9c4b9bf5be8760f19f1747e26da33920bce4b] [ 30.948708] ? get_cached_acl+0x4d/0xa0 [ 30.948759] path_openat+0x60d/0x1030 [ 30.948809] do_filp_open+0xa5/0x150 [ 30.948859] do_sys_openat2+0xc4/0x190 [ 30.948904] __x64_sys_openat+0x53/0xa0 [ 30.948948] do_syscall_64+0x5c/0x90 [ 30.948989] ? exc_page_fault+0x72/0x180 [ 30.949034] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 30.949091] RIP: 0033:0x7fc7521e25bb [ 30.950849] Code: 25 00 00 41 00 3d 00 00 41 00 74 4b 64 8b 04 25 18 00 00 00 85 c0 75 67 44 89 e2 48 89 ee bf 9c ff ff ff b8 01 01 0 0 00 0f 05 <48> 3d 00 f0 ff ff 0f 87 91 00 00 00 48 8b 54 24 28 64 48 2b 14 25 Core of the problem is that ceph checks for return code from security_dentry_init_security() and if return code is 0, it assumes everything is fine and continues to call strlen(name), which crashes. Typically SELinux LSM returns 0 and sets name to "security.selinux" and it is not a problem. Or if selinux is not compiled in or disabled, it returns -EOPNOTSUP and ceph deals with it. But somehow in this configuration, 0 is being returned and "name" is not being initialized and that's creating the problem. Our suspicion is that BPF LSM is registering a hook for dentry_init_security() and returns hook default of 0. LSM_HOOK(int, 0, dentry_init_security, struct dentry *dentry,...) I have not been able to reproduce it just by doing CONFIG_BPF_LSM=y. Stephen has tested the patch though and confirms it solves the problem for him. dentry_init_security() is written in such a way that it expects only one LSM to register the hook. Atleast that's the expectation with current code. If another LSM returns a hook and returns default, it will simply return 0 as of now and that will break ceph. Hence, suggestion is that change semantics of this hook a bit. If there are no LSMs or no LSM is taking ownership and initializing security context, then return -EOPNOTSUP. Also allow at max one LSM to initialize security context. This hook can't deal with multiple LSMs trying to init security context. This patch implements this new behavior. Reported-by:
Stephen Muth <smuth4@gmail.com> Tested-by:
Stephen Muth <smuth4@gmail.com> Suggested-by:
Casey Schaufler <casey@schaufler-ca.com> Acked-by:
Casey Schaufler <casey@schaufler-ca.com> Reviewed-by:
Serge Hallyn <serge@hallyn.com> Cc: Jeff Layton <jlayton@kernel.org> Cc: Christian Brauner <brauner@kernel.org> Cc: Paul Moore <paul@paul-moore.com> Cc: <stable@vger.kernel.org> # 5.16.0 Signed-off-by:
Vivek Goyal <vgoyal@redhat.com> Reviewed-by:
Jeff Layton <jlayton@kernel.org> Acked-by:
Paul Moore <paul@paul-moore.com> Acked-by:
Christian Brauner <brauner@kernel.org> Signed-off-by:
James Morris <jmorris@namei.org>
-
- Jan 27, 2022
-
-
Casey Schaufler authored
The usual LSM hook "bail on fail" scheme doesn't work for cases where a security module may return an error code indicating that it does not recognize an input. In this particular case Smack sees a mount option that it recognizes, and returns 0. A call to a BPF hook follows, which returns -ENOPARAM, which confuses the caller because Smack has processed its data. The SELinux hook incorrectly returns 1 on success. There was a time when this was correct, however the current expectation is that it return 0 on success. This is repaired. Reported-by:
<syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com> Signed-off-by:
Casey Schaufler <casey@schaufler-ca.com> Acked-by:
James Morris <jamorris@linux.microsoft.com> Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
Paul Moore authored
In the process of removing an explicit type cast to preserve a cred const qualifier in cred_init_security() we ran into a problem where the task_struct::real_cred field is defined with the "__rcu" attribute but the selinux_cred() function parameter is not, leading to a sparse warning: security/selinux/hooks.c:216:36: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct cred const *cred @@ got struct cred const [noderef] __rcu *real_cred As we don't want to add the "__rcu" attribute to the selinux_cred() parameter, we're going to add an explicit cast back to cred_init_security(). Fixes: b084e189 ("selinux: simplify cred_init_security") Reported-by:
kernel test robot <lkp@intel.com> Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
- Jan 26, 2022
-
-
Christian Göttsche authored
The macro _DEBUG_HASHES is nowhere used. The configuration DEBUG_HASHES enables debugging of the SELinux hash tables, but the with an underscore prefixed macro definition has no direct impact or any documentation. Reported by clang [-Wunused-macros] Signed-off-by:
Christian Göttsche <cgzones@googlemail.com> Reviewed-by:
Nick Desaulniers <ndesaulniers@google.com> Signed-off-by:
Paul Moore <paul@paul-moore.com>
-
Christian Göttsche authored
The parameter of selinux_cred() is declared const, so an explicit cast dropping the const qualifier is not necessary. Without the cast the local variable cred serves no purpose. Reported by clang [-Wcast-qual] Signed-off-by:
Christian Göttsche <cgzones@googlemail.com> Signed-off-by:
Paul Moore <paul@paul-moore.com>
-