linux-stable.git/security/integrity/ima, branch linux-6.9.y

ima: Avoid blocking in RCU read-side critical section

2024-06-27T11:52:30+00:00

commit 9a95c5bfbf02a0a7f5983280fe284a0ff0836c34 upstream.

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 c7423dbdbc9e ("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: c7423dbdbc9e ("ima: Handle -ESTALE returned by ima_filter_rule_match()")
Cc: stable@vger.kernel.org
Signed-off-by: GUO Zihua 
Acked-by: John Johansen 
Reviewed-by: Mimi Zohar 
Reviewed-by: Casey Schaufler 
[PM: fixed missing comment, long lines, !CONFIG_IMA_LSM_RULES case]
Signed-off-by: Paul Moore 
Signed-off-by: Greg Kroah-Hartman

ima: Fix use-after-free on a dentry's dname.name

2024-06-21T12:40:38+00:00

commit be84f32bb2c981ca670922e047cdde1488b233de upstream.

->d_name.name can change on rename and the earlier value can be freed;
there are conditions sufficient to stabilize it (->d_lock on dentry,
->d_lock on its parent, ->i_rwsem exclusive on the parent's inode,
rename_lock), but none of those are met at any of the sites. Take a stable
snapshot of the name instead.

Link: https://lore.kernel.org/all/20240202182732.GE2087318@ZenIV/
Signed-off-by: Al Viro 
Signed-off-by: Stefan Berger 
Signed-off-by: Mimi Zohar 
Signed-off-by: Greg Kroah-Hartman

ima: Make it independent from 'integrity' LSM

2024-02-16T04:43:47+00:00

Make the 'ima' LSM independent from the 'integrity' LSM by introducing IMA
own integrity metadata (ima_iint_cache structure, with IMA-specific fields
from the integrity_iint_cache structure), and by managing it directly from
the 'ima' LSM.

Create ima_iint.c and introduce the same integrity metadata management
functions found in iint.c (renamed with ima_). However, instead of putting
metadata in an rbtree, reserve space from IMA in the inode security blob
for a pointer, and introduce the ima_inode_set_iint()/ima_inode_get_iint()
primitives to store/retrieve that pointer. This improves search time from
logarithmic to constant.

Consequently, don't include the inode pointer as field in the
ima_iint_cache structure, since the association with the inode is clear.
Since the inode field is missing in ima_iint_cache, pass the extra inode
parameter to ima_get_verity_digest().

Prefer storing the pointer instead of the entire ima_iint_cache structure,
to avoid too much memory pressure. Use the same mechanism as before, a
cache named ima_iint_cache (renamed from iint_cache), to quickly allocate
a new ima_iint_cache structure when requested by the IMA policy.

Create the new ima_iint_cache in ima_iintcache_init(),
called by init_ima_lsm(), during the initialization of the 'ima' LSM. And,
register ima_inode_free_security() to free the ima_iint_cache structure, if
exists.

Replace integrity_iint_cache with ima_iint_cache in various places of the
IMA code. Also, replace integrity_inode_get() and integrity_iint_find(),
respectively with ima_inode_get() and ima_iint_find().

Finally, move the remaining IMA-specific flags
to security/integrity/ima/ima.h, since they are now unnecessary in the
common integrity layer.

Signed-off-by: Roberto Sassu 
Reviewed-by: Casey Schaufler 
Reviewed-by: Stefan Berger 
Reviewed-by: Mimi Zohar 
Acked-by: Mimi Zohar 
Signed-off-by: Paul Moore

evm: Make it independent from 'integrity' LSM

2024-02-16T04:43:47+00:00

Define a new structure for EVM-specific metadata, called evm_iint_cache,
and embed it in the inode security blob. Introduce evm_iint_inode() to
retrieve metadata, and register evm_inode_alloc_security() for the
inode_alloc_security LSM hook, to initialize the structure (before
splitting metadata, this task was done by iint_init_always()).

Keep the non-NULL checks after calling evm_iint_inode() except in
evm_inode_alloc_security(), to take into account inodes for which
security_inode_alloc() was not called. When using shared metadata,
obtaining a NULL pointer from integrity_iint_find() meant that the file
wasn't in the IMA policy. Now, because IMA and EVM use disjoint metadata,
the EVM status has to be stored for every inode regardless of the IMA
policy.

Given that from now on EVM relies on its own metadata, remove the iint
parameter from evm_verifyxattr(). Also, directly retrieve the iint in
evm_verify_hmac(), called by both evm_verifyxattr() and
evm_verify_current_integrity(), since now there is no performance penalty
in retrieving EVM metadata (constant time).

Replicate the management of the IMA_NEW_FILE flag, by introducing
evm_post_path_mknod() and evm_file_release() to respectively set and clear
the newly introduced flag EVM_NEW_FILE, at the same time IMA does. Like for
IMA, select CONFIG_SECURITY_PATH when EVM is enabled, to ensure that files
are marked as new.

Unlike ima_post_path_mknod(), evm_post_path_mknod() cannot check if a file
must be appraised. Thus, it marks all affected files. Also, it does not
clear EVM_NEW_FILE depending on i_version, but that is not a problem
because IMA_NEW_FILE is always cleared when set in ima_check_last_writer().

Move the EVM-specific flag EVM_IMMUTABLE_DIGSIG to
security/integrity/evm/evm.h, since that definition is now unnecessary in
the common integrity layer.

Finally, switch to the LSM reservation mechanism for the EVM xattr, and
consequently decrement by one the number of xattrs to allocate in
security_inode_init_security().

Signed-off-by: Roberto Sassu 
Reviewed-by: Casey Schaufler 
Reviewed-by: Stefan Berger 
Reviewed-by: Mimi Zohar 
Acked-by: Mimi Zohar 
Signed-off-by: Paul Moore

ima: Move IMA-Appraisal to LSM infrastructure

2024-02-16T04:43:46+00:00

A few additional IMA hooks are needed to reset the cached appraisal
status, causing the file's integrity to be re-evaluated on next access.
Register these IMA-appraisal only functions separately from the rest of IMA
functions, as appraisal is a separate feature not necessarily enabled in
the kernel configuration.

Reuse the same approach as for other IMA functions, move hardcoded calls
from various places in the kernel to the LSM infrastructure. Declare the
functions as static and register them as hook implementations in
init_ima_appraise_lsm(), called by init_ima_lsm().

Also move the inline function ima_inode_remove_acl() from the public ima.h
header to ima_appraise.c.

Signed-off-by: Roberto Sassu 
Reviewed-by: Stefan Berger 
Reviewed-by: Mimi Zohar 
Reviewed-by: Casey Schaufler 
Acked-by: Christian Brauner 
Acked-by: Mimi Zohar 
Signed-off-by: Paul Moore

ima: Move to LSM infrastructure

2024-02-16T04:43:46+00:00

Move hardcoded IMA function calls (not appraisal-specific functions) from
various places in the kernel to the LSM infrastructure, by introducing a
new LSM named 'ima' (at the end of the LSM list and always enabled like
'integrity').

Having IMA before EVM in the Makefile is sufficient to preserve the
relative order of the new 'ima' LSM in respect to the upcoming 'evm' LSM,
and thus the order of IMA and EVM function calls as when they were
hardcoded.

Make moved functions as static (except ima_post_key_create_or_update(),
which is not in ima_main.c), and register them as implementation of the
respective hooks in the new function init_ima_lsm().

Select CONFIG_SECURITY_PATH, to ensure that the path-based LSM hook
path_post_mknod is always available and ima_post_path_mknod() is always
executed to mark files as new, as before the move.

A slight difference is that IMA and EVM functions registered for the
inode_post_setattr, inode_post_removexattr, path_post_mknod,
inode_post_create_tmpfile, inode_post_set_acl and inode_post_remove_acl
won't be executed for private inodes. Since those inodes are supposed to be
fs-internal, they should not be of interest to IMA or EVM. The S_PRIVATE
flag is used for anonymous inodes, hugetlbfs, reiserfs xattrs, XFS scrub
and kernel-internal tmpfs files.

Conditionally register ima_post_key_create_or_update() if
CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS is enabled. Also, conditionally register
ima_kernel_module_request() if CONFIG_INTEGRITY_ASYMMETRIC_KEYS is enabled.

Finally, add the LSM_ID_IMA case in lsm_list_modules_test.c.

Signed-off-by: Roberto Sassu 
Acked-by: Chuck Lever 
Acked-by: Casey Schaufler 
Acked-by: Christian Brauner 
Reviewed-by: Stefan Berger 
Reviewed-by: Mimi Zohar 
Acked-by: Mimi Zohar 
Signed-off-by: Paul Moore

integrity: Move integrity_kernel_module_request() to IMA

2024-02-16T04:43:45+00:00

In preparation for removing the 'integrity' LSM, move
integrity_kernel_module_request() to IMA, and rename it to
ima_kernel_module_request(). Rewrite the function documentation, to explain
better what the problem is.

Compile it conditionally if CONFIG_INTEGRITY_ASYMMETRIC_KEYS is enabled,
and call it from security.c (removed afterwards with the move of IMA to the
LSM infrastructure).

Adding this hook cannot be avoided, since IMA has no control on the flags
passed to crypto_alloc_sig() in public_key_verify_signature(), and thus
cannot pass CRYPTO_NOLOAD, which solved the problem for EVM hashing with
commit e2861fa71641 ("evm: Don't deadlock if a crypto algorithm is
unavailable").

EVM alone does not need to implement this hook, first because there is no
mutex to deadlock, and second because even if it had it, there should be a
recursive call. However, since verification from EVM can be initiated only
by setting inode metadata, deadlock would occur if modprobe would do the
same while loading a kernel module (which is unlikely).

Signed-off-by: Roberto Sassu 
Reviewed-by: Stefan Berger 
Reviewed-by: Mimi Zohar 
Acked-by: Mimi Zohar 
Signed-off-by: Paul Moore

ima: Align ima_post_read_file() definition with LSM infrastructure

2024-02-16T04:43:40+00:00

Change ima_post_read_file() definition, by making "void *buf" a
"char *buf", so that it can be registered as implementation of the
post_read_file hook.

Signed-off-by: Roberto Sassu 
Reviewed-by: Stefan Berger 
Reviewed-by: Mimi Zohar 
Reviewed-by: Casey Schaufler 
Acked-by: Mimi Zohar 
Signed-off-by: Paul Moore

ima: Align ima_inode_removexattr() definition with LSM infrastructure

2024-02-16T04:43:39+00:00

Change ima_inode_removexattr() definition, so that it can be registered as
implementation of the inode_removexattr hook.

Signed-off-by: Roberto Sassu 
Reviewed-by: Stefan Berger 
Reviewed-by: Casey Schaufler 
Reviewed-by: Mimi Zohar 
Acked-by: Mimi Zohar 
Signed-off-by: Paul Moore

ima: Align ima_inode_setxattr() definition with LSM infrastructure

2024-02-16T04:43:39+00:00

Change ima_inode_setxattr() definition, so that it can be registered as
implementation of the inode_setxattr hook.

Signed-off-by: Roberto Sassu 
Reviewed-by: Stefan Berger 
Reviewed-by: Mimi Zohar 
Reviewed-by: Casey Schaufler 
Acked-by: Mimi Zohar 
Signed-off-by: Paul Moore