Skip to content

Commit 06cca51

Browse files
robertosassupcmoore
authored andcommitted
integrity: Move integrity_kernel_module_request() to IMA
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 e2861fa ("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 <[email protected]> Reviewed-by: Stefan Berger <[email protected]> Reviewed-by: Mimi Zohar <[email protected]> Acked-by: Mimi Zohar <[email protected]> Signed-off-by: Paul Moore <[email protected]>
1 parent b8d9970 commit 06cca51

File tree

5 files changed

+44
-37
lines changed

5 files changed

+44
-37
lines changed

include/linux/ima.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,4 +256,14 @@ static inline bool ima_appraise_signature(enum kernel_read_file_id func)
256256
return false;
257257
}
258258
#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
259+
260+
#if defined(CONFIG_IMA) && defined(CONFIG_INTEGRITY_ASYMMETRIC_KEYS)
261+
extern int ima_kernel_module_request(char *kmod_name);
262+
#else
263+
static inline int ima_kernel_module_request(char *kmod_name)
264+
{
265+
return 0;
266+
}
267+
268+
#endif
259269
#endif /* _LINUX_IMA_H */

include/linux/integrity.h

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,4 @@ static inline void integrity_load_keys(void)
4242
}
4343
#endif /* CONFIG_INTEGRITY */
4444

45-
#ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
46-
47-
extern int integrity_kernel_module_request(char *kmod_name);
48-
49-
#else
50-
51-
static inline int integrity_kernel_module_request(char *kmod_name)
52-
{
53-
return 0;
54-
}
55-
56-
#endif /* CONFIG_INTEGRITY_ASYMMETRIC_KEYS */
57-
5845
#endif /* _LINUX_INTEGRITY_H */

security/integrity/digsig_asymmetric.c

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -132,26 +132,3 @@ int asymmetric_verify(struct key *keyring, const char *sig,
132132
pr_debug("%s() = %d\n", __func__, ret);
133133
return ret;
134134
}
135-
136-
/**
137-
* integrity_kernel_module_request - prevent crypto-pkcs1pad(rsa,*) requests
138-
* @kmod_name: kernel module name
139-
*
140-
* We have situation, when public_key_verify_signature() in case of RSA
141-
* algorithm use alg_name to store internal information in order to
142-
* construct an algorithm on the fly, but crypto_larval_lookup() will try
143-
* to use alg_name in order to load kernel module with same name.
144-
* Since we don't have any real "crypto-pkcs1pad(rsa,*)" kernel modules,
145-
* we are safe to fail such module request from crypto_larval_lookup().
146-
*
147-
* In this way we prevent modprobe execution during digsig verification
148-
* and avoid possible deadlock if modprobe and/or it's dependencies
149-
* also signed with digsig.
150-
*/
151-
int integrity_kernel_module_request(char *kmod_name)
152-
{
153-
if (strncmp(kmod_name, "crypto-pkcs1pad(rsa,", 20) == 0)
154-
return -EINVAL;
155-
156-
return 0;
157-
}

security/integrity/ima/ima_main.c

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,6 +1091,39 @@ int ima_measure_critical_data(const char *event_label,
10911091
}
10921092
EXPORT_SYMBOL_GPL(ima_measure_critical_data);
10931093

1094+
#ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
1095+
1096+
/**
1097+
* ima_kernel_module_request - Prevent crypto-pkcs1pad(rsa,*) requests
1098+
* @kmod_name: kernel module name
1099+
*
1100+
* Avoid a verification loop where verifying the signature of the modprobe
1101+
* binary requires executing modprobe itself. Since the modprobe iint->mutex
1102+
* is already held when the signature verification is performed, a deadlock
1103+
* occurs as soon as modprobe is executed within the critical region, since
1104+
* the same lock cannot be taken again.
1105+
*
1106+
* This happens when public_key_verify_signature(), in case of RSA algorithm,
1107+
* use alg_name to store internal information in order to construct an
1108+
* algorithm on the fly, but crypto_larval_lookup() will try to use alg_name
1109+
* in order to load a kernel module with same name.
1110+
*
1111+
* Since we don't have any real "crypto-pkcs1pad(rsa,*)" kernel modules,
1112+
* we are safe to fail such module request from crypto_larval_lookup(), and
1113+
* avoid the verification loop.
1114+
*
1115+
* Return: Zero if it is safe to load the kernel module, -EINVAL otherwise.
1116+
*/
1117+
int ima_kernel_module_request(char *kmod_name)
1118+
{
1119+
if (strncmp(kmod_name, "crypto-pkcs1pad(rsa,", 20) == 0)
1120+
return -EINVAL;
1121+
1122+
return 0;
1123+
}
1124+
1125+
#endif /* CONFIG_INTEGRITY_ASYMMETRIC_KEYS */
1126+
10941127
static int __init init_ima(void)
10951128
{
10961129
int error;

security/security.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3249,7 +3249,7 @@ int security_kernel_module_request(char *kmod_name)
32493249
ret = call_int_hook(kernel_module_request, 0, kmod_name);
32503250
if (ret)
32513251
return ret;
3252-
return integrity_kernel_module_request(kmod_name);
3252+
return ima_kernel_module_request(kmod_name);
32533253
}
32543254

32553255
/**

0 commit comments

Comments
 (0)