* [PATCH v3] ima: Access decompressed kernel module to verify appended signature
From: Coiby Xu @ 2025-11-19 3:47 UTC (permalink / raw)
To: linux-integrity, Mimi Zohar
Cc: Karel Srot, Paul Moore, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, James Morris, Serge E. Hallyn, Fan Wu,
Stephen Smalley, Ondrej Mosnacek, open list,
open list:MODULE SUPPORT, open list:SECURITY SUBSYSTEM,
open list:SELINUX SECURITY MODULE
In-Reply-To: <20251031074016.1975356-1-coxu@redhat.com>
Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
is enabled, IMA has no way to verify the appended module signature as it
can't decompress the module.
Define a new kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
IMA can know only to collect original module data hash on
READING_MODULE_COMPRESSED and defer appraising/measuring it until on
READING_MODULE when the module has been decompressed.
Before enabling in-kernel module decompression, a kernel module in
initramfs can still be loaded with ima_policy=secure_boot. So adjust the
kernel module rule in secure_boot policy to allow either an IMA
signature OR an appended signature i.e. to use
"appraise func=MODULE_CHECK appraise_type=imasig|modsig".
Reported-by: Karel Srot <ksrot@redhat.com>
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Suggested-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
include/linux/kernel_read_file.h | 1 +
kernel/module/main.c | 17 ++++++++++++++---
security/integrity/ima/ima_main.c | 24 ++++++++++++++++--------
security/integrity/ima/ima_policy.c | 3 ++-
security/ipe/hooks.c | 1 +
security/selinux/hooks.c | 5 +++--
6 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
index 90451e2e12bd..d613a7b4dd35 100644
--- a/include/linux/kernel_read_file.h
+++ b/include/linux/kernel_read_file.h
@@ -14,6 +14,7 @@
id(KEXEC_INITRAMFS, kexec-initramfs) \
id(POLICY, security-policy) \
id(X509_CERTIFICATE, x509-certificate) \
+ id(MODULE_COMPRESSED, kernel-module-compressed) \
id(MAX_ID, )
#define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..7b3ec2fa6e7c 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3675,24 +3675,35 @@ static int idempotent_wait_for_completion(struct idempotent *u)
static int init_module_from_file(struct file *f, const char __user * uargs, int flags)
{
+ bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
struct load_info info = { };
void *buf = NULL;
int len;
+ int err;
- len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
+ len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
+ compressed ? READING_MODULE_COMPRESSED :
+ READING_MODULE);
if (len < 0) {
mod_stat_inc(&failed_kreads);
return len;
}
- if (flags & MODULE_INIT_COMPRESSED_FILE) {
- int err = module_decompress(&info, buf, len);
+ if (compressed) {
+ err = module_decompress(&info, buf, len);
vfree(buf); /* compressed data is no longer needed */
if (err) {
mod_stat_inc(&failed_decompress);
mod_stat_add_long(len, &invalid_decompress_bytes);
return err;
}
+ err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
+ READING_MODULE);
+ if (err) {
+ mod_stat_inc(&failed_kreads);
+ free_copy(&info, flags);
+ return err;
+ }
} else {
info.hdr = buf;
info.len = len;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index cdd225f65a62..49f8b2b1a9af 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -235,7 +235,8 @@ static void ima_file_free(struct file *file)
static int process_measurement(struct file *file, const struct cred *cred,
struct lsm_prop *prop, char *buf, loff_t size,
- int mask, enum ima_hooks func)
+ int mask, enum ima_hooks func,
+ enum kernel_read_file_id read_id)
{
struct inode *real_inode, *inode = file_inode(file);
struct ima_iint_cache *iint = NULL;
@@ -406,6 +407,12 @@ static int process_measurement(struct file *file, const struct cred *cred,
if (rc != 0 && rc != -EBADF && rc != -EINVAL)
goto out_locked;
+ /* Defer measuring/appraising kernel modules to READING_MODULE */
+ if (read_id == READING_MODULE_COMPRESSED) {
+ must_appraise = 0;
+ goto out_locked;
+ }
+
if (!pathbuf) /* ima_rdwr_violation possibly pre-fetched */
pathname = ima_d_path(&file->f_path, &pathbuf, filename);
@@ -486,14 +493,14 @@ static int ima_file_mmap(struct file *file, unsigned long reqprot,
if (reqprot & PROT_EXEC) {
ret = process_measurement(file, current_cred(), &prop, NULL,
- 0, MAY_EXEC, MMAP_CHECK_REQPROT);
+ 0, MAY_EXEC, MMAP_CHECK_REQPROT, 0);
if (ret)
return ret;
}
if (prot & PROT_EXEC)
return process_measurement(file, current_cred(), &prop, NULL,
- 0, MAY_EXEC, MMAP_CHECK);
+ 0, MAY_EXEC, MMAP_CHECK, 0);
return 0;
}
@@ -578,13 +585,13 @@ static int ima_bprm_check(struct linux_binprm *bprm)
security_current_getlsmprop_subj(&prop);
ret = process_measurement(bprm->file, current_cred(),
- &prop, NULL, 0, MAY_EXEC, BPRM_CHECK);
+ &prop, NULL, 0, MAY_EXEC, BPRM_CHECK, 0);
if (ret)
return ret;
security_cred_getlsmprop(bprm->cred, &prop);
return process_measurement(bprm->file, bprm->cred, &prop, NULL, 0,
- MAY_EXEC, CREDS_CHECK);
+ MAY_EXEC, CREDS_CHECK, 0);
}
/**
@@ -632,7 +639,7 @@ static int ima_file_check(struct file *file, int mask)
security_current_getlsmprop_subj(&prop);
return process_measurement(file, current_cred(), &prop, NULL, 0,
mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
- MAY_APPEND), FILE_CHECK);
+ MAY_APPEND), FILE_CHECK, 0);
}
static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf,
@@ -851,12 +858,13 @@ static int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
func = read_idmap[read_id] ?: FILE_CHECK;
security_current_getlsmprop_subj(&prop);
return process_measurement(file, current_cred(), &prop, NULL, 0,
- MAY_READ, func);
+ MAY_READ, func, 0);
}
const int read_idmap[READING_MAX_ID] = {
[READING_FIRMWARE] = FIRMWARE_CHECK,
[READING_MODULE] = MODULE_CHECK,
+ [READING_MODULE_COMPRESSED] = MODULE_CHECK,
[READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK,
[READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK,
[READING_POLICY] = POLICY_CHECK
@@ -894,7 +902,7 @@ static int ima_post_read_file(struct file *file, char *buf, loff_t size,
func = read_idmap[read_id] ?: FILE_CHECK;
security_current_getlsmprop_subj(&prop);
return process_measurement(file, current_cred(), &prop, buf, size,
- MAY_READ, func);
+ MAY_READ, func, read_id);
}
/**
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 128fab897930..ae520e6bb1cf 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -241,7 +241,8 @@ static struct ima_rule_entry build_appraise_rules[] __ro_after_init = {
static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
{.action = APPRAISE, .func = MODULE_CHECK,
- .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+ .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED |
+ IMA_CHECK_BLACKLIST},
{.action = APPRAISE, .func = FIRMWARE_CHECK,
.flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
{.action = APPRAISE, .func = KEXEC_KERNEL_CHECK,
diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
index d0323b81cd8f..1053a4acf589 100644
--- a/security/ipe/hooks.c
+++ b/security/ipe/hooks.c
@@ -118,6 +118,7 @@ int ipe_kernel_read_file(struct file *file, enum kernel_read_file_id id,
op = IPE_OP_FIRMWARE;
break;
case READING_MODULE:
+ case READING_MODULE_COMPRESSED:
op = IPE_OP_KERNEL_MODULE;
break;
case READING_KEXEC_INITRAMFS:
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index dfc22da42f30..c1ff69d5d76e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4275,7 +4275,7 @@ static int selinux_kernel_read_file(struct file *file,
{
int rc = 0;
- BUILD_BUG_ON_MSG(READING_MAX_ID > 7,
+ BUILD_BUG_ON_MSG(READING_MAX_ID > 8,
"New kernel_read_file_id introduced; update SELinux!");
switch (id) {
@@ -4283,6 +4283,7 @@ static int selinux_kernel_read_file(struct file *file,
rc = selinux_kernel_load_from_file(file, SYSTEM__FIRMWARE_LOAD);
break;
case READING_MODULE:
+ case READING_MODULE_COMPRESSED:
rc = selinux_kernel_load_from_file(file, SYSTEM__MODULE_LOAD);
break;
case READING_KEXEC_IMAGE:
@@ -4311,7 +4312,7 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents)
{
int rc = 0;
- BUILD_BUG_ON_MSG(LOADING_MAX_ID > 7,
+ BUILD_BUG_ON_MSG(LOADING_MAX_ID > 8,
"New kernel_load_data_id introduced; update SELinux!");
switch (id) {
base-commit: 6a23ae0a96a600d1d12557add110e0bb6e32730c
--
2.51.1
^ permalink raw reply related
* Re: [PATCH] KEYS: encrypted: Replace deprecated strcpy and improve get_derived_key
From: Jarkko Sakkinen @ 2025-11-19 2:50 UTC (permalink / raw)
To: Thorsten Blum
Cc: Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, linux-hardening, linux-integrity, keyrings,
linux-security-module, linux-kernel
In-Reply-To: <20251113135831.98587-1-thorsten.blum@linux.dev>
On Thu, Nov 13, 2025 at 02:58:31PM +0100, Thorsten Blum wrote:
> Determine 'key_name' before allocating memory for 'derived_buf' to only
> allocate as many bytes as needed. Currently, we potentially allocate one
Who is "we"?
> more byte than necessary when 'key_name' is "ENC_KEY".
>
> strcpy() is deprecated and uses an additional strlen() internally; use
> memcpy() directly to copy 'key_name' since we already know its length
> and that it is guaranteed to be NUL-terminated.
>
> Also reuse 'key_name_len' when copying 'master_key' instead of calling
> strlen() again.
>
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> security/keys/encrypted-keys/encrypted.c | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> index 15841466b5d4..b16a5b8b935b 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -12,6 +12,7 @@
> */
>
> #include <linux/uaccess.h>
> +#include <linux/minmax.h>
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/slab.h>
> @@ -330,23 +331,18 @@ static int get_derived_key(u8 *derived_key, enum derived_key_type key_type,
> const u8 *master_key, size_t master_keylen)
> {
> u8 *derived_buf;
> - unsigned int derived_buf_len;
> -
> - derived_buf_len = strlen("AUTH_KEY") + 1 + master_keylen;
> - if (derived_buf_len < HASH_SIZE)
> - derived_buf_len = HASH_SIZE;
> + size_t derived_buf_len;
> + const char *key_name;
> + size_t key_name_len;
>
> + key_name = key_type ? "AUTH_KEY" : "ENC_KEY";
> + key_name_len = strlen(key_name) + 1;
> + derived_buf_len = max(key_name_len + master_keylen, HASH_SIZE);
> derived_buf = kzalloc(derived_buf_len, GFP_KERNEL);
> if (!derived_buf)
> return -ENOMEM;
> -
> - if (key_type)
> - strcpy(derived_buf, "AUTH_KEY");
> - else
> - strcpy(derived_buf, "ENC_KEY");
> -
> - memcpy(derived_buf + strlen(derived_buf) + 1, master_key,
> - master_keylen);
> + memcpy(derived_buf, key_name, key_name_len);
> + memcpy(derived_buf + key_name_len, master_key, master_keylen);
> sha256(derived_buf, derived_buf_len, derived_key);
> kfree_sensitive(derived_buf);
> return 0;
> --
> 2.51.1
>
I don't see much value in this other than potentially causing merge
conflicts when backporting bug fixes.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: encrypted: Use pr_fmt()
From: Jarkko Sakkinen @ 2025-11-19 2:48 UTC (permalink / raw)
To: Thorsten Blum
Cc: Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, linux-integrity, keyrings, linux-security-module,
linux-kernel
In-Reply-To: <20251113123544.11287-2-thorsten.blum@linux.dev>
On Thu, Nov 13, 2025 at 01:35:44PM +0100, Thorsten Blum wrote:
> Use pr_fmt() to automatically prefix all pr_<level>() log messages with
This fails to describe what "use" means.
> "encrypted_key: " and remove all manually added prefixes.
>
> Reformat the code accordingly and avoid line breaks in log messages.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> security/keys/encrypted-keys/encrypted.c | 74 +++++++++++-------------
> security/keys/encrypted-keys/encrypted.h | 2 +-
> 2 files changed, 35 insertions(+), 41 deletions(-)
>
> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> index 513c09e2b01c..a8e8bf949b4b 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -11,6 +11,8 @@
> * See Documentation/security/keys/trusted-encrypted.rst
> */
>
Should have undef prepending.
> +#define pr_fmt(fmt) "encrypted_key: " fmt
> +
> #include <linux/uaccess.h>
> #include <linux/module.h>
> #include <linux/init.h>
> @@ -84,8 +86,7 @@ static int aes_get_sizes(void)
>
> tfm = crypto_alloc_skcipher(blkcipher_alg, 0, CRYPTO_ALG_ASYNC);
> if (IS_ERR(tfm)) {
> - pr_err("encrypted_key: failed to alloc_cipher (%ld)\n",
> - PTR_ERR(tfm));
> + pr_err("failed to alloc_cipher (%ld)\n", PTR_ERR(tfm));
> return PTR_ERR(tfm);
> }
> ivsize = crypto_skcipher_ivsize(tfm);
> @@ -106,15 +107,14 @@ static int valid_ecryptfs_desc(const char *ecryptfs_desc)
> int i;
>
> if (strlen(ecryptfs_desc) != KEY_ECRYPTFS_DESC_LEN) {
> - pr_err("encrypted_key: key description must be %d hexadecimal "
> - "characters long\n", KEY_ECRYPTFS_DESC_LEN);
> + pr_err("key description must be %d hexadecimal characters long\n",
> + KEY_ECRYPTFS_DESC_LEN);
> return -EINVAL;
> }
>
> for (i = 0; i < KEY_ECRYPTFS_DESC_LEN; i++) {
> if (!isxdigit(ecryptfs_desc[i])) {
> - pr_err("encrypted_key: key description must contain "
> - "only hexadecimal characters\n");
> + pr_err("key description must contain only hexadecimal characters\n");
> return -EINVAL;
> }
> }
> @@ -180,7 +180,7 @@ static int datablob_parse(char *datablob, const char **format,
>
> keyword = strsep(&datablob, " \t");
> if (!keyword) {
> - pr_info("encrypted_key: insufficient parameters specified\n");
> + pr_info("insufficient parameters specified\n");
> return ret;
> }
> key_cmd = match_token(keyword, key_tokens, args);
> @@ -188,7 +188,7 @@ static int datablob_parse(char *datablob, const char **format,
> /* Get optional format: default | ecryptfs */
> p = strsep(&datablob, " \t");
> if (!p) {
> - pr_err("encrypted_key: insufficient parameters specified\n");
> + pr_err("insufficient parameters specified\n");
> return ret;
> }
>
> @@ -206,20 +206,20 @@ static int datablob_parse(char *datablob, const char **format,
> }
>
> if (!*master_desc) {
> - pr_info("encrypted_key: master key parameter is missing\n");
> + pr_info("master key parameter is missing\n");
> goto out;
> }
>
> if (valid_master_desc(*master_desc, NULL) < 0) {
> - pr_info("encrypted_key: master key parameter \'%s\' "
> - "is invalid\n", *master_desc);
> + pr_info("master key parameter \'%s\' is invalid\n",
> + *master_desc);
> goto out;
> }
>
> if (decrypted_datalen) {
> *decrypted_datalen = strsep(&datablob, " \t");
> if (!*decrypted_datalen) {
> - pr_info("encrypted_key: keylen parameter is missing\n");
> + pr_info("keylen parameter is missing\n");
> goto out;
> }
> }
> @@ -227,8 +227,8 @@ static int datablob_parse(char *datablob, const char **format,
> switch (key_cmd) {
> case Opt_new:
> if (!decrypted_datalen) {
> - pr_info("encrypted_key: keyword \'%s\' not allowed "
> - "when called from .update method\n", keyword);
> + pr_info("keyword \'%s\' not allowed when called from .update method\n",
> + keyword);
> break;
> }
> *decrypted_data = strsep(&datablob, " \t");
> @@ -236,29 +236,27 @@ static int datablob_parse(char *datablob, const char **format,
> break;
> case Opt_load:
> if (!decrypted_datalen) {
> - pr_info("encrypted_key: keyword \'%s\' not allowed "
> - "when called from .update method\n", keyword);
> + pr_info("keyword \'%s\' not allowed when called from .update method\n",
> + keyword);
> break;
> }
> *hex_encoded_iv = strsep(&datablob, " \t");
> if (!*hex_encoded_iv) {
> - pr_info("encrypted_key: hex blob is missing\n");
> + pr_info("hex blob is missing\n");
> break;
> }
> ret = 0;
> break;
> case Opt_update:
> if (decrypted_datalen) {
> - pr_info("encrypted_key: keyword \'%s\' not allowed "
> - "when called from .instantiate method\n",
> + pr_info("keyword \'%s\' not allowed when called from .instantiate method\n",
> keyword);
> break;
> }
> ret = 0;
> break;
> case Opt_err:
> - pr_info("encrypted_key: keyword \'%s\' not recognized\n",
> - keyword);
> + pr_info("keyword \'%s\' not recognized\n", keyword);
> break;
> }
> out:
> @@ -362,22 +360,21 @@ static struct skcipher_request *init_skcipher_req(const u8 *key,
>
> tfm = crypto_alloc_skcipher(blkcipher_alg, 0, CRYPTO_ALG_ASYNC);
> if (IS_ERR(tfm)) {
> - pr_err("encrypted_key: failed to load %s transform (%ld)\n",
> - blkcipher_alg, PTR_ERR(tfm));
> + pr_err("failed to load %s transform (%ld)\n", blkcipher_alg,
> + PTR_ERR(tfm));
> return ERR_CAST(tfm);
> }
>
> ret = crypto_skcipher_setkey(tfm, key, key_len);
> if (ret < 0) {
> - pr_err("encrypted_key: failed to setkey (%d)\n", ret);
> + pr_err("failed to setkey (%d)\n", ret);
> crypto_free_skcipher(tfm);
> return ERR_PTR(ret);
> }
>
> req = skcipher_request_alloc(tfm, GFP_KERNEL);
> if (!req) {
> - pr_err("encrypted_key: failed to allocate request for %s\n",
> - blkcipher_alg);
> + pr_err("failed to allocate request for %s\n", blkcipher_alg);
> crypto_free_skcipher(tfm);
> return ERR_PTR(-ENOMEM);
> }
> @@ -406,13 +403,10 @@ static struct key *request_master_key(struct encrypted_key_payload *epayload,
>
> if (IS_ERR(mkey)) {
> int ret = PTR_ERR(mkey);
> -
> if (ret == -ENOTSUPP)
> - pr_info("encrypted_key: key %s not supported",
> - epayload->master_desc);
> + pr_info("key %s not supported", epayload->master_desc);
> else
> - pr_info("encrypted_key: key %s not found",
> - epayload->master_desc);
> + pr_info("key %s not found", epayload->master_desc);
> goto out;
> }
>
> @@ -457,7 +451,7 @@ static int derived_key_encrypt(struct encrypted_key_payload *epayload,
> skcipher_request_free(req);
> crypto_free_skcipher(tfm);
> if (ret < 0)
> - pr_err("encrypted_key: failed to encrypt (%d)\n", ret);
> + pr_err("failed to encrypt (%d)\n", ret);
> else
> dump_encrypted_data(epayload, encrypted_datalen);
> out:
> @@ -596,16 +590,16 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
>
> if (decrypted_data) {
> if (!user_decrypted_data) {
> - pr_err("encrypted key: instantiation of keys using provided decrypted data is disabled since CONFIG_USER_DECRYPTED_DATA is set to false\n");
> + pr_err("instantiation of keys using provided decrypted data is disabled since CONFIG_USER_DECRYPTED_DATA is set to false\n");
> return ERR_PTR(-EINVAL);
> }
> if (strlen(decrypted_data) != decrypted_datalen * 2) {
> - pr_err("encrypted key: decrypted data provided does not match decrypted data length provided\n");
> + pr_err("decrypted data provided does not match decrypted data length provided\n");
> return ERR_PTR(-EINVAL);
> }
> for (i = 0; i < strlen(decrypted_data); i++) {
> if (!isxdigit(decrypted_data[i])) {
> - pr_err("encrypted key: decrypted data provided must contain only hexadecimal characters\n");
> + pr_err("decrypted data provided must contain only hexadecimal characters\n");
> return ERR_PTR(-EINVAL);
> }
> }
> @@ -614,7 +608,7 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
> if (format) {
> if (!strcmp(format, key_format_ecryptfs)) {
> if (dlen != ECRYPTFS_MAX_KEY_BYTES) {
> - pr_err("encrypted_key: keylen for the ecryptfs format must be equal to %d bytes\n",
> + pr_err("keylen for the ecryptfs format must be equal to %d bytes\n",
> ECRYPTFS_MAX_KEY_BYTES);
> return ERR_PTR(-EINVAL);
> }
> @@ -622,8 +616,8 @@ static struct encrypted_key_payload *encrypted_key_alloc(struct key *key,
> payload_datalen = sizeof(struct ecryptfs_auth_tok);
> } else if (!strcmp(format, key_format_enc32)) {
> if (decrypted_datalen != KEY_ENC32_PAYLOAD_LEN) {
> - pr_err("encrypted_key: enc32 key payload incorrect length: %d\n",
> - decrypted_datalen);
> + pr_err("enc32 key payload incorrect length: %d\n",
> + decrypted_datalen);
> return ERR_PTR(-EINVAL);
> }
> }
> @@ -689,7 +683,7 @@ static int encrypted_key_decrypt(struct encrypted_key_payload *epayload,
>
> ret = datablob_hmac_verify(epayload, format, master_key, master_keylen);
> if (ret < 0) {
> - pr_err("encrypted_key: bad hmac (%d)\n", ret);
> + pr_err("bad hmac (%d)\n", ret);
> goto out;
> }
>
> @@ -699,7 +693,7 @@ static int encrypted_key_decrypt(struct encrypted_key_payload *epayload,
>
> ret = derived_key_decrypt(epayload, derived_key, sizeof derived_key);
> if (ret < 0)
> - pr_err("encrypted_key: failed to decrypt key (%d)\n", ret);
> + pr_err("failed to decrypt key (%d)\n", ret);
> out:
> up_read(&mkey->sem);
> key_put(mkey);
> diff --git a/security/keys/encrypted-keys/encrypted.h b/security/keys/encrypted-keys/encrypted.h
> index 1809995db452..7b05c66bafa6 100644
> --- a/security/keys/encrypted-keys/encrypted.h
> +++ b/security/keys/encrypted-keys/encrypted.h
> @@ -41,7 +41,7 @@ static inline void dump_hmac(const char *str, const u8 *digest,
> unsigned int hmac_size)
> {
> if (str)
> - pr_info("encrypted_key: %s", str);
> + pr_info("%s", str);
> print_hex_dump(KERN_ERR, "hmac: ", DUMP_PREFIX_NONE, 32, 1, digest,
> hmac_size, 0);
> }
> --
> 2.51.1
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] keys: Replace memcpy with strscpy in proc_keys_show
From: Jarkko Sakkinen @ 2025-11-19 2:36 UTC (permalink / raw)
To: Thorsten Blum
Cc: David Howells, Paul Moore, James Morris, Serge E. Hallyn,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20251112172620.4254-2-thorsten.blum@linux.dev>
On Wed, Nov 12, 2025 at 06:26:21PM +0100, Thorsten Blum wrote:
> Use strscpy() to copy strings into the fixed-size buffer 'xbuf' instead
> of hardcoding the number of bytes to copy. This improves maintainability
I don't consider maintainaibility as a stimulus of applying change given
that as a word it means me absolutely nothing.
> and ensures the buffer is always NUL-terminated.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> security/keys/proc.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/security/keys/proc.c b/security/keys/proc.c
> index 4f4e2c1824f1..1d380766f45d 100644
> --- a/security/keys/proc.c
> +++ b/security/keys/proc.c
> @@ -10,6 +10,7 @@
> #include <linux/fs.h>
> #include <linux/proc_fs.h>
> #include <linux/seq_file.h>
> +#include <linux/string.h>
> #include <asm/errno.h>
> #include "internal.h"
>
> @@ -199,9 +200,9 @@ static int proc_keys_show(struct seq_file *m, void *v)
> /* come up with a suitable timeout value */
> expiry = READ_ONCE(key->expiry);
> if (expiry == TIME64_MAX) {
> - memcpy(xbuf, "perm", 5);
> + strscpy(xbuf, "perm");
> } else if (now >= expiry) {
> - memcpy(xbuf, "expd", 5);
> + strscpy(xbuf, "expd");
> } else {
> timo = expiry - now;
>
> --
> 2.51.0
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: Remove the ad-hoc compilation flag CAAM_DEBUG
From: Jarkko Sakkinen @ 2025-11-19 2:25 UTC (permalink / raw)
To: Ye Bin
Cc: a.fatoum, kernel, James.Bottomley, zohar, dhowells, paul, jmorris,
serge, linux-integrity, keyrings, linux-security-module, yebin10
In-Reply-To: <20251028132254.841715-1-yebin@huaweicloud.com>
On Tue, Oct 28, 2025 at 09:22:54PM +0800, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
>
> Fix the broken design based on Jarkko Sakkinen's suggestions as follows:
>
> 1. Remove the ad-hoc compilation flag (i.e., CAAM_DEBUG).
> 2. Substitute pr_info calls with pr_debug calls.
>
> Closes: https://patchwork.kernel.org/project/linux-integrity/patch/20251024061153.61470-1-yebin@huaweicloud.com/
> Signed-off-by: Ye Bin <yebin10@huawei.com>
$ git am -3 20251028_yebin_keys_remove_the_ad_hoc_compilation_flag_caam_debug.mbx
Applying: KEYS: Remove the ad-hoc compilation flag CAAM_DEBUG
error: sha1 information is lacking or useless (security/keys/trusted-keys/trusted_caam.c).
error: could not build fake ancestor
Patch failed at 0001 KEYS: Remove the ad-hoc compilation flag CAAM_DEBUG
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Hmm.. Could you send me a new revision rebased on top of my tree?
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] Clarify the rootid_owns_currentns
From: Paul Moore @ 2025-11-19 0:29 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: lkml, linux-security-module, Ryan Foster, Christian Brauner
In-Reply-To: <aR0JrOvDxDKZPELd@mail.hallyn.com>
On Tue, Nov 18, 2025 at 7:05 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> On Tue, Nov 18, 2025 at 10:47:06AM -0500, Paul Moore wrote:
> > On Tue, Nov 18, 2025 at 9:16 AM Serge E. Hallyn <serge@hallyn.com> wrote:
> > > On Fri, Nov 14, 2025 at 03:33:19PM -0600, Serge E. Hallyn wrote:
> > > > Split most of the rootid_owns_currentns() functionality
> > > > into a more generic rootid_owns_ns() function which
> > > > will be easier to write tests for.
> > > >
> > > > Rename the functions and variables to make clear that
> > > > the ids being tested could be any uid.
> > > >
> > > > Signed-off-by: Serge Hallyn <serge@hallyn.com>
> > > > CC: Ryan Foster <foster.ryan.r@gmail.com>
> > > > CC: Christian Brauner <brauner@kernel.org>
> > >
> > > Paul, Christian, let me know if you have any objections, else I will
> > > queue this up in caps-next.
> >
> > Seems reasonable to me, but it would be good to fix the parameter doc
> > bug that the kernel test robot identified. I suspect it is just the
> > extra vertical comment space between the top one line summary and the
> > parameter list.
>
> Actually I think it was probably the use of - instead of : after the
> parameter name, but I went ahead and changed both, thanks.
Ah, yeah, at least one of those was it I'm sure.
> Pushed the below patch for linux-next.
Thanks.
--
paul-moore.com
^ permalink raw reply
* [PATCH] lsm: use unrcu_pointer() for current->cred in security_init()
From: Paul Moore @ 2025-11-19 0:28 UTC (permalink / raw)
To: linux-security-module
We need to directly allocate the cred's LSM state for the initial task
when we initialize the LSM framework. Unfortunately, this results in a
RCU related type mismatch, use the unrcu_pointer() macro to handle this
a bit more elegantly.
The explicit type casting still remains as we need to work around the
constification of current->cred in this particular case.
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
security/lsm_init.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/lsm_init.c b/security/lsm_init.c
index 6bb67d41ce52..4dec9199e4c2 100644
--- a/security/lsm_init.c
+++ b/security/lsm_init.c
@@ -467,7 +467,8 @@ int __init security_init(void)
blob_sizes.lbs_inode, 0,
SLAB_PANIC, NULL);
- if (lsm_cred_alloc((struct cred __rcu *)current->cred, GFP_KERNEL))
+ if (lsm_cred_alloc(unrcu_pointer((struct cred __rcu *)current->cred),
+ GFP_KERNEL))
panic("early LSM cred alloc failed\n");
if (lsm_task_alloc(current))
panic("early LSM task alloc failed\n");
--
2.52.0
^ permalink raw reply related
* Re: [PATCH] Clarify the rootid_owns_currentns
From: Serge E. Hallyn @ 2025-11-19 0:05 UTC (permalink / raw)
To: Paul Moore
Cc: Serge E. Hallyn, lkml, linux-security-module, Ryan Foster,
Christian Brauner
In-Reply-To: <CAHC9VhQu7gSVO-QZFE_iaCB0qBqB3surdHQo4Vg71zc890uEhA@mail.gmail.com>
On Tue, Nov 18, 2025 at 10:47:06AM -0500, Paul Moore wrote:
> On Tue, Nov 18, 2025 at 9:16 AM Serge E. Hallyn <serge@hallyn.com> wrote:
> > On Fri, Nov 14, 2025 at 03:33:19PM -0600, Serge E. Hallyn wrote:
> > > Split most of the rootid_owns_currentns() functionality
> > > into a more generic rootid_owns_ns() function which
> > > will be easier to write tests for.
> > >
> > > Rename the functions and variables to make clear that
> > > the ids being tested could be any uid.
> > >
> > > Signed-off-by: Serge Hallyn <serge@hallyn.com>
> > > CC: Ryan Foster <foster.ryan.r@gmail.com>
> > > CC: Christian Brauner <brauner@kernel.org>
> >
> > Paul, Christian, let me know if you have any objections, else I will
> > queue this up in caps-next.
>
> Seems reasonable to me, but it would be good to fix the parameter doc
> bug that the kernel test robot identified. I suspect it is just the
> extra vertical comment space between the top one line summary and the
> parameter list.
Actually I think it was probably the use of - instead of : after the
parameter name, but I went ahead and changed both, thanks.
Pushed the below patch for linux-next.
Subject: [PATCH 1/1] Clarify the rootid_owns_currentns
Split most of the rootid_owns_currentns() functionality
into a more generic rootid_owns_ns() function which
will be easier to write tests for.
Rename the functions and variables to make clear that
the ids being tested could be any uid.
Signed-off-by: Serge Hallyn <serge@hallyn.com>
CC: Ryan Foster <foster.ryan.r@gmail.com>
CC: Christian Brauner <brauner@kernel.org>
---
v2: change the function parameter documentation to mollify the bot.
---
security/commoncap.c | 34 ++++++++++++++++++++++------------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/security/commoncap.c b/security/commoncap.c
index 6bd4adeb4795..496e054c5d37 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -358,17 +358,17 @@ int cap_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry)
return error;
}
-static bool rootid_owns_currentns(vfsuid_t rootvfsuid)
+/**
+ * kuid_root_in_ns - check whether the given kuid is root in the given ns
+ * @kuid: the kuid to be tested
+ * @ns: the user namespace to test against
+ *
+ * Returns true if @kuid represents the root user in @ns, false otherwise.
+ */
+static bool kuid_root_in_ns(kuid_t kuid, struct user_namespace *ns)
{
- struct user_namespace *ns;
- kuid_t kroot;
-
- if (!vfsuid_valid(rootvfsuid))
- return false;
-
- kroot = vfsuid_into_kuid(rootvfsuid);
- for (ns = current_user_ns();; ns = ns->parent) {
- if (from_kuid(ns, kroot) == 0)
+ for (;; ns = ns->parent) {
+ if (from_kuid(ns, kuid) == 0)
return true;
if (ns == &init_user_ns)
break;
@@ -377,6 +377,16 @@ static bool rootid_owns_currentns(vfsuid_t rootvfsuid)
return false;
}
+static bool vfsuid_root_in_currentns(vfsuid_t vfsuid)
+{
+ kuid_t kuid;
+
+ if (!vfsuid_valid(vfsuid))
+ return false;
+ kuid = vfsuid_into_kuid(vfsuid);
+ return kuid_root_in_ns(kuid, current_user_ns());
+}
+
static __u32 sansflags(__u32 m)
{
return m & ~VFS_CAP_FLAGS_EFFECTIVE;
@@ -481,7 +491,7 @@ int cap_inode_getsecurity(struct mnt_idmap *idmap,
goto out_free;
}
- if (!rootid_owns_currentns(vfsroot)) {
+ if (!vfsuid_root_in_currentns(vfsroot)) {
size = -EOVERFLOW;
goto out_free;
}
@@ -722,7 +732,7 @@ int get_vfs_caps_from_disk(struct mnt_idmap *idmap,
/* Limit the caps to the mounter of the filesystem
* or the more limited uid specified in the xattr.
*/
- if (!rootid_owns_currentns(rootvfsuid))
+ if (!vfsuid_root_in_currentns(rootvfsuid))
return -ENODATA;
cpu_caps->permitted.val = le32_to_cpu(caps->data[0].permitted);
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] Clarify the rootid_owns_currentns
From: Serge E. Hallyn @ 2025-11-18 23:59 UTC (permalink / raw)
To: Paul Moore
Cc: Serge E. Hallyn, lkml, linux-security-module, Ryan Foster,
Christian Brauner
In-Reply-To: <CAHC9VhQu7gSVO-QZFE_iaCB0qBqB3surdHQo4Vg71zc890uEhA@mail.gmail.com>
On Tue, Nov 18, 2025 at 10:47:06AM -0500, Paul Moore wrote:
> On Tue, Nov 18, 2025 at 9:16 AM Serge E. Hallyn <serge@hallyn.com> wrote:
> > On Fri, Nov 14, 2025 at 03:33:19PM -0600, Serge E. Hallyn wrote:
> > > Split most of the rootid_owns_currentns() functionality
> > > into a more generic rootid_owns_ns() function which
> > > will be easier to write tests for.
> > >
> > > Rename the functions and variables to make clear that
> > > the ids being tested could be any uid.
> > >
> > > Signed-off-by: Serge Hallyn <serge@hallyn.com>
> > > CC: Ryan Foster <foster.ryan.r@gmail.com>
> > > CC: Christian Brauner <brauner@kernel.org>
> >
> > Paul, Christian, let me know if you have any objections, else I will
> > queue this up in caps-next.
>
> Seems reasonable to me, but it would be good to fix the parameter doc
> bug that the kernel test robot identified. I suspect it is just the
> extra vertical comment space between the top one line summary and the
> parameter list.
ooh, is that it. I couldn't for the life of me figure out what it
was complaining about. Will try that, thanks.
^ permalink raw reply
* [PATCH v18] exec: Fix dead-lock in de_thread with ptrace_attach
From: Bernd Edlinger @ 2025-11-18 18:13 UTC (permalink / raw)
To: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Eric W. Biederman,
Adrian Reber, Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Alexey Dobriyan,
Jens Axboe, Paul Moore, Elena Reshetova, David Windsor,
Mateusz Guzik, YueHaibing, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, tiozhang, Penglei Jiang, Lorenzo Stoakes,
Adrian Ratiu, Ingo Molnar, Peter Zijlstra (Intel),
Cyrill Gorcunov, Eric Dumazet
In-Reply-To: <GV2PPF74270EBEE9EF78827D73D3D7212F7E432A@GV2PPF74270EBEE.EURP195.PROD.OUTLOOK.COM>
This introduces signal->exec_bprm, which is used to
fix the case when at least one of the sibling threads
is traced, and therefore the trace process may dead-lock
in ptrace_attach, but de_thread will need to wait for the
tracer to continue execution.
The problem happens when a tracer tries to ptrace_attach
to a multi-threaded process, that does an execve in one of
the threads at the same time, without doing that in a forked
sub-process. That means: There is a race condition, when one
or more of the threads are already ptraced, but the thread
that invoked the execve is not yet traced. Now in this
case the execve locks the cred_guard_mutex and waits for
de_thread to complete. But that waits for the traced
sibling threads to exit, and those have to wait for the
tracer to receive the exit signal, but the tracer cannot
call wait right now, because it is waiting for the ptrace
call to complete, and this never does not happen.
The traced process and the tracer are now in a deadlock
situation, and can only be killed by a fatal signal.
The solution is to detect this situation and allow
ptrace_attach to continue by temporarily releasing the
cred_guard_mutex, while de_thread() is still waiting for
traced zombies to be eventually released by the tracer.
In the case of the thread group leader we only have to wait
for the thread to become a zombie, which may also need
co-operation from the tracer due to PTRACE_O_TRACEEXIT.
When a tracer wants to ptrace_attach a task that already
is in execve, we simply retry the ptrace_may_access
check with the new PTRACE_MODE_BPRMCREDS flag to check
that the tracer has permission to trace the new credentials
and dumpability which are about to be used after execve
completes. If the ptrace_attach happens on a thread that
is a sibling-thread of the thread doing execve, it is
sufficient to check against the old credentials, as this
thread will be waited for, before the new credentials are
installed.
Other threads die quickly since the cred_guard_mutex is
released, but a deadly signal is already pending. In case
the mutex_lock_killable misses the signal, the non-zero
current->signal->exec_bprm makes sure they release the
mutex immediately and return with -ERESTARTNOINTR.
This means there is no API change, unlike the previous
version of this patch which was discussed here:
https://lore.kernel.org/lkml/b6537ae6-31b1-5c50-f32b-8b8332ace882@hotmail.de/
See tools/testing/selftests/ptrace/vmaccess.c
for a test case that gets fixed by this change.
Note that since the test case was originally designed to
test the ptrace_attach returning an error in this situation,
the test expectation needed to be adjusted, to allow the
API to succeed at the first attempt.
Signed-off-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
---
fs/exec.c | 86 +++++++++++---
fs/proc/base.c | 12 ++
include/linux/cred.h | 1 +
include/linux/ptrace.h | 1 +
include/linux/sched/signal.h | 18 +++
kernel/cred.c | 30 ++++-
kernel/ptrace.c | 29 ++++-
kernel/seccomp.c | 18 ++-
security/apparmor/lsm.c | 5 +-
security/commoncap.c | 5 +-
security/landlock/task.c | 7 +-
security/selinux/hooks.c | 7 +-
security/smack/smack_lsm.c | 5 +-
security/yama/yama_lsm.c | 11 +-
tools/testing/selftests/ptrace/vmaccess.c | 135 ++++++++++++++++++++--
15 files changed, 324 insertions(+), 46 deletions(-)
v10: Changes to previous version, make the PTRACE_ATTACH
return -EAGAIN, instead of execve return -ERESTARTSYS.
Added some lessions learned to the description.
v11: Check old and new credentials in PTRACE_ATTACH again without
changing the API.
Note: I got actually one response from an automatic checker to the v11 patch,
https://lore.kernel.org/lkml/202107121344.wu68hEPF-lkp@intel.com/
which is complaining about:
>> >> kernel/ptrace.c:425:26: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct cred const *old_cred @@ got struct cred const [noderef] __rcu *real_cred @@
417 struct linux_binprm *bprm = task->signal->exec_bprm;
418 const struct cred *old_cred;
419 struct mm_struct *old_mm;
420
421 retval = down_write_killable(&task->signal->exec_update_lock);
422 if (retval)
423 goto unlock_creds;
424 task_lock(task);
> 425 old_cred = task->real_cred;
v12: Essentially identical to v11.
- Fixed a minor merge conflict in linux v5.17, and fixed the
above mentioned nit by adding __rcu to the declaration.
- re-tested the patch with all linux versions from v5.11 to v6.6
v10 was an alternative approach which did imply an API change.
But I would prefer to avoid such an API change.
The difficult part is getting the right dumpability flags assigned
before de_thread starts, hope you like this version.
If not, the v10 is of course also acceptable.
v13: Fixed duplicated Return section in function header of
is_dumpability_changed which was reported by the kernel test robot
v14: rebased to v6.7, refreshed and retested.
And added a more detailed description of the actual bug.
v15: rebased to v6.8-rc1, addressed some review comments.
Split the test case vmaccess into vmaccess1 and vmaccess2
to improve overall test coverage.
v16: rebased to 6.17-rc2, fixed some minor merge conflicts.
v17: avoid use of task->in_execve in ptrace_attach.
v18: Add some more comments, avoid the temporaty impersonation of the
new credentials, and use instead a new option to ptrace_may_access.
All security engines have to handle this option, but the advantage is
that the engines could detect and maybe also deny the unsafe execve.
Thanks
Bernd.
diff --git a/fs/exec.c b/fs/exec.c
index 4298e7e08d5d..02f3e8469125 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -905,11 +905,13 @@ static int exec_mmap(struct mm_struct *mm)
return 0;
}
-static int de_thread(struct task_struct *tsk)
+static int de_thread(struct task_struct *tsk, struct linux_binprm *bprm)
{
struct signal_struct *sig = tsk->signal;
struct sighand_struct *oldsighand = tsk->sighand;
spinlock_t *lock = &oldsighand->siglock;
+ struct task_struct *t;
+ bool unsafe_execve_in_progress = false;
if (thread_group_empty(tsk))
goto no_thread_group;
@@ -932,6 +934,36 @@ static int de_thread(struct task_struct *tsk)
if (!thread_group_leader(tsk))
sig->notify_count--;
+ for_other_threads(tsk, t) {
+ if (unlikely(t->ptrace) &&
+ (t != tsk->group_leader || !t->exit_state)) {
+ unsafe_execve_in_progress = true;
+ break;
+ }
+ }
+
+ if (unlikely(unsafe_execve_in_progress)) {
+ /*
+ * Since the spin lock was acquired while holding the
+ * mutex, both should be unlocked in reverse sequence and
+ * the spin lock re-acquired after releasing the mutex.
+ */
+ spin_unlock_irq(lock);
+ /*
+ * Sibling threads are notified by the non-zero exec_bprm,
+ * that they have just been zapped, and the cred_guard_mutex
+ * is to be released by them immediately.
+ * The caller of ptrace_attach on the other hand is allowed
+ * to ptrace any additional sibling threads that may not yet
+ * have ben ptraced, but if the group_exec_task is being
+ * ptraced, an additional check has to be performed, that the
+ * tracer is allowed to ptrace the new exec credentials.
+ */
+ sig->exec_bprm = bprm;
+ mutex_unlock(&sig->cred_guard_mutex);
+ spin_lock_irq(lock);
+ }
+
while (sig->notify_count) {
__set_current_state(TASK_KILLABLE);
spin_unlock_irq(lock);
@@ -1021,6 +1053,11 @@ static int de_thread(struct task_struct *tsk)
release_task(leader);
}
+ if (unlikely(unsafe_execve_in_progress)) {
+ mutex_lock(&sig->cred_guard_mutex);
+ sig->exec_bprm = NULL;
+ }
+
sig->group_exec_task = NULL;
sig->notify_count = 0;
@@ -1032,6 +1069,11 @@ static int de_thread(struct task_struct *tsk)
return 0;
killed:
+ if (unlikely(unsafe_execve_in_progress)) {
+ mutex_lock(&sig->cred_guard_mutex);
+ sig->exec_bprm = NULL;
+ }
+
/* protects against exit_notify() and __exit_signal() */
read_lock(&tasklist_lock);
sig->group_exec_task = NULL;
@@ -1114,13 +1156,31 @@ int begin_new_exec(struct linux_binprm * bprm)
*/
trace_sched_prepare_exec(current, bprm);
+ /* If the binary is not readable then enforce mm->dumpable=0 */
+ would_dump(bprm, bprm->file);
+ if (bprm->have_execfd)
+ would_dump(bprm, bprm->executable);
+
+ /*
+ * Figure out dumpability. Note that this checking only of current
+ * is wrong, but userspace depends on it. This should be testing
+ * bprm->secureexec instead.
+ */
+ if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
+ is_dumpability_changed(current_cred(), bprm->cred) ||
+ !(uid_eq(current_euid(), current_uid()) &&
+ gid_eq(current_egid(), current_gid())))
+ set_dumpable(bprm->mm, suid_dumpable);
+ else
+ set_dumpable(bprm->mm, SUID_DUMP_USER);
+
/*
* Ensure all future errors are fatal.
*/
bprm->point_of_no_return = true;
/* Make this the only thread in the thread group */
- retval = de_thread(me);
+ retval = de_thread(me, bprm);
if (retval)
goto out;
/* see the comment in check_unsafe_exec() */
@@ -1144,11 +1204,6 @@ int begin_new_exec(struct linux_binprm * bprm)
if (retval)
goto out;
- /* If the binary is not readable then enforce mm->dumpable=0 */
- would_dump(bprm, bprm->file);
- if (bprm->have_execfd)
- would_dump(bprm, bprm->executable);
-
/*
* Release all of the old mmap stuff
*/
@@ -1210,18 +1265,6 @@ int begin_new_exec(struct linux_binprm * bprm)
me->sas_ss_sp = me->sas_ss_size = 0;
- /*
- * Figure out dumpability. Note that this checking only of current
- * is wrong, but userspace depends on it. This should be testing
- * bprm->secureexec instead.
- */
- if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
- !(uid_eq(current_euid(), current_uid()) &&
- gid_eq(current_egid(), current_gid())))
- set_dumpable(current->mm, suid_dumpable);
- else
- set_dumpable(current->mm, SUID_DUMP_USER);
-
perf_event_exec();
/*
@@ -1275,6 +1318,10 @@ int begin_new_exec(struct linux_binprm * bprm)
* cred_guard_mutex must be held at least to this point to prevent
* ptrace_attach() from altering our determination of the task's
* credentials; any time after this it may be unlocked.
+ * Note that de_thread may temporarily release the cred_guard_mutex,
+ * but the credentials are pre-determined in that case and the ptrace
+ * access check guarantees, that the access permissions of the tracer
+ * are sufficient to trace the task also with the new credentials.
*/
security_bprm_committed_creds(bprm);
@@ -1361,6 +1408,7 @@ static int prepare_bprm_creds(struct linux_binprm *bprm)
if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
return -ERESTARTNOINTR;
+ /* It is not necessary to check current->signal->exec_bprm here. */
bprm->cred = prepare_exec_creds();
if (likely(bprm->cred))
return 0;
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 6299878e3d97..f554c2638ffb 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2838,6 +2838,18 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
if (rv < 0)
goto out_free;
+ /*
+ * A fatal signal is guaranteed to be already pending in the
+ * unlikely event, that current->signal->exec_bprm happens
+ * to be non-zero here, so just release the mutex again
+ * and continue as if mutex_lock_interruptible did fail.
+ */
+ if (unlikely(current->signal->exec_bprm)) {
+ mutex_unlock(¤t->signal->cred_guard_mutex);
+ rv = -ERESTARTNOINTR;
+ goto out_free;
+ }
+
rv = security_setprocattr(PROC_I(inode)->op.lsmid,
file->f_path.dentry->d_name.name, page,
count);
diff --git a/include/linux/cred.h b/include/linux/cred.h
index 89ae50ad2ace..4204c19de714 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -153,6 +153,7 @@ extern const struct cred *get_task_cred(struct task_struct *);
extern struct cred *cred_alloc_blank(void);
extern struct cred *prepare_creds(void);
extern struct cred *prepare_exec_creds(void);
+extern bool is_dumpability_changed(const struct cred *, const struct cred *);
extern int commit_creds(struct cred *);
extern void abort_creds(struct cred *);
extern struct cred *prepare_kernel_cred(struct task_struct *);
diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 90507d4afcd6..dbd58a4807bc 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -64,6 +64,7 @@ extern void exit_ptrace(struct task_struct *tracer, struct list_head *dead);
#define PTRACE_MODE_NOAUDIT 0x04
#define PTRACE_MODE_FSCREDS 0x08
#define PTRACE_MODE_REALCREDS 0x10
+#define PTRACE_MODE_BPRMCREDS 0x20
/* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */
#define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS)
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 7d6449982822..ade7d7173875 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -241,9 +241,27 @@ struct signal_struct {
struct mm_struct *oom_mm; /* recorded mm when the thread group got
* killed by the oom killer */
+ struct linux_binprm *exec_bprm; /* Used to check ptrace_may_access
+ * against new credentials while
+ * de_thread is waiting for other
+ * traced threads to terminate.
+ * Set while de_thread is executing.
+ * The cred_guard_mutex is released
+ * after de_thread() has called
+ * zap_other_threads(), therefore
+ * a fatal signal is guaranteed to be
+ * already pending in the unlikely
+ * event, that
+ * current->signal->exec_bprm happens
+ * to be non-zero after the
+ * cred_guard_mutex was acquired.
+ */
+
struct mutex cred_guard_mutex; /* guard against foreign influences on
* credential calculations
* (notably. ptrace)
+ * Held while execve runs, except when
+ * a sibling thread is being traced.
* Deprecated do not use in new code.
* Use exec_update_lock instead.
*/
diff --git a/kernel/cred.c b/kernel/cred.c
index dbf6b687dc5c..69fd0de813c0 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -375,6 +375,30 @@ static bool cred_cap_issubset(const struct cred *set, const struct cred *subset)
return false;
}
+/**
+ * is_dumpability_changed - Will changing creds affect dumpability?
+ * @old: The old credentials.
+ * @new: The new credentials.
+ *
+ * If the @new credentials have no elevated privileges compared to the
+ * @old credentials, the task may remain dumpable. Otherwise we have
+ * to mark the task as undumpable to avoid information leaks from higher
+ * to lower privilege domains.
+ *
+ * Return: True if the task will become undumpable.
+ */
+bool is_dumpability_changed(const struct cred *old, const struct cred *new)
+{
+ if (!uid_eq(old->euid, new->euid) ||
+ !gid_eq(old->egid, new->egid) ||
+ !uid_eq(old->fsuid, new->fsuid) ||
+ !gid_eq(old->fsgid, new->fsgid) ||
+ !cred_cap_issubset(old, new))
+ return true;
+
+ return false;
+}
+
/**
* commit_creds - Install new credentials upon the current task
* @new: The credentials to be assigned
@@ -403,11 +427,7 @@ int commit_creds(struct cred *new)
get_cred(new); /* we will require a ref for the subj creds too */
/* dumpability changes */
- if (!uid_eq(old->euid, new->euid) ||
- !gid_eq(old->egid, new->egid) ||
- !uid_eq(old->fsuid, new->fsuid) ||
- !gid_eq(old->fsgid, new->fsgid) ||
- !cred_cap_issubset(old, new)) {
+ if (is_dumpability_changed(old, new)) {
if (task->mm)
set_dumpable(task->mm, suid_dumpable);
task->pdeath_signal = 0;
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 75a84efad40f..ac750d1ccd04 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -20,6 +20,7 @@
#include <linux/pagemap.h>
#include <linux/ptrace.h>
#include <linux/security.h>
+#include <linux/binfmts.h>
#include <linux/signal.h>
#include <linux/uio.h>
#include <linux/audit.h>
@@ -285,6 +286,11 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
return -EPERM;
}
+ if ((mode & PTRACE_MODE_BPRMCREDS) && !task->signal->exec_bprm) {
+ WARN(1, "denying ptrace access check with PTRACE_MODE_BPRMCREDS\n");
+ return -EPERM;
+ }
+
/* May we inspect the given task?
* This check is used both for attaching with ptrace
* and for allowing access to sensitive information in /proc.
@@ -313,7 +319,10 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
caller_uid = cred->uid;
caller_gid = cred->gid;
}
- tcred = __task_cred(task);
+ if (mode & PTRACE_MODE_BPRMCREDS)
+ tcred = task->signal->exec_bprm->cred;
+ else
+ tcred = __task_cred(task);
if (uid_eq(caller_uid, tcred->euid) &&
uid_eq(caller_uid, tcred->suid) &&
uid_eq(caller_uid, tcred->uid) &&
@@ -337,7 +346,10 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
* Pairs with a write barrier in commit_creds().
*/
smp_rmb();
- mm = task->mm;
+ if (mode & PTRACE_MODE_BPRMCREDS)
+ mm = task->signal->exec_bprm->mm;
+ else
+ mm = task->mm;
if (mm &&
((get_dumpable(mm) != SUID_DUMP_USER) &&
!ptrace_has_cap(mm->user_ns, mode)))
@@ -451,6 +463,14 @@ static int ptrace_attach(struct task_struct *task, long request,
retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
if (retval)
return retval;
+
+ if (unlikely(task == task->signal->group_exec_task)) {
+ retval = __ptrace_may_access(task,
+ PTRACE_MODE_ATTACH_REALCREDS |
+ PTRACE_MODE_BPRMCREDS);
+ if (retval)
+ return retval;
+ }
}
scoped_guard (write_lock_irq, &tasklist_lock) {
@@ -488,6 +508,10 @@ static int ptrace_traceme(void)
{
int ret = -EPERM;
+ if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
+ return -ERESTARTNOINTR;
+
+ /* It is not necessary to check current->signal->exec_bprm here. */
write_lock_irq(&tasklist_lock);
/* Are we already being traced? */
if (!current->ptrace) {
@@ -503,6 +527,7 @@ static int ptrace_traceme(void)
}
}
write_unlock_irq(&tasklist_lock);
+ mutex_unlock(¤t->signal->cred_guard_mutex);
return ret;
}
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 25f62867a16d..6e4ff108faa0 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -2010,9 +2010,21 @@ static long seccomp_set_mode_filter(unsigned int flags,
* Make sure we cannot change seccomp or nnp state via TSYNC
* while another thread is in the middle of calling exec.
*/
- if (flags & SECCOMP_FILTER_FLAG_TSYNC &&
- mutex_lock_killable(¤t->signal->cred_guard_mutex))
- goto out_put_fd;
+ if (flags & SECCOMP_FILTER_FLAG_TSYNC) {
+ if (mutex_lock_killable(¤t->signal->cred_guard_mutex))
+ goto out_put_fd;
+
+ /*
+ * A fatal signal is guaranteed to be already pending in the
+ * unlikely event, that current->signal->exec_bprm happens
+ * to be non-zero here, so just release the mutex again
+ * and continue as if mutex_lock_killable did fail.
+ */
+ if (unlikely(current->signal->exec_bprm)) {
+ mutex_unlock(¤t->signal->cred_guard_mutex);
+ goto out_put_fd;
+ }
+ }
spin_lock_irq(¤t->sighand->siglock);
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index b3f7a3258a2c..80c2cd968f05 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -129,7 +129,10 @@ static int apparmor_ptrace_access_check(struct task_struct *child,
int error;
bool needput;
- cred = get_task_cred(child);
+ if (mode & PTRACE_MODE_BPRMCREDS)
+ cred = get_cred(child->signal->exec_bprm->cred);
+ else
+ cred = get_task_cred(child);
tracee = cred_label(cred); /* ref count on cred */
tracer = __begin_current_label_crit_section(&needput);
error = aa_may_ptrace(current_cred(), tracer, cred, tracee,
diff --git a/security/commoncap.c b/security/commoncap.c
index 6bd4adeb4795..e23a78a4514c 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -169,7 +169,10 @@ int cap_ptrace_access_check(struct task_struct *child, unsigned int mode)
rcu_read_lock();
cred = current_cred();
- child_cred = __task_cred(child);
+ if (mode & PTRACE_MODE_BPRMCREDS)
+ child_cred = child->signal->exec_bprm->cred;
+ else
+ child_cred = __task_cred(child);
if (mode & PTRACE_MODE_FSCREDS)
caller_caps = &cred->cap_effective;
else
diff --git a/security/landlock/task.c b/security/landlock/task.c
index 2385017418ca..46ce9b6e4728 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -17,6 +17,7 @@
#include <linux/rcupdate.h>
#include <linux/sched.h>
#include <linux/sched/signal.h>
+#include <linux/binfmts.h>
#include <net/af_unix.h>
#include <net/sock.h>
@@ -96,7 +97,11 @@ static int hook_ptrace_access_check(struct task_struct *const child,
scoped_guard(rcu)
{
- child_dom = landlock_get_task_domain(child);
+ if (mode & PTRACE_MODE_BPRMCREDS)
+ child_dom = landlock_cred(child->signal->
+ exec_bprm->cred)->domain;
+ else
+ child_dom = landlock_get_task_domain(child);
err = domain_ptrace(parent_subject->domain, child_dom);
}
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index dfc22da42f30..016e21180e96 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2111,7 +2111,12 @@ static int selinux_ptrace_access_check(struct task_struct *child,
unsigned int mode)
{
u32 sid = current_sid();
- u32 csid = task_sid_obj(child);
+ u32 csid;
+
+ if (mode & PTRACE_MODE_BPRMCREDS)
+ csid = cred_sid(child->signal->exec_bprm->cred);
+ else
+ csid = task_sid_obj(child);
if (mode & PTRACE_MODE_READ)
return avc_has_perm(sid, csid, SECCLASS_FILE, FILE__READ,
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index af986587841d..5c2ca49baa05 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -476,7 +476,10 @@ static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode)
{
struct smack_known *skp;
- skp = smk_of_task_struct_obj(ctp);
+ if (mode & PTRACE_MODE_BPRMCREDS)
+ skp = smk_of_task(smack_cred(ctp->signal->exec_bprm->cred));
+ else
+ skp = smk_of_task_struct_obj(ctp);
return smk_ptrace_rule_check(current, skp, mode, __func__);
}
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index 3d064dd4e03f..8ac9ce41d4f0 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -11,6 +11,7 @@
#include <linux/lsm_hooks.h>
#include <linux/sysctl.h>
#include <linux/ptrace.h>
+#include <linux/binfmts.h>
#include <linux/prctl.h>
#include <linux/ratelimit.h>
#include <linux/workqueue.h>
@@ -363,13 +364,19 @@ static int yama_ptrace_access_check(struct task_struct *child,
rc = -EPERM;
if (!rc && !task_is_descendant(current, child) &&
!ptracer_exception_found(current, child) &&
- !ns_capable(__task_cred(child)->user_ns, CAP_SYS_PTRACE))
+ !ns_capable(mode & PTRACE_MODE_BPRMCREDS ?
+ child->signal->exec_bprm->cred->user_ns :
+ __task_cred(child)->user_ns,
+ CAP_SYS_PTRACE))
rc = -EPERM;
rcu_read_unlock();
break;
case YAMA_SCOPE_CAPABILITY:
rcu_read_lock();
- if (!ns_capable(__task_cred(child)->user_ns, CAP_SYS_PTRACE))
+ if (!ns_capable(mode & PTRACE_MODE_BPRMCREDS ?
+ child->signal->exec_bprm->cred->user_ns :
+ __task_cred(child)->user_ns,
+ CAP_SYS_PTRACE))
rc = -EPERM;
rcu_read_unlock();
break;
diff --git a/tools/testing/selftests/ptrace/vmaccess.c b/tools/testing/selftests/ptrace/vmaccess.c
index 4db327b44586..5d4a65eb5a8d 100644
--- a/tools/testing/selftests/ptrace/vmaccess.c
+++ b/tools/testing/selftests/ptrace/vmaccess.c
@@ -14,6 +14,7 @@
#include <signal.h>
#include <unistd.h>
#include <sys/ptrace.h>
+#include <sys/syscall.h>
static void *thread(void *arg)
{
@@ -23,7 +24,7 @@ static void *thread(void *arg)
TEST(vmaccess)
{
- int f, pid = fork();
+ int s, f, pid = fork();
char mm[64];
if (!pid) {
@@ -31,19 +32,42 @@ TEST(vmaccess)
pthread_create(&pt, NULL, thread, NULL);
pthread_join(pt, NULL);
- execlp("true", "true", NULL);
+ execlp("false", "false", NULL);
+ return;
}
sleep(1);
sprintf(mm, "/proc/%d/mem", pid);
+ /* deadlock did happen here */
f = open(mm, O_RDONLY);
ASSERT_GE(f, 0);
close(f);
- f = kill(pid, SIGCONT);
- ASSERT_EQ(f, 0);
+ f = waitpid(-1, &s, WNOHANG);
+ ASSERT_NE(f, -1);
+ ASSERT_NE(f, 0);
+ ASSERT_NE(f, pid);
+ ASSERT_EQ(WIFEXITED(s), 1);
+ ASSERT_EQ(WEXITSTATUS(s), 0);
+ f = waitpid(-1, &s, 0);
+ ASSERT_EQ(f, pid);
+ ASSERT_EQ(WIFEXITED(s), 1);
+ ASSERT_EQ(WEXITSTATUS(s), 1);
+ f = waitpid(-1, NULL, 0);
+ ASSERT_EQ(f, -1);
+ ASSERT_EQ(errno, ECHILD);
}
-TEST(attach)
+/*
+ * Same test as previous, except that
+ * we try to ptrace the group leader,
+ * which is about to call execve,
+ * when the other thread is already ptraced.
+ * This exercises the code in de_thread
+ * where it is waiting inside the
+ * while (sig->notify_count) {
+ * loop.
+ */
+TEST(attach1)
{
int s, k, pid = fork();
@@ -52,19 +76,76 @@ TEST(attach)
pthread_create(&pt, NULL, thread, NULL);
pthread_join(pt, NULL);
- execlp("sleep", "sleep", "2", NULL);
+ execlp("false", "false", NULL);
+ return;
}
sleep(1);
+ /* deadlock may happen here */
k = ptrace(PTRACE_ATTACH, pid, 0L, 0L);
- ASSERT_EQ(errno, EAGAIN);
- ASSERT_EQ(k, -1);
+ ASSERT_EQ(k, 0);
k = waitpid(-1, &s, WNOHANG);
ASSERT_NE(k, -1);
ASSERT_NE(k, 0);
ASSERT_NE(k, pid);
ASSERT_EQ(WIFEXITED(s), 1);
ASSERT_EQ(WEXITSTATUS(s), 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGTRAP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGSTOP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFEXITED(s), 1);
+ ASSERT_EQ(WEXITSTATUS(s), 1);
+ k = waitpid(-1, NULL, 0);
+ ASSERT_EQ(k, -1);
+ ASSERT_EQ(errno, ECHILD);
+}
+
+/*
+ * Same test as previous, except that
+ * the group leader is ptraced first,
+ * but this time with PTRACE_O_TRACEEXIT,
+ * and the thread that does execve is
+ * not yet ptraced. This exercises the
+ * code block in de_thread where the
+ * if (!thread_group_leader(tsk)) {
+ * is executed and enters a wait state.
+ */
+static long thread2_tid;
+static void *thread2(void *arg)
+{
+ thread2_tid = syscall(__NR_gettid);
+ sleep(2);
+ execlp("false", "false", NULL);
+ return NULL;
+}
+
+TEST(attach2)
+{
+ int s, k, pid = fork();
+
+ if (!pid) {
+ pthread_t pt;
+
+ pthread_create(&pt, NULL, thread2, NULL);
+ pthread_join(pt, NULL);
+ return;
+ }
+
sleep(1);
k = ptrace(PTRACE_ATTACH, pid, 0L, 0L);
ASSERT_EQ(k, 0);
@@ -72,12 +153,46 @@ TEST(attach)
ASSERT_EQ(k, pid);
ASSERT_EQ(WIFSTOPPED(s), 1);
ASSERT_EQ(WSTOPSIG(s), SIGSTOP);
- k = ptrace(PTRACE_DETACH, pid, 0L, 0L);
+ k = ptrace(PTRACE_SETOPTIONS, pid, 0L, PTRACE_O_TRACEEXIT);
+ ASSERT_EQ(k, 0);
+ thread2_tid = ptrace(PTRACE_PEEKDATA, pid, &thread2_tid, 0L);
+ ASSERT_NE(thread2_tid, -1);
+ ASSERT_NE(thread2_tid, 0);
+ ASSERT_NE(thread2_tid, pid);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ sleep(2);
+ /* deadlock may happen here */
+ k = ptrace(PTRACE_ATTACH, thread2_tid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGTRAP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGTRAP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGSTOP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
ASSERT_EQ(k, 0);
k = waitpid(-1, &s, 0);
ASSERT_EQ(k, pid);
ASSERT_EQ(WIFEXITED(s), 1);
- ASSERT_EQ(WEXITSTATUS(s), 0);
+ ASSERT_EQ(WEXITSTATUS(s), 1);
k = waitpid(-1, NULL, 0);
ASSERT_EQ(k, -1);
ASSERT_EQ(errno, ECHILD);
--
2.39.5
^ permalink raw reply related
* Re: [PATCH] Clarify the rootid_owns_currentns
From: Paul Moore @ 2025-11-18 15:47 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: lkml, linux-security-module, Ryan Foster, Christian Brauner
In-Reply-To: <aRx/1MvvBqu5MhKv@mail.hallyn.com>
On Tue, Nov 18, 2025 at 9:16 AM Serge E. Hallyn <serge@hallyn.com> wrote:
> On Fri, Nov 14, 2025 at 03:33:19PM -0600, Serge E. Hallyn wrote:
> > Split most of the rootid_owns_currentns() functionality
> > into a more generic rootid_owns_ns() function which
> > will be easier to write tests for.
> >
> > Rename the functions and variables to make clear that
> > the ids being tested could be any uid.
> >
> > Signed-off-by: Serge Hallyn <serge@hallyn.com>
> > CC: Ryan Foster <foster.ryan.r@gmail.com>
> > CC: Christian Brauner <brauner@kernel.org>
>
> Paul, Christian, let me know if you have any objections, else I will
> queue this up in caps-next.
Seems reasonable to me, but it would be good to fix the parameter doc
bug that the kernel test robot identified. I suspect it is just the
extra vertical comment space between the top one line summary and the
parameter list.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] Clarify the rootid_owns_currentns
From: Serge E. Hallyn @ 2025-11-18 14:16 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: lkml, linux-security-module, Paul Moore, Ryan Foster,
Christian Brauner
In-Reply-To: <aRegH8P4cPlzzlX9@mail.hallyn.com>
On Fri, Nov 14, 2025 at 03:33:19PM -0600, Serge E. Hallyn wrote:
> Split most of the rootid_owns_currentns() functionality
> into a more generic rootid_owns_ns() function which
> will be easier to write tests for.
>
> Rename the functions and variables to make clear that
> the ids being tested could be any uid.
>
> Signed-off-by: Serge Hallyn <serge@hallyn.com>
> CC: Ryan Foster <foster.ryan.r@gmail.com>
> CC: Christian Brauner <brauner@kernel.org>
Paul, Christian, let me know if you have any objections, else I will
queue this up in caps-next.
Ryan, based on this you would be able to do more useful unit ktests:
you could create some simple user namespaces with mappings which do
or do not have uid 0 in ns mapped to the kuid you are querying.
> ---
> security/commoncap.c | 35 +++++++++++++++++++++++------------
> 1 file changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 6bd4adeb4795..8a81fdc12cbe 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -358,17 +358,18 @@ int cap_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry)
> return error;
> }
>
> -static bool rootid_owns_currentns(vfsuid_t rootvfsuid)
> +/**
> + * kuid_root_in_ns - check whether the given kuid is root in the given ns
> + *
> + * @kuid - the kuid to be tested
> + * @ns - the user namespace to test against
> + *
> + * Returns true if @kuid represents the root user in @ns, false otherwise.
> + */
> +static bool kuid_root_in_ns(kuid_t kuid, struct user_namespace *ns)
> {
> - struct user_namespace *ns;
> - kuid_t kroot;
> -
> - if (!vfsuid_valid(rootvfsuid))
> - return false;
> -
> - kroot = vfsuid_into_kuid(rootvfsuid);
> - for (ns = current_user_ns();; ns = ns->parent) {
> - if (from_kuid(ns, kroot) == 0)
> + for (;; ns = ns->parent) {
> + if (from_kuid(ns, kuid) == 0)
> return true;
> if (ns == &init_user_ns)
> break;
> @@ -377,6 +378,16 @@ static bool rootid_owns_currentns(vfsuid_t rootvfsuid)
> return false;
> }
>
> +static bool vfsuid_root_in_currentns(vfsuid_t vfsuid)
> +{
> + kuid_t kuid;
> +
> + if (!vfsuid_valid(vfsuid))
> + return false;
> + kuid = vfsuid_into_kuid(vfsuid);
> + return kuid_root_in_ns(kuid, current_user_ns());
> +}
> +
> static __u32 sansflags(__u32 m)
> {
> return m & ~VFS_CAP_FLAGS_EFFECTIVE;
> @@ -481,7 +492,7 @@ int cap_inode_getsecurity(struct mnt_idmap *idmap,
> goto out_free;
> }
>
> - if (!rootid_owns_currentns(vfsroot)) {
> + if (!vfsuid_root_in_currentns(vfsroot)) {
> size = -EOVERFLOW;
> goto out_free;
> }
> @@ -722,7 +733,7 @@ int get_vfs_caps_from_disk(struct mnt_idmap *idmap,
> /* Limit the caps to the mounter of the filesystem
> * or the more limited uid specified in the xattr.
> */
> - if (!rootid_owns_currentns(rootvfsuid))
> + if (!vfsuid_root_in_currentns(rootvfsuid))
> return -ENODATA;
>
> cpu_caps->permitted.val = le32_to_cpu(caps->data[0].permitted);
> --
> 2.34.1
>
^ permalink raw reply
* [RFC PATCH v4 13/19] selftests/landlock: Test SCTP peeloff restriction
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
It is possible to branch off an SCTP UDP association into a separate
user space UDP socket. Add test validating that such scenario is
restricted by Landlock.
Move setup_loopback() helper from net_test to common.h to use it to
enable connection in this test.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Restricts branching off SCTP association.
* Changes commit message.
* Chages fixture from socket_create to mini.
---
tools/testing/selftests/landlock/common.h | 13 ++
tools/testing/selftests/landlock/net_test.c | 11 --
.../testing/selftests/landlock/socket_test.c | 128 ++++++++++++++++++
3 files changed, 141 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/landlock/common.h b/tools/testing/selftests/landlock/common.h
index e9378a229a4c..ff1b7fc94a5b 100644
--- a/tools/testing/selftests/landlock/common.h
+++ b/tools/testing/selftests/landlock/common.h
@@ -16,6 +16,7 @@
#include <sys/un.h>
#include <sys/wait.h>
#include <unistd.h>
+#include <sched.h>
#include "../kselftest_harness.h"
#include "wrappers.h"
@@ -255,3 +256,15 @@ static void __maybe_unused set_unix_address(struct service_fixture *const srv,
srv->unix_addr_len = SUN_LEN(&srv->unix_addr);
srv->unix_addr.sun_path[0] = '\0';
}
+
+static void __maybe_unused
+setup_loopback(struct __test_metadata *const _metadata)
+{
+ set_cap(_metadata, CAP_SYS_ADMIN);
+ ASSERT_EQ(0, unshare(CLONE_NEWNET));
+ clear_cap(_metadata, CAP_SYS_ADMIN);
+
+ set_ambient_cap(_metadata, CAP_NET_ADMIN);
+ ASSERT_EQ(0, system("ip link set dev lo up"));
+ clear_ambient_cap(_metadata, CAP_NET_ADMIN);
+}
diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
index 2a45208551e6..a71ea275cf10 100644
--- a/tools/testing/selftests/landlock/net_test.c
+++ b/tools/testing/selftests/landlock/net_test.c
@@ -75,17 +75,6 @@ static int set_service(struct service_fixture *const srv,
return 1;
}
-static void setup_loopback(struct __test_metadata *const _metadata)
-{
- set_cap(_metadata, CAP_SYS_ADMIN);
- ASSERT_EQ(0, unshare(CLONE_NEWNET));
- clear_cap(_metadata, CAP_SYS_ADMIN);
-
- set_ambient_cap(_metadata, CAP_NET_ADMIN);
- ASSERT_EQ(0, system("ip link set dev lo up"));
- clear_ambient_cap(_metadata, CAP_NET_ADMIN);
-}
-
static bool prot_is_tcp(const struct protocol_variant *const prot)
{
return (prot->domain == AF_INET || prot->domain == AF_INET6) &&
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index d1a004c2e0f5..e9f56a86f456 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -12,6 +12,10 @@
#include <linux/pfkeyv2.h>
#include <linux/kcm.h>
#include <linux/can.h>
+#include <sys/socket.h>
+#include <stdint.h>
+#include <linux/sctp.h>
+#include <arpa/inet.h>
#include "common.h"
@@ -921,4 +925,128 @@ TEST_F(mini, socketpair)
EXPECT_EQ(EACCES, test_socketpair(AF_UNIX, SOCK_STREAM, 0));
}
+/* clang-format off */
+FIXTURE(connection_restriction) {};
+/* clang-format on */
+
+FIXTURE_VARIANT(connection_restriction)
+{
+ bool sandboxed;
+};
+
+FIXTURE_SETUP(connection_restriction)
+{
+ disable_caps(_metadata);
+ setup_loopback(_metadata);
+};
+
+FIXTURE_TEARDOWN(connection_restriction)
+{
+}
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(connection_restriction, allowed) {
+ /* clang-format on */
+ .sandboxed = false,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(connection_restriction, sandboxed) {
+ /* clang-format on */
+ .sandboxed = true,
+};
+
+static const char loopback_ipv4[] = "127.0.0.1";
+static const int backlog = 10;
+static const int loopback_port = 1024;
+
+TEST_F(connection_restriction, sctp_peeloff)
+{
+ int status, ret;
+ pid_t child;
+ struct sockaddr_in addr;
+ int server_fd;
+
+ server_fd =
+ socket(AF_INET, SOCK_SEQPACKET | SOCK_CLOEXEC, IPPROTO_SCTP);
+ ASSERT_LE(0, server_fd);
+
+ addr.sin_family = AF_INET;
+ addr.sin_port = htons(loopback_port);
+ addr.sin_addr.s_addr = inet_addr(loopback_ipv4);
+
+ ASSERT_EQ(0, bind(server_fd, &addr, sizeof(addr)));
+ ASSERT_EQ(0, listen(server_fd, backlog));
+
+ child = fork();
+ ASSERT_LE(0, child);
+ if (child == 0) {
+ int client_fd;
+ sctp_peeloff_flags_arg_t peeloff;
+ socklen_t peeloff_size = sizeof(peeloff);
+
+ /* Closes listening socket for the child. */
+ ASSERT_EQ(0, close(server_fd));
+
+ client_fd = socket(AF_INET, SOCK_SEQPACKET | SOCK_CLOEXEC,
+ IPPROTO_SCTP);
+ ASSERT_LE(0, client_fd);
+
+ /*
+ * Establishes connection between sockets and
+ * gets SCTP association id.
+ */
+ ret = setsockopt(client_fd, IPPROTO_SCTP, SCTP_SOCKOPT_CONNECTX,
+ &addr, sizeof(addr));
+ ASSERT_LE(0, ret);
+
+ if (variant->sandboxed) {
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket =
+ LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ /* Denies creation of SCTP sockets. */
+ int ruleset_fd = landlock_create_ruleset(
+ &ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+ }
+ /*
+ * Branches off current SCTP association into a separate socket
+ * and returns it to user space.
+ */
+ peeloff.p_arg.associd = ret;
+ ret = getsockopt(client_fd, IPPROTO_SCTP, SCTP_SOCKOPT_PEELOFF,
+ &peeloff, &peeloff_size);
+
+ /*
+ * Branching off existing SCTP association leads to creation of user space
+ * SCTP UDP socket and should be restricted by Landlock.
+ */
+ if (variant->sandboxed) {
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(EACCES, errno);
+ } else {
+ ASSERT_LE(0, ret);
+ }
+
+ /* getsockopt(2) returns 0 on success. */
+ if (ret == 0) {
+ /* Closes peeloff socket if such was created. */
+ ASSERT_EQ(0, close(peeloff.p_arg.sd));
+ }
+ ASSERT_EQ(0, close(client_fd));
+ _exit(_metadata->exit_code);
+ return;
+ }
+
+ ASSERT_EQ(child, waitpid(child, &status, 0));
+ ASSERT_EQ(1, WIFEXITED(status));
+ ASSERT_EQ(EXIT_SUCCESS, WEXITSTATUS(status));
+
+ ASSERT_EQ(0, close(server_fd));
+}
+
TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 19/19] landlock: Document socket rule type support
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Extend documentation with socket rule type description.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Fixes identantion.
---
Documentation/userspace-api/landlock.rst | 48 ++++++++++++++++++++----
1 file changed, 41 insertions(+), 7 deletions(-)
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 1d0c2c15c22e..49fdc897db24 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -8,7 +8,7 @@ Landlock: unprivileged access control
=====================================
:Author: Mickaël Salaün
-:Date: March 2025
+:Date: November 2025
The goal of Landlock is to enable restriction of ambient rights (e.g. global
filesystem or network access) for a set of processes. Because Landlock
@@ -33,7 +33,7 @@ A Landlock rule describes an action on an object which the process intends to
perform. A set of rules is aggregated in a ruleset, which can then restrict
the thread enforcing it, and its future children.
-The two existing types of rules are:
+The three existing types of rules are:
Filesystem rules
For these rules, the object is a file hierarchy,
@@ -44,14 +44,18 @@ Network rules (since ABI v4)
For these rules, the object is a TCP port,
and the related actions are defined with `network access rights`.
+Socket rules (since ABI v8)
+ For these rules, the object is a pair of an address family and a socket type,
+ and the related actions are defined with `socket access rights`.
+
Defining and enforcing a security policy
----------------------------------------
We first need to define the ruleset that will contain our rules.
For this example, the ruleset will contain rules that only allow filesystem
-read actions and establish a specific TCP connection. Filesystem write
-actions and other TCP actions will be denied.
+read actions, create TCP sockets and establish a specific TCP connection.
+Filesystem write actions, non-TCP sockets creation other TCP actions will be denied.
The ruleset then needs to handle both these kinds of actions. This is
required for backward and forward compatibility (i.e. the kernel and user
@@ -81,6 +85,8 @@ to be explicit about the denied-by-default access rights.
.handled_access_net =
LANDLOCK_ACCESS_NET_BIND_TCP |
LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ .handled_access_socket =
+ LANDLOCK_ACCESS_SOCKET_CREATE,
.scoped =
LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
LANDLOCK_SCOPE_SIGNAL,
@@ -127,6 +133,11 @@ version, and only use the available subset of access rights:
/* Removes LANDLOCK_SCOPE_* for ABI < 6 */
ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
LANDLOCK_SCOPE_SIGNAL);
+ case 6:
+ case 7:
+ /* Removes LANDLOCK_ACCESS_SOCKET for ABI < 8 */
+ ruleset_attr.handled_access_socket &=
+ ~LANDLOCK_ACCESS_SOCKET_CREATE;
}
This enables the creation of an inclusive ruleset that will contain our rules.
@@ -178,6 +189,21 @@ for the ruleset creation, by filtering access rights according to the Landlock
ABI version. In this example, this is not required because all of the requested
``allowed_access`` rights are already available in ABI 1.
+For socket access-control, we can add a rule to allow TCP sockets creation. UNIX,
+UDP/IP and other protocols will be denied by the ruleset.
+
+.. code-block:: c
+
+ struct landlock_net_port_attr tcp_socket = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = AF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ };
+
+ err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &tcp_socket, 0);
+
For network access-control, we can add a set of rules that allow to use a port
number for a specific action: HTTPS connections.
@@ -194,7 +220,8 @@ number for a specific action: HTTPS connections.
The next step is to restrict the current thread from gaining more privileges
(e.g. through a SUID binary). We now have a ruleset with the first rule
allowing read access to ``/usr`` while denying all other handled accesses for
-the filesystem, and a second rule allowing HTTPS connections.
+the filesystem, a second rule allowing TCP sockets and a third rule allowing
+HTTPS connections.
.. code-block:: c
@@ -442,7 +469,7 @@ Access rights
-------------
.. kernel-doc:: include/uapi/linux/landlock.h
- :identifiers: fs_access net_access scope
+ :identifiers: fs_access net_access socket_access scope
Creating a new ruleset
----------------------
@@ -461,7 +488,7 @@ Extending a ruleset
.. kernel-doc:: include/uapi/linux/landlock.h
:identifiers: landlock_rule_type landlock_path_beneath_attr
- landlock_net_port_attr
+ landlock_net_port_attr landlock_socket_attr
Enforcing a ruleset
-------------------
@@ -604,6 +631,13 @@ Landlock audit events with the ``LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF``,
sys_landlock_restrict_self(). See Documentation/admin-guide/LSM/landlock.rst
for more details on audit.
+Socket support (ABI < 8)
+-------------------------
+
+Starting with the Landlock ABI version 8, it is now possible to restrict
+creation of user space sockets to only a set of allowed protocols thanks
+to the new ``LANDLOCK_ACCESS_SOCKET_CREATE`` access right.
+
.. _kernel_support:
Kernel support
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 17/19] selftests/landlock: Test socket creation denial log for audit
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Test single socket blocker: socket.create.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
.../testing/selftests/landlock/socket_test.c | 55 +++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index ea1590e555b7..a091b8a883c8 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -17,6 +17,7 @@
#include <linux/sctp.h>
#include <arpa/inet.h>
+#include "audit.h"
#include "common.h"
#define ACCESS_LAST LANDLOCK_ACCESS_SOCKET_CREATE
@@ -1111,4 +1112,58 @@ TEST_F(connection_restriction, accept)
ASSERT_EQ(0, close(client_fd));
}
+FIXTURE(audit)
+{
+ struct audit_filter audit_filter;
+ int audit_fd;
+};
+
+FIXTURE_SETUP(audit)
+{
+ set_cap(_metadata, CAP_AUDIT_CONTROL);
+ self->audit_fd = audit_init_with_exe_filter(&self->audit_filter);
+ EXPECT_LE(0, self->audit_fd);
+ disable_caps(_metadata);
+};
+
+FIXTURE_TEARDOWN(audit)
+{
+ set_cap(_metadata, CAP_AUDIT_CONTROL);
+ EXPECT_EQ(0, audit_cleanup(self->audit_fd, &self->audit_filter));
+ clear_cap(_metadata, CAP_AUDIT_CONTROL);
+}
+
+TEST_F(audit, socket_create)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ struct audit_records records;
+ int ruleset_fd;
+ const char log_template[] = REGEX_LANDLOCK_PREFIX
+ " blockers=socket.create family=%d sock_type=%d protocol=0$";
+ /* Family and type should not exceed 2-digit number. */
+ char log_match[sizeof(log_template) + 4];
+ int log_match_len;
+
+ log_match_len = snprintf(log_match, sizeof(log_match), log_template,
+ AF_INET, SOCK_STREAM);
+ ASSERT_LT(log_match_len, sizeof(log_match));
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ ASSERT_EQ(EACCES, test_socket(AF_INET, SOCK_STREAM, 0));
+
+ EXPECT_EQ(0, audit_match_record(self->audit_fd, AUDIT_LANDLOCK_ACCESS,
+ log_match, NULL));
+
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+ EXPECT_EQ(0, records.access);
+ EXPECT_EQ(1, records.domain);
+}
+
TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 15/19] lsm: Support logging socket common data
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add LSM_AUDIT_DATA_SOCKET type to log socket-related data in
audit_log_lsm_data(). This may be useful (for example) to log socket
creation denials.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
include/linux/lsm_audit.h | 8 ++++++++
security/lsm_audit.c | 4 ++++
2 files changed, 12 insertions(+)
diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
index 382c56a97bba..7c7617df41b5 100644
--- a/include/linux/lsm_audit.h
+++ b/include/linux/lsm_audit.h
@@ -57,6 +57,12 @@ struct lsm_ibendport_audit {
u8 port;
};
+struct lsm_socket_audit {
+ s32 family;
+ s32 type;
+ s32 protocol;
+};
+
/* Auxiliary data to use in generating the audit record. */
struct common_audit_data {
char type;
@@ -78,6 +84,7 @@ struct common_audit_data {
#define LSM_AUDIT_DATA_NOTIFICATION 16
#define LSM_AUDIT_DATA_ANONINODE 17
#define LSM_AUDIT_DATA_NLMSGTYPE 18
+#define LSM_AUDIT_DATA_SOCKET 19
union {
struct path path;
struct dentry *dentry;
@@ -97,6 +104,7 @@ struct common_audit_data {
struct file *file;
struct lsm_ibpkey_audit *ibpkey;
struct lsm_ibendport_audit *ibendport;
+ struct lsm_socket_audit *socket;
int reason;
const char *anonclass;
u16 nlmsg_type;
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index 7d623b00495c..7e18241290ce 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -403,6 +403,10 @@ void audit_log_lsm_data(struct audit_buffer *ab,
case LSM_AUDIT_DATA_NLMSGTYPE:
audit_log_format(ab, " nl-msgtype=%hu", a->u.nlmsg_type);
break;
+ case LSM_AUDIT_DATA_SOCKET:
+ audit_log_format(ab, " family=%d sock_type=%d protocol=%d",
+ a->u.socket->family, a->u.socket->type, a->u.socket->protocol);
+ break;
} /* switch (a->type) */
}
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 18/19] samples/landlock: Support socket protocol restrictions
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add socket protocol control support in sandboxer demo. It's possible
to allow a sandboxer to create sockets with specified family and type
values. This is controlled with the new LL_SOCKET_CREATE environment
variable. Single token in this variable looks like this:
'{family}.{type}.{protocol}', where {family}, {type} and {protocol} are
integers corresponding to requested protocol definition.
Change LANDLOCK_ABI_LAST to 8.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Changes ABI from 6 to 8.
* Adds protocol field support.
* Fixes commit message.
* Minor fixes.
Changes since v2:
* Changes representation of socket protocol in LL_SOCKET_CREATE into
pair of integer values.
* Changes commit message.
* Minor fixes.
Changes since v1:
* Refactors get_socket_protocol(). Rename it to parse_socket_protocol().
* Changes LANDLOCK_ABI_LAST to 6 since ioctl patchlist updated it to 5.
* Refactors commit message.
* Formats with clang-format.
* Minor changes.
---
samples/landlock/sandboxer.c | 118 ++++++++++++++++++++++++++++++++---
1 file changed, 109 insertions(+), 9 deletions(-)
diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index e7af02f98208..96930c505807 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -60,9 +60,11 @@ static inline int landlock_restrict_self(const int ruleset_fd,
#define ENV_FS_RW_NAME "LL_FS_RW"
#define ENV_TCP_BIND_NAME "LL_TCP_BIND"
#define ENV_TCP_CONNECT_NAME "LL_TCP_CONNECT"
+#define ENV_SOCKET_CREATE_NAME "LL_SOCKET_CREATE"
#define ENV_SCOPED_NAME "LL_SCOPED"
#define ENV_FORCE_LOG_NAME "LL_FORCE_LOG"
#define ENV_DELIMITER ":"
+#define ENV_TOKEN_INTERNAL_DELIMITER "."
static int str2num(const char *numstr, __u64 *num_dst)
{
@@ -226,6 +228,83 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd,
return ret;
}
+static int populate_ruleset_socket(const char *const env_var,
+ const int ruleset_fd,
+ const __u64 allowed_access)
+{
+ int ret = 1;
+ char *env_protocol_name, *strprotocol, *strfamily, *strtype, *strproto;
+ unsigned long long family_ull, type_ull, proto_ull;
+ struct landlock_socket_attr protocol = {
+ .allowed_access = allowed_access,
+ };
+
+ env_protocol_name = getenv(env_var);
+ if (!env_protocol_name)
+ return 0;
+ env_protocol_name = strdup(env_protocol_name);
+ unsetenv(env_var);
+
+ while ((strprotocol = strsep(&env_protocol_name, ENV_DELIMITER))) {
+ strfamily = strsep(&strprotocol, ENV_TOKEN_INTERNAL_DELIMITER);
+ strtype = strsep(&strprotocol, ENV_TOKEN_INTERNAL_DELIMITER);
+ strproto = strsep(&strprotocol, ENV_TOKEN_INTERNAL_DELIMITER);
+
+ /* strsep should make this NULL if it had less than two delimiters. */
+ if (strprotocol) {
+ fprintf(stderr, "Invalid format of socket protocol\n");
+ goto out_free_name;
+ }
+ if (!strtype) {
+ fprintf(stderr,
+ "Failed to extract socket protocol with "
+ "unspecified type value\n");
+ goto out_free_name;
+ } else if (!strproto) {
+ fprintf(stderr,
+ "Failed to extract socket protocol with "
+ "unspecified protocol value\n");
+ goto out_free_name;
+ }
+
+ if (str2num(strfamily, &family_ull)) {
+ fprintf(stderr,
+ "Failed to convert \"%s\" into a number\n",
+ strfamily);
+ goto out_free_name;
+ }
+ if (str2num(strtype, &type_ull)) {
+ fprintf(stderr,
+ "Failed to convert \"%s\" into a number\n",
+ strtype);
+ goto out_free_name;
+ }
+ if (str2num(strproto, &proto_ull)) {
+ fprintf(stderr,
+ "Failed to convert \"%s\" into a number\n",
+ strproto);
+ goto out_free_name;
+ }
+ protocol.family = (int)family_ull;
+ protocol.type = (int)type_ull;
+ protocol.protocol = (int)proto_ull;
+
+ if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &protocol, 0)) {
+ fprintf(stderr,
+ "Failed to update the ruleset with "
+ "family \"%s\" and type \"%s\": %s\n",
+ strfamily, strtype, strerror(errno));
+ goto out_free_name;
+ }
+ }
+ ret = 0;
+
+out_free_name:
+ free(env_protocol_name);
+ return ret;
+}
+
/* Returns true on error, false otherwise. */
static bool check_ruleset_scope(const char *const env_var,
struct landlock_ruleset_attr *ruleset_attr)
@@ -299,7 +378,7 @@ static bool check_ruleset_scope(const char *const env_var,
/* clang-format on */
-#define LANDLOCK_ABI_LAST 7
+#define LANDLOCK_ABI_LAST 8
#define XSTR(s) #s
#define STR(s) XSTR(s)
@@ -311,7 +390,7 @@ static const char help[] =
"[other environment variables] %1$s <cmd> [args]...\n"
"\n"
"Execute the given command in a restricted environment.\n"
- "Multi-valued settings (lists of ports, paths, scopes) are colon-delimited.\n"
+ "Multi-valued settings (lists of ports, paths, protocols, scopes) are colon-delimited.\n"
"\n"
"Mandatory settings:\n"
"* " ENV_FS_RO_NAME ": paths allowed to be used in a read-only way\n"
@@ -322,6 +401,9 @@ static const char help[] =
"means an empty list):\n"
"* " ENV_TCP_BIND_NAME ": ports allowed to bind (server)\n"
"* " ENV_TCP_CONNECT_NAME ": ports allowed to connect (client)\n"
+ "* " ENV_SOCKET_CREATE_NAME ": list of socket protocols allowed to be created\n"
+ " To define protocol format \"{family}.{type}.{protocol}\" is used\n"
+ " with numerical values of family, type and protocol (eg. 2.1.0 for TCP/IP)\n"
"* " ENV_SCOPED_NAME ": actions denied on the outside of the landlock domain\n"
" - \"a\" to restrict opening abstract unix sockets\n"
" - \"s\" to restrict sending signals\n"
@@ -334,6 +416,7 @@ static const char help[] =
ENV_FS_RW_NAME "=\"/dev/null:/dev/full:/dev/zero:/dev/pts:/tmp\" "
ENV_TCP_BIND_NAME "=\"9418\" "
ENV_TCP_CONNECT_NAME "=\"80:443\" "
+ ENV_SOCKET_CREATE_NAME "=\"2.1.0\" "
ENV_SCOPED_NAME "=\"a:s\" "
"%1$s bash -i\n"
"\n"
@@ -347,7 +430,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
const char *cmd_path;
char *const *cmd_argv;
int ruleset_fd, abi;
- char *env_port_name, *env_force_log;
+ char *env_opt_name, *env_force_log;
__u64 access_fs_ro = ACCESS_FS_ROUGHLY_READ,
access_fs_rw = ACCESS_FS_ROUGHLY_READ | ACCESS_FS_ROUGHLY_WRITE;
@@ -355,6 +438,7 @@ int main(const int argc, char *const argv[], char *const *const envp)
.handled_access_fs = access_fs_rw,
.handled_access_net = LANDLOCK_ACCESS_NET_BIND_TCP |
LANDLOCK_ACCESS_NET_CONNECT_TCP,
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
.scoped = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
LANDLOCK_SCOPE_SIGNAL,
};
@@ -437,6 +521,12 @@ int main(const int argc, char *const argv[], char *const *const envp)
supported_restrict_flags &=
~LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON;
+ __attribute__((fallthrough));
+ case 7:
+ /* Removes LANDLOCK_ACCESS_SOCKET_CREATE for ABI < 8 */
+ ruleset_attr.handled_access_socket &=
+ ~LANDLOCK_ACCESS_SOCKET_CREATE;
+
/* Must be printed for any ABI < LANDLOCK_ABI_LAST. */
fprintf(stderr,
"Hint: You should update the running kernel "
@@ -456,18 +546,24 @@ int main(const int argc, char *const argv[], char *const *const envp)
access_fs_ro &= ruleset_attr.handled_access_fs;
access_fs_rw &= ruleset_attr.handled_access_fs;
- /* Removes bind access attribute if not supported by a user. */
- env_port_name = getenv(ENV_TCP_BIND_NAME);
- if (!env_port_name) {
+ /* Removes bind access attribute if not requested by a user. */
+ env_opt_name = getenv(ENV_TCP_BIND_NAME);
+ if (!env_opt_name) {
ruleset_attr.handled_access_net &=
~LANDLOCK_ACCESS_NET_BIND_TCP;
}
- /* Removes connect access attribute if not supported by a user. */
- env_port_name = getenv(ENV_TCP_CONNECT_NAME);
- if (!env_port_name) {
+ /* Removes connect access attribute if not requested by a user. */
+ env_opt_name = getenv(ENV_TCP_CONNECT_NAME);
+ if (!env_opt_name) {
ruleset_attr.handled_access_net &=
~LANDLOCK_ACCESS_NET_CONNECT_TCP;
}
+ /* Removes socket creation access attribute if not requested by a user. */
+ env_opt_name = getenv(ENV_SOCKET_CREATE_NAME);
+ if (!env_opt_name) {
+ ruleset_attr.handled_access_socket &=
+ ~LANDLOCK_ACCESS_SOCKET_CREATE;
+ }
if (check_ruleset_scope(ENV_SCOPED_NAME, &ruleset_attr))
return 1;
@@ -512,6 +608,10 @@ int main(const int argc, char *const argv[], char *const *const envp)
LANDLOCK_ACCESS_NET_CONNECT_TCP)) {
goto err_close_ruleset;
}
+ if (populate_ruleset_socket(ENV_SOCKET_CREATE_NAME, ruleset_fd,
+ LANDLOCK_ACCESS_SOCKET_CREATE)) {
+ goto err_close_ruleset;
+ }
if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
perror("Failed to restrict privileges");
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 11/19] selftests/landlock: Test protocol mappings
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
It is possible to create sockets of the same protocol with different
protocol number (cf. socket(2)) values. For example, TCP sockets can
be created using one of the following commands:
1. fd = socket(AF_INET, SOCK_STREAM, 0);
2. fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
Whereas IPPROTO_TCP = 6. Protocol number 0 correspond to the default
protocol of the given protocol family and can be mapped to another
value. Mapping is handled on the protocol family level.
Socket rules should not perform such mappings to not increase complexity
of rules definition and their maintenance.
(AF_INET, SOCK_PACKET) is an alias for (AF_PACKET, SOCK_PACKET)
(cf. __sock_create) handled due to compatibility reasons. Compared to TCP
network stack performs mapping before calling LSM hook related to socket
creation. Therefore Landlock should not restrict one pair if the other
was allowed.
Add `packet_protocol` and `tcp_protocol` fixtures and tests to validate
these scenarios.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Adds verification of TCP mapping.
* Changes commit message.
---
.../testing/selftests/landlock/socket_test.c | 157 ++++++++++++++++++
1 file changed, 157 insertions(+)
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index ce9a6e283be6..e22e10edb103 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -709,4 +709,161 @@ TEST_F(mini, kernel_socket)
EXPECT_EQ(0, test_socket(AF_SMC, SOCK_STREAM, 0));
}
+/* clang-format off */
+FIXTURE(packet_protocol) {};
+/* clang-format on */
+
+FIXTURE_VARIANT(packet_protocol)
+{
+ int family, type, protocol;
+};
+
+/* clang-format off */
+FIXTURE_SETUP(packet_protocol) {};
+/* clang-format on */
+
+FIXTURE_TEARDOWN(packet_protocol)
+{
+}
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(packet_protocol, pf_inet) {
+ /* clang-format on */
+ .family = AF_INET,
+ .type = SOCK_PACKET,
+ .protocol = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(packet_protocol, pf_packet) {
+ /* clang-format on */
+ .family = AF_PACKET,
+ .type = SOCK_PACKET,
+ .protocol = 0,
+};
+
+/*
+ * (AF_INET, SOCK_PACKET) is an alias for the (AF_PACKET, SOCK_PACKET)
+ * handled in socket layer (cf. __sock_create) due to compatibility reasons.
+ *
+ * Checks that Landlock does not restrict one pair if the other was allowed.
+ */
+TEST_F(packet_protocol, alias_restriction)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ const int family = variant->family;
+ const int type = variant->type;
+ const int protocol = variant->protocol;
+ const struct landlock_socket_attr packet_socket_create = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = family,
+ .type = type,
+ .protocol = protocol,
+ };
+ int ruleset_fd;
+
+ /*
+ * Checks that packet socket is created successfully without
+ * landlock restrictions.
+ *
+ * Packet sockets require CAP_NET_RAW capability.
+ */
+ set_cap(_metadata, CAP_NET_RAW);
+ ASSERT_EQ(0, test_socket(AF_INET, SOCK_PACKET, 0));
+ ASSERT_EQ(0, test_socket(AF_PACKET, SOCK_PACKET, 0));
+ clear_cap(_metadata, CAP_NET_RAW);
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &packet_socket_create, 0));
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ set_cap(_metadata, CAP_NET_RAW);
+ EXPECT_EQ(0, test_socket(AF_INET, SOCK_PACKET, 0));
+ EXPECT_EQ(0, test_socket(AF_PACKET, SOCK_PACKET, 0));
+ clear_cap(_metadata, CAP_NET_RAW);
+}
+
+/* clang-format off */
+FIXTURE(tcp_protocol) {};
+/* clang-format on */
+
+FIXTURE_VARIANT(tcp_protocol)
+{
+ int family, type, protocol;
+};
+
+/* clang-format off */
+FIXTURE_SETUP(tcp_protocol) {};
+/* clang-format on */
+
+FIXTURE_TEARDOWN(tcp_protocol)
+{
+}
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(tcp_protocol, variant1) {
+ /* clang-format on */
+ .family = AF_INET,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(tcp_protocol, variant2) {
+ /* clang-format on */
+ .family = AF_INET,
+ .type = SOCK_STREAM,
+ .protocol = IPPROTO_TCP, /* = 6 */
+};
+
+/*
+ * Landlock doesn't perform protocol mappings handled by network stack on
+ * protocol family level. Test verifies that if only one definition is
+ * allowed another becomes restricted.
+ */
+TEST_F(tcp_protocol, alias_restriction)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ const int family = variant->family;
+ const int type = variant->type;
+ const int protocol = variant->protocol;
+ const struct landlock_socket_attr tcp_socket_create = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = family,
+ .type = type,
+ .protocol = protocol,
+ };
+ int ruleset_fd;
+
+ ASSERT_EQ(0, test_socket(AF_INET, SOCK_STREAM, 0));
+ ASSERT_EQ(0, test_socket(AF_INET, SOCK_STREAM, IPPROTO_TCP));
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &tcp_socket_create, 0));
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ if (protocol == 0) {
+ EXPECT_EQ(0, test_socket(AF_INET, SOCK_STREAM, 0));
+ EXPECT_EQ(EACCES,
+ test_socket(AF_PACKET, SOCK_STREAM, IPPROTO_TCP));
+ } else if (protocol == IPPROTO_TCP) {
+ EXPECT_EQ(EACCES, test_socket(AF_INET, SOCK_STREAM, 0));
+ EXPECT_EQ(0, test_socket(AF_INET, SOCK_STREAM, IPPROTO_TCP));
+ }
+}
+
TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 08/19] selftests/landlock: Test network stack error code consistency
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add test validating that Landlock returns EACCES for unsupported
address family and protocol.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Access check doesn't handle error consistency due to change to
socket_create from socket_post_create LSM hook.
* Renames commit.
* Minor changes.
---
.../testing/selftests/landlock/socket_test.c | 67 +++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index 1b6c709d2893..ebb39cbf9211 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -511,4 +511,71 @@ TEST_F(protocol, restrict_socket)
self->requires_caps));
}
+/*
+ * Errors related to AF internal validation of supported protocol attributes
+ * are not consistent in sandboxed mode.
+ */
+TEST_F(mini, unsupported_af_and_prot)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ const struct landlock_socket_attr socket_af_unsupported = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = AF_UNSPEC, /* cf __sock_create */
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ };
+ const struct landlock_socket_attr socket_type_unsupported = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = AF_UNIX,
+ .type = SOCK_PACKET, /* cf. unix_create */
+ .protocol = 0,
+ };
+ const struct landlock_socket_attr socket_proto_unsupported = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = AF_UNIX,
+ .type = SOCK_STREAM,
+ .protocol = PF_UNIX + 1, /* cf. unix_create */
+ };
+ int ruleset_fd;
+
+ /* Tries to create a socket when ruleset is not established. */
+ ASSERT_EQ(EAFNOSUPPORT, test_socket(AF_UNSPEC, SOCK_STREAM, 0));
+ ASSERT_EQ(ESOCKTNOSUPPORT, test_socket(AF_UNIX, SOCK_PACKET, 0));
+ ASSERT_EQ(EPROTONOSUPPORT,
+ test_socket(AF_UNIX, SOCK_STREAM, PF_UNIX + 1));
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ /* Landlock allows creating rules for meaningless protocols. */
+ EXPECT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &socket_af_unsupported, 0));
+ EXPECT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &socket_type_unsupported, 0));
+ EXPECT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &socket_proto_unsupported, 0));
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Tries to create a socket when protocols are allowed. */
+ EXPECT_EQ(EAFNOSUPPORT, test_socket(AF_UNSPEC, SOCK_STREAM, 0));
+ EXPECT_EQ(ESOCKTNOSUPPORT, test_socket(AF_UNIX, SOCK_PACKET, 0));
+ EXPECT_EQ(EPROTONOSUPPORT,
+ test_socket(AF_UNIX, SOCK_STREAM, PF_UNIX + 1));
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Tries to create a socket when protocols are restricted. */
+ EXPECT_EQ(EACCES, test_socket(AF_UNSPEC, SOCK_STREAM, 0));
+ EXPECT_EQ(EACCES, test_socket(AF_UNIX, SOCK_PACKET, 0));
+ EXPECT_EQ(EACCES, test_socket(AF_UNIX, SOCK_STREAM, PF_UNIX + 1));
+}
+
TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 10/19] selftests/landlock: Test that kernel space sockets are not restricted
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add test validating that Landlock provides restriction of user space
sockets only.
Reviewed-by: Günther Noack <gnoack@google.com>
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Grammar fixes.
* Adds mini fixture.
---
.../testing/selftests/landlock/socket_test.c | 39 +++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index 8b8913290a64..ce9a6e283be6 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -670,4 +670,43 @@ TEST_F(mini, ruleset_with_wildcards_overlap)
EXPECT_EQ(EACCES, test_socket(AF_INET, SOCK_DGRAM, 0));
}
+/* mini.kernel_socket will fail with EAFNOSUPPORT if SMC is not supported. */
+TEST_F(mini, kernel_socket)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ const struct landlock_socket_attr smc_socket_create = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = AF_SMC,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ };
+ int ruleset_fd;
+
+ /*
+ * Checks that SMC socket is created successfully without
+ * landlock restrictions.
+ */
+ ASSERT_EQ(0, test_socket(AF_SMC, SOCK_STREAM, 0));
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &smc_socket_create, 0));
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /*
+ * During the creation of an SMC socket, an internal service TCP socket
+ * is also created (Cf. smc_create_clcsk).
+ *
+ * Checks that Landlock does not restrict creation of the kernel space
+ * socket.
+ */
+ EXPECT_EQ(0, test_socket(AF_SMC, SOCK_STREAM, 0));
+}
+
TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 06/19] landlock: Add hook on socket creation
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add hook on security_socket_create(), which checks whether the socket
of requested protocol is allowed by domain.
Due to support of masked protocols Landlock tries to find one of the
4 rules that can allow creation of requested protocol.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Changes LSM hook from socket_post_create to socket_create so
creation would be blocked before socket allocation and initialization.
* Uses credential instead of domain in hook_socket create.
* Removes get_raw_handled_socket_accesses.
* Adds checks for rules with wildcard type and protocol values.
* Minor refactoring, fixes.
Changes since v2:
* Adds check in `hook_socket_create()` to not restrict kernel space
sockets.
* Inlines `current_check_access_socket()` in the `hook_socket_create()`.
* Fixes commit message.
Changes since v1:
* Uses lsm hook arguments instead of struct socket fields as family-type
values.
* Packs socket family and type using helper.
* Fixes commit message.
* Formats with clang-format.
---
security/landlock/setup.c | 2 +
security/landlock/socket.c | 78 ++++++++++++++++++++++++++++++++++++++
security/landlock/socket.h | 2 +
3 files changed, 82 insertions(+)
diff --git a/security/landlock/setup.c b/security/landlock/setup.c
index bd53c7a56ab9..140a53b022f7 100644
--- a/security/landlock/setup.c
+++ b/security/landlock/setup.c
@@ -17,6 +17,7 @@
#include "fs.h"
#include "id.h"
#include "net.h"
+#include "socket.h"
#include "setup.h"
#include "task.h"
@@ -68,6 +69,7 @@ static int __init landlock_init(void)
landlock_add_task_hooks();
landlock_add_fs_hooks();
landlock_add_net_hooks();
+ landlock_add_socket_hooks();
landlock_init_id();
landlock_initialized = true;
pr_info("Up and running.\n");
diff --git a/security/landlock/socket.c b/security/landlock/socket.c
index 28a80dcad629..d7e6e7b92b7a 100644
--- a/security/landlock/socket.c
+++ b/security/landlock/socket.c
@@ -103,3 +103,81 @@ int landlock_append_socket_rule(struct landlock_ruleset *const ruleset,
return err;
}
+
+static int check_socket_access(const struct landlock_ruleset *dom,
+ uintptr_t key,
+ layer_mask_t (*const layer_masks)[],
+ access_mask_t handled_access)
+{
+ const struct landlock_rule *rule;
+ struct landlock_id id = {
+ .type = LANDLOCK_KEY_SOCKET,
+ };
+
+ id.key.data = key;
+ rule = landlock_find_rule(dom, id);
+ if (landlock_unmask_layers(rule, handled_access, layer_masks,
+ LANDLOCK_NUM_ACCESS_SOCKET))
+ return 0;
+ return -EACCES;
+}
+
+static int hook_socket_create(int family, int type, int protocol, int kern)
+{
+ layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_SOCKET] = {};
+ access_mask_t handled_access;
+ const struct access_masks masks = {
+ .socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ const struct landlock_cred_security *const subject =
+ landlock_get_applicable_subject(current_cred(), masks, NULL);
+ uintptr_t key;
+
+ if (!subject)
+ return 0;
+ /* Checks only user space sockets. */
+ if (kern)
+ return 0;
+
+ handled_access = landlock_init_layer_masks(
+ subject->domain, LANDLOCK_ACCESS_SOCKET_CREATE, &layer_masks,
+ LANDLOCK_KEY_SOCKET);
+ /*
+ * Error could happen due to parameters are outside of the allowed range,
+ * so this combination couldn't be added in ruleset previously.
+ * Therefore, it's not permitted.
+ */
+ if (pack_socket_key(family, type, protocol, &key) == -EACCES)
+ return -EACCES;
+ if (check_socket_access(subject->domain, key, &layer_masks,
+ handled_access) == 0)
+ return 0;
+
+ /* Ranges were already checked. */
+ (void)pack_socket_key(family, TYPE_ALL, protocol, &key);
+ if (check_socket_access(subject->domain, key, &layer_masks,
+ handled_access) == 0)
+ return 0;
+
+ (void)pack_socket_key(family, type, PROTOCOL_ALL, &key);
+ if (check_socket_access(subject->domain, key, &layer_masks,
+ handled_access) == 0)
+ return 0;
+
+ (void)pack_socket_key(family, TYPE_ALL, PROTOCOL_ALL, &key);
+ if (check_socket_access(subject->domain, key, &layer_masks,
+ handled_access) == 0)
+ return 0;
+
+ return -EACCES;
+}
+
+static struct security_hook_list landlock_hooks[] __ro_after_init = {
+ LSM_HOOK_INIT(socket_create, hook_socket_create),
+};
+
+__init void landlock_add_socket_hooks(void)
+{
+ security_add_hooks(landlock_hooks, ARRAY_SIZE(landlock_hooks),
+ &landlock_lsmid);
+}
diff --git a/security/landlock/socket.h b/security/landlock/socket.h
index bd0ac74c39e2..3980a3d46534 100644
--- a/security/landlock/socket.h
+++ b/security/landlock/socket.h
@@ -15,4 +15,6 @@ int landlock_append_socket_rule(struct landlock_ruleset *const ruleset,
const s32 protocol,
access_mask_t access_rights);
+__init void landlock_add_socket_hooks(void);
+
#endif /* _SECURITY_LANDLOCK_SOCKET_H */
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 16/19] landlock: Log socket creation denials
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add new type in landlock_requet_type related to socket access checks
auditing. Print blocker related to socket access in get_blocker() and
log socket creation denials in hook_socket_create().
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
security/landlock/audit.c | 12 ++++++++++++
security/landlock/audit.h | 1 +
security/landlock/socket.c | 15 +++++++++++++++
3 files changed, 28 insertions(+)
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index c52d079cdb77..c2c0e8fd38cb 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -48,6 +48,12 @@ static const char *const net_access_strings[] = {
static_assert(ARRAY_SIZE(net_access_strings) == LANDLOCK_NUM_ACCESS_NET);
+static const char *const socket_access_strings[] = {
+ [BIT_INDEX(LANDLOCK_ACCESS_SOCKET_CREATE)] = "socket.create",
+};
+
+static_assert(ARRAY_SIZE(socket_access_strings) == LANDLOCK_NUM_ACCESS_SOCKET);
+
static __attribute_const__ const char *
get_blocker(const enum landlock_request_type type,
const unsigned long access_bit)
@@ -71,6 +77,12 @@ get_blocker(const enum landlock_request_type type,
return "unknown";
return net_access_strings[access_bit];
+ case LANDLOCK_REQUEST_SOCKET_ACCESS:
+ if (WARN_ON_ONCE(access_bit >=
+ ARRAY_SIZE(socket_access_strings)))
+ return "unknown";
+ return socket_access_strings[access_bit];
+
case LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET:
WARN_ON_ONCE(access_bit != -1);
return "scope.abstract_unix_socket";
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 92428b7fc4d8..b78d4503b0a5 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -19,6 +19,7 @@ enum landlock_request_type {
LANDLOCK_REQUEST_FS_CHANGE_TOPOLOGY,
LANDLOCK_REQUEST_FS_ACCESS,
LANDLOCK_REQUEST_NET_ACCESS,
+ LANDLOCK_REQUEST_SOCKET_ACCESS,
LANDLOCK_REQUEST_SCOPE_ABSTRACT_UNIX_SOCKET,
LANDLOCK_REQUEST_SCOPE_SIGNAL,
};
diff --git a/security/landlock/socket.c b/security/landlock/socket.c
index d7e6e7b92b7a..6afd5a0ac6d7 100644
--- a/security/landlock/socket.c
+++ b/security/landlock/socket.c
@@ -10,6 +10,7 @@
#include <linux/stddef.h>
#include <net/ipv6.h>
+#include "audit.h"
#include "limits.h"
#include "ruleset.h"
#include "socket.h"
@@ -132,6 +133,11 @@ static int hook_socket_create(int family, int type, int protocol, int kern)
const struct landlock_cred_security *const subject =
landlock_get_applicable_subject(current_cred(), masks, NULL);
uintptr_t key;
+ struct lsm_socket_audit audit_socket = {
+ .family = family,
+ .type = type,
+ .protocol = protocol,
+ };
if (!subject)
return 0;
@@ -169,6 +175,15 @@ static int hook_socket_create(int family, int type, int protocol, int kern)
handled_access) == 0)
return 0;
+ landlock_log_denial(subject,
+ &(struct landlock_request){
+ .type = LANDLOCK_REQUEST_SOCKET_ACCESS,
+ .audit.type = LSM_AUDIT_DATA_SOCKET,
+ .audit.u.socket = &audit_socket,
+ .access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .layer_masks = &layer_masks,
+ .layer_masks_size = ARRAY_SIZE(layer_masks),
+ });
return -EACCES;
}
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 07/19] selftests/landlock: Test basic socket restriction
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add `protocol` fixture to test biggest part of communication protocol
variants. Add config options required to create sockets for each of these
protocols. Support CAP_NET_RAW capability which is required by some
of the tested protocols.
Add protocols_define.h file containing definitions of tested protocols.
Add test that validates Landlock ability to control creation of sockets
via socket(2) syscall.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Removes some of the protocols from testing (SMC, KMC and XDP).
* Rewrites commit description.
* Changes test name to "restrict_socket_creation".
* Minor changes.
Changes since v2:
* Extends variants of `protocol` fixture with every socket protocol
that can be used to create user space sockets.
* Adds `SYS_ADMIN`, `NET_ADMIN` and `NET_RAW` capabilities required for
some socket protocols.
* Removes network namespace creation in `protocol` fixture setup.
Sockets of some protocols can be created only in initial network
namespace. This shouldn't cause any issues until `protocol` fixture
is used in connection or binding tests.
* Extends config file with a set of options required by socket protocols.
* Adds CAP_NET_RAW capability to landlock selftests which is required
to create sockets of some protocols.
* Adds protocol field to the `protocol` fixture.
* Adds test_socket_variant() helper and changes the signature of
test_socket() helper.
* Checks socket(2) when ruleset is not established.
* Removes checks for AF_UNSPEC. This is moved to unsupported_af_and_prot
test.
* Removes `service_fixture` struct.
* Minor fixes.
* Refactors commit message and title.
Changes since v1:
* Replaces test_socket_create() and socket_variant() helpers
with test_socket().
* Renames domain to family in protocol fixture.
* Remove AF_UNSPEC fixture entry and add unspec_srv0 fixture field to
check AF_UNSPEC socket creation case.
* Formats code with clang-format.
* Refactors commit message.
---
tools/testing/selftests/landlock/common.h | 1 +
tools/testing/selftests/landlock/config | 47 +++++
.../selftests/landlock/protocols_define.h | 169 ++++++++++++++++++
.../testing/selftests/landlock/socket_test.c | 132 ++++++++++++++
4 files changed, 349 insertions(+)
create mode 100644 tools/testing/selftests/landlock/protocols_define.h
diff --git a/tools/testing/selftests/landlock/common.h b/tools/testing/selftests/landlock/common.h
index 88a3c78f5d98..e9378a229a4c 100644
--- a/tools/testing/selftests/landlock/common.h
+++ b/tools/testing/selftests/landlock/common.h
@@ -47,6 +47,7 @@ static void _init_caps(struct __test_metadata *const _metadata, bool drop_all)
CAP_SETUID,
CAP_SYS_ADMIN,
CAP_SYS_CHROOT,
+ CAP_NET_RAW,
/* clang-format on */
};
const unsigned int noroot = SECBIT_NOROOT | SECBIT_NOROOT_LOCKED;
diff --git a/tools/testing/selftests/landlock/config b/tools/testing/selftests/landlock/config
index 8fe9b461b1fd..98b3996c36a8 100644
--- a/tools/testing/selftests/landlock/config
+++ b/tools/testing/selftests/landlock/config
@@ -17,3 +17,50 @@ CONFIG_SHMEM=y
CONFIG_SYSFS=y
CONFIG_TMPFS=y
CONFIG_TMPFS_XATTR=y
+
+#
+# Support of socket protocols for socket_test
+#
+CONFIG_AF_RXRPC=y
+CONFIG_ATALK=y
+CONFIG_ATM=y
+CONFIG_AX25=y
+CONFIG_BT=y
+CONFIG_CAIF=y
+CONFIG_CAN_BCM=y
+CONFIG_CAN=y
+CONFIG_CRYPTO_USER_API_AEAD=y
+CONFIG_CRYPTO=y
+CONFIG_HAMRADIO=y
+CONFIG_IEEE802154_SOCKET=y
+CONFIG_IEEE802154=y
+CONFIG_INET=y
+CONFIG_INFINIBAND=y
+CONFIG_IP_SCTP=y
+CONFIG_ISDN=y
+CONFIG_LLC2=y
+CONFIG_LLC=y
+CONFIG_MPTCP=y
+CONFIG_MPTCP_IPV6=y
+CONFIG_MCTP=y
+CONFIG_MISDN=y
+CONFIG_NETDEVICES=y
+CONFIG_NET_KEY=y
+CONFIG_NETROM=y
+CONFIG_NFC=y
+CONFIG_PACKET=y
+CONFIG_PCI=y
+CONFIG_PHONET=y
+CONFIG_PPPOE=y
+CONFIG_PPP=y
+CONFIG_QRTR=y
+CONFIG_RDS=y
+CONFIG_ROSE=y
+CONFIG_SMC=y
+CONFIG_TIPC=y
+CONFIG_UNIX=y
+CONFIG_VMWARE_VMCI_VSOCKETS=y
+CONFIG_VMWARE_VMCI=y
+CONFIG_VSOCKETS=y
+CONFIG_X25=y
+CONFIG_XDP_SOCKETS=y
diff --git a/tools/testing/selftests/landlock/protocols_define.h b/tools/testing/selftests/landlock/protocols_define.h
new file mode 100644
index 000000000000..e44d2278d289
--- /dev/null
+++ b/tools/testing/selftests/landlock/protocols_define.h
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock tests - Tested socket protocols definitions
+ *
+ * Copyright © 2025 Huawei Tech. Co., Ltd.
+ */
+
+/* Almost every protocol that can be used to create socket using create() method
+ * of net_proto_family structure is tested (e.g. this method is used to
+ * create socket with socket(2)).
+ *
+ * List of address families that are not tested:
+ * - Protocol families requiring CAP_SYS_ADMIN or CAP_NET_RAW (eg. AF_PACKET,
+ * AF_CAIF).
+ * - AF_SMC, AF_KMC, AF_XDP.
+ * - AF_ASH, AF_SNA, AF_WANPIPE, AF_NETBEUI, AF_IPX, AF_DECNET, AF_ECONET
+ * and AF_IRDA are not implemented in kernel.
+ * - AF_BRIDGE, AF_MPLS can't be used for creating sockets.
+ * - AF_SECURITY - pseudo AF (Cf. socket.h).
+ * - AF_IB is reserved by infiniband.
+ */
+
+/* Cf. unix_create */
+PROTOCOL_VARIANT_ADD(UNIX, STREAM, 0);
+PROTOCOL_VARIANT_ADD(UNIX, RAW, 0);
+PROTOCOL_VARIANT_ADD(UNIX, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(UNIX, SEQPACKET, 0);
+
+/* Cf. inet_create */
+PROTOCOL_VARIANT_ADD(INET, STREAM, 0);
+PROTOCOL_VARIANT_ADD(INET, STREAM, IPPROTO_TCP);
+PROTOCOL_VARIANT_ADD(INET, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(INET, DGRAM, IPPROTO_UDP);
+PROTOCOL_VARIANT_ADD_CAPS(INET, RAW, IPPROTO_TCP);
+PROTOCOL_VARIANT_ADD(INET, SEQPACKET, IPPROTO_SCTP);
+PROTOCOL_VARIANT_ADD(INET, STREAM, IPPROTO_MPTCP);
+
+/* Cf. ax25_create */
+PROTOCOL_VARIANT_ADD(AX25, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(AX25, SEQPACKET, 0);
+PROTOCOL_VARIANT_ADD_CAPS(AX25, RAW, 0);
+
+/* Cf. atalk_create */
+PROTOCOL_VARIANT_ADD_CAPS(APPLETALK, RAW, 0);
+PROTOCOL_VARIANT_ADD(APPLETALK, DGRAM, 0);
+
+/* Cf. nr_create */
+PROTOCOL_VARIANT_ADD(NETROM, SEQPACKET, 0);
+
+/* Cf. pvc_create */
+PROTOCOL_VARIANT_ADD(ATMPVC, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(ATMPVC, RAW, 0);
+PROTOCOL_VARIANT_ADD(ATMPVC, RDM, 0);
+PROTOCOL_VARIANT_ADD(ATMPVC, SEQPACKET, 0);
+PROTOCOL_VARIANT_ADD(ATMPVC, DCCP, 0);
+PROTOCOL_VARIANT_ADD(ATMPVC, PACKET, 0);
+
+/* Cf. x25_create */
+PROTOCOL_VARIANT_ADD(X25, SEQPACKET, 0);
+
+/* Cf. inet6_create */
+PROTOCOL_VARIANT_ADD(INET6, STREAM, 0);
+PROTOCOL_VARIANT_ADD(INET6, STREAM, IPPROTO_TCP);
+PROTOCOL_VARIANT_ADD(INET6, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(INET6, DGRAM, IPPROTO_UDP);
+PROTOCOL_VARIANT_ADD_CAPS(INET6, RAW, IPPROTO_TCP);
+PROTOCOL_VARIANT_ADD(INET6, SEQPACKET, IPPROTO_SCTP);
+PROTOCOL_VARIANT_ADD(INET6, STREAM, IPPROTO_MPTCP);
+
+/* Cf. rose_create */
+PROTOCOL_VARIANT_ADD(ROSE, SEQPACKET, 0);
+
+/* Cf. pfkey_create */
+PROTOCOL_VARIANT_ADD_CAPS(KEY, RAW, PF_KEY_V2);
+
+/* Cf. netlink_create */
+PROTOCOL_VARIANT_ADD(NETLINK, RAW, 0);
+PROTOCOL_VARIANT_ADD(NETLINK, DGRAM, 0);
+
+/* Cf. packet_create */
+PROTOCOL_VARIANT_ADD_CAPS(PACKET, DGRAM, 0);
+PROTOCOL_VARIANT_ADD_CAPS(PACKET, RAW, 0);
+PROTOCOL_VARIANT_ADD_CAPS(PACKET, PACKET, 0);
+
+/* Cf. svc_create */
+PROTOCOL_VARIANT_ADD(ATMSVC, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(ATMSVC, RAW, 0);
+PROTOCOL_VARIANT_ADD(ATMSVC, RDM, 0);
+PROTOCOL_VARIANT_ADD(ATMSVC, SEQPACKET, 0);
+PROTOCOL_VARIANT_ADD(ATMSVC, DCCP, 0);
+PROTOCOL_VARIANT_ADD(ATMSVC, PACKET, 0);
+
+/* Cf. rds_create */
+PROTOCOL_VARIANT_ADD(RDS, SEQPACKET, 0);
+
+/* Cf. pppox_create + pppoe_create */
+PROTOCOL_VARIANT_ADD(PPPOX, STREAM, 0);
+PROTOCOL_VARIANT_ADD(PPPOX, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(PPPOX, RAW, 0);
+PROTOCOL_VARIANT_ADD(PPPOX, RDM, 0);
+PROTOCOL_VARIANT_ADD(PPPOX, SEQPACKET, 0);
+PROTOCOL_VARIANT_ADD(PPPOX, DCCP, 0);
+PROTOCOL_VARIANT_ADD(PPPOX, PACKET, 0);
+
+/* Cf. llc_ui_create */
+PROTOCOL_VARIANT_ADD_CAPS(LLC, DGRAM, 0);
+PROTOCOL_VARIANT_ADD_CAPS(LLC, STREAM, 0);
+
+/* Cf. can_create */
+PROTOCOL_VARIANT_ADD(CAN, DGRAM, CAN_BCM);
+
+/* Cf. tipc_sk_create */
+PROTOCOL_VARIANT_ADD(TIPC, STREAM, 0);
+PROTOCOL_VARIANT_ADD(TIPC, SEQPACKET, 0);
+PROTOCOL_VARIANT_ADD(TIPC, DGRAM, 0);
+PROTOCOL_VARIANT_ADD(TIPC, RDM, 0);
+
+/* Cf. l2cap_sock_create */
+#ifndef __s390x__
+PROTOCOL_VARIANT_ADD(BLUETOOTH, SEQPACKET, 0);
+PROTOCOL_VARIANT_ADD(BLUETOOTH, STREAM, 0);
+PROTOCOL_VARIANT_ADD(BLUETOOTH, DGRAM, 0);
+PROTOCOL_VARIANT_ADD_CAPS(BLUETOOTH, RAW, 0);
+#endif
+
+/* Cf. iucv_sock_create */
+#ifdef __s390x__
+PROTOCOL_VARIANT_ADD(IUCV, STREAM, 0);
+PROTOCOL_VARIANT_ADD(IUCV, SEQPACKET, 0);
+#endif
+
+/* Cf. rxrpc_create */
+PROTOCOL_VARIANT_ADD(RXRPC, DGRAM, PF_INET);
+
+/* Cf. mISDN_sock_create */
+#define ISDN_P_BASE 0 /* Cf. linux/mISDNif.h */
+#define ISDN_P_TE_S0 0x01 /* Cf. linux/mISDNif.h */
+PROTOCOL_VARIANT_ADD_CAPS(ISDN, RAW, ISDN_P_BASE);
+PROTOCOL_VARIANT_ADD(ISDN, DGRAM, ISDN_P_TE_S0);
+
+/* Cf. pn_socket_create */
+PROTOCOL_VARIANT_ADD_CAPS(PHONET, DGRAM, 0);
+PROTOCOL_VARIANT_ADD_CAPS(PHONET, SEQPACKET, 0);
+
+/* Cf. ieee802154_create */
+PROTOCOL_VARIANT_ADD_CAPS(IEEE802154, RAW, 0);
+PROTOCOL_VARIANT_ADD(IEEE802154, DGRAM, 0);
+
+/* Cf. caif_create */
+PROTOCOL_VARIANT_ADD_CAPS(CAIF, SEQPACKET, 0);
+PROTOCOL_VARIANT_ADD_CAPS(CAIF, STREAM, 0);
+
+/* Cf. alg_create */
+PROTOCOL_VARIANT_ADD(ALG, SEQPACKET, 0);
+
+/* Cf. nfc_sock_create + rawsock_create */
+PROTOCOL_VARIANT_ADD(NFC, SEQPACKET, 0);
+
+/* Cf. vsock_create */
+#if defined(__x86_64__) || defined(__aarch64__)
+PROTOCOL_VARIANT_ADD(VSOCK, STREAM, 0);
+PROTOCOL_VARIANT_ADD(VSOCK, SEQPACKET, 0);
+#endif
+
+/* Cf. qrtr_create */
+PROTOCOL_VARIANT_ADD(QIPCRTR, DGRAM, 0);
+
+/* Cf. mctp_pf_create */
+PROTOCOL_VARIANT_ADD(MCTP, DGRAM, 0);
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index 16477614dfed..1b6c709d2893 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -9,6 +9,9 @@
#include <linux/landlock.h>
#include <sys/prctl.h>
+#include <linux/pfkeyv2.h>
+#include <linux/kcm.h>
+#include <linux/can.h>
#include "common.h"
@@ -379,4 +382,133 @@ TEST_F(prot_outside_range, add_rule)
ASSERT_EQ(0, close(ruleset_fd));
}
+FIXTURE(protocol)
+{
+ struct protocol_variant prot;
+ bool requires_caps;
+};
+
+FIXTURE_VARIANT(protocol)
+{
+ struct protocol_variant prot;
+ bool requires_caps;
+};
+
+FIXTURE_SETUP(protocol)
+{
+ disable_caps(_metadata);
+
+ self->prot = variant->prot;
+ self->requires_caps = variant->requires_caps;
+};
+
+FIXTURE_TEARDOWN(protocol)
+{
+}
+
+#define _PROTOCOL_VARIANT_ADD(family_, type_, protocol_, caps_) \
+ FIXTURE_VARIANT_ADD(protocol, family_##_##type_##_##protocol_) \
+ { \
+ .prot = { \
+ .domain = AF_##family_, \
+ .type = SOCK_##type_, \
+ .protocol = protocol_, \
+ }, \
+ .requires_caps = caps_, \
+ }
+
+#define PROTOCOL_VARIANT_ADD(family, type, protocol) \
+ _PROTOCOL_VARIANT_ADD(family, type, protocol, false)
+
+#define PROTOCOL_VARIANT_ADD_CAPS(family, type, protocol) \
+ _PROTOCOL_VARIANT_ADD(family, type, protocol, true)
+
+#include "protocols_define.h"
+
+#undef _PROTOCOL_VARIANT_ADD
+#undef PROTOCOL_VARIANT_ADD
+#undef PROTOCOL_VARIANT_ADD_CAPS
+
+static int test_socket(int family, int type, int protocol)
+{
+ int fd;
+
+ fd = socket(family, type | SOCK_CLOEXEC, protocol);
+ if (fd < 0)
+ return errno;
+ /*
+ * Mixing error codes from close(2) and socket(2) should not lead to
+ * any (access type) confusion for this tests.
+ */
+ if (close(fd) != 0)
+ return errno;
+ return 0;
+}
+
+static int test_socket_variant(struct __test_metadata *const _metadata,
+ const struct protocol_variant *const prot,
+ bool requires_caps)
+{
+ int err;
+
+ if (requires_caps) {
+ set_cap(_metadata, CAP_NET_RAW);
+ set_cap(_metadata, CAP_SYS_ADMIN);
+ set_cap(_metadata, CAP_NET_ADMIN);
+ }
+
+ err = test_socket(prot->domain, prot->type, prot->protocol);
+
+ if (requires_caps) {
+ clear_cap(_metadata, CAP_NET_RAW);
+ clear_cap(_metadata, CAP_SYS_ADMIN);
+ clear_cap(_metadata, CAP_NET_ADMIN);
+ }
+
+ return err;
+}
+
+TEST_F(protocol, restrict_socket)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ int ruleset_fd;
+ const struct landlock_socket_attr create_socket_attr = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = self->prot.domain,
+ .type = self->prot.type,
+ .protocol = self->prot.protocol,
+ };
+
+ /* Verifies default socket creation. */
+ ASSERT_EQ(0, test_socket_variant(_metadata, &self->prot,
+ self->requires_caps));
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &create_socket_attr, 0));
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Tries to create socket when protocol is allowed. */
+ EXPECT_EQ(0, test_socket_variant(_metadata, &self->prot,
+ self->requires_caps));
+
+ /* Denies creation. */
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Tries to create a socket when protocol is restricted. */
+ EXPECT_EQ(EACCES, test_socket_variant(_metadata, &self->prot,
+ self->requires_caps));
+}
+
TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 12/19] selftests/landlock: Test socketpair(2) restriction
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add test that checks the restriction on socket creation using
socketpair(2).
Add `socket_creation` fixture to configure sandboxing in tests in
which different socket creation actions are tested.
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Removes socket_creation fixture.
---
.../testing/selftests/landlock/socket_test.c | 55 +++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
index e22e10edb103..d1a004c2e0f5 100644
--- a/tools/testing/selftests/landlock/socket_test.c
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -866,4 +866,59 @@ TEST_F(tcp_protocol, alias_restriction)
}
}
+static int test_socketpair(int family, int type, int protocol)
+{
+ int fds[2];
+ int err;
+
+ err = socketpair(family, type | SOCK_CLOEXEC, protocol, fds);
+ if (err)
+ return errno;
+ /*
+ * Mixing error codes from close(2) and socketpair(2) should not lead to
+ * any (access type) confusion for this test.
+ */
+ if (close(fds[0]) != 0)
+ return errno;
+ if (close(fds[1]) != 0)
+ return errno;
+ return 0;
+}
+
+TEST_F(mini, socketpair)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+ };
+ const struct landlock_socket_attr unix_socket_create = {
+ .allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+ .family = AF_UNIX,
+ .type = SOCK_STREAM,
+ .protocol = 0,
+ };
+ int ruleset_fd;
+
+ /* Tries to create socket when ruleset is not established. */
+ ASSERT_EQ(0, test_socketpair(AF_UNIX, SOCK_STREAM, 0));
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, ruleset_fd);
+
+ ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+ &unix_socket_create, 0));
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Tries to create socket when protocol is allowed */
+ EXPECT_EQ(0, test_socketpair(AF_UNIX, SOCK_STREAM, 0));
+
+ ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Tries to create socket when protocol is restricted. */
+ EXPECT_EQ(EACCES, test_socketpair(AF_UNIX, SOCK_STREAM, 0));
+}
+
TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v4 02/19] selftests/landlock: Test creating a ruleset with unknown access
From: Mikhail Ivanov @ 2025-11-18 13:46 UTC (permalink / raw)
To: mic, gnoack
Cc: willemdebruijn.kernel, matthieu, linux-security-module, netdev,
netfilter-devel, yusongping, artem.kuzin, konstantin.meskhidze
In-Reply-To: <20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com>
Add test that validates behaviour of Landlock after ruleset with
unknown access is created.
Reviewed-by: Günther Noack <gnoack@google.com>
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
Changes since v3:
* Adds fixture `mini`. Socket creation should be tested with capabilities
disabled.
Changes since v2:
* Removes fixture `mini`. Network namespace is not used, so this
fixture has become useless.
* Changes commit title and message.
Changes since v1:
* Refactors commit message.
---
.../testing/selftests/landlock/socket_test.c | 47 +++++++++++++++++++
1 file changed, 47 insertions(+)
create mode 100644 tools/testing/selftests/landlock/socket_test.c
diff --git a/tools/testing/selftests/landlock/socket_test.c b/tools/testing/selftests/landlock/socket_test.c
new file mode 100644
index 000000000000..d5716149d03f
--- /dev/null
+++ b/tools/testing/selftests/landlock/socket_test.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock tests - Socket
+ *
+ * Copyright © 2025 Huawei Tech. Co., Ltd.
+ */
+
+#define _GNU_SOURCE
+
+#include <linux/landlock.h>
+#include <sys/prctl.h>
+
+#include "common.h"
+
+#define ACCESS_LAST LANDLOCK_ACCESS_SOCKET_CREATE
+#define ACCESS_ALL LANDLOCK_ACCESS_SOCKET_CREATE
+
+/* clang-format off */
+FIXTURE(mini) {};
+/* clang-format on */
+
+FIXTURE_SETUP(mini)
+{
+ disable_caps(_metadata);
+};
+
+FIXTURE_TEARDOWN(mini)
+{
+}
+
+TEST_F(mini, ruleset_with_unknown_access)
+{
+ __u64 access_mask;
+
+ for (access_mask = 1ULL << 63; access_mask != ACCESS_LAST;
+ access_mask >>= 1) {
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .handled_access_socket = access_mask,
+ };
+
+ ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr,
+ sizeof(ruleset_attr), 0));
+ ASSERT_EQ(EINVAL, errno);
+ }
+}
+
+TEST_HARNESS_MAIN
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox