* [PATCH v6 06/15] digest_cache: Add hash tables and operations
From: Roberto Sassu @ 2024-11-19 10:49 UTC (permalink / raw)
To: zohar, dmitry.kasatkin, eric.snowberg, corbet, mcgrof, petr.pavlu,
samitolvanen, da.gomez, akpm, paul, jmorris, serge, shuah,
mcoquelin.stm32, alexandre.torgue
Cc: linux-integrity, linux-doc, linux-kernel, linux-api,
linux-modules, linux-security-module, linux-kselftest, wufan,
pbrobinson, zbyszek, hch, mjg59, pmatilai, jannh, dhowells, jikos,
mkoutny, ppavlu, petr.vorel, mzerqung, kgold, Roberto Sassu
In-Reply-To: <20241119104922.2772571-1-roberto.sassu@huaweicloud.com>
From: Roberto Sassu <roberto.sassu@huawei.com>
Add a linked list of hash tables to the digest cache, one per algorithm,
containing the digests extracted from digest lists.
The number of hash table slots is determined by dividing the number of
digests to add to the average depth of the collision list defined with
CONFIG_DIGEST_CACHE_HTABLE_DEPTH (currently set to 30). It can be changed
in the kernel configuration.
Add digest_cache_htable_init(), digest_cache_htable_add() and
digest_cache_htable_lookup() to the Parser API, so that digest list parsers
can allocate the hash tables and add/lookup extracted digests.
Add digest_cache_htable_free(), to let the Integrity Digest Cache free the
hash tables at the time a digest cache is freed.
Add digest_cache_lookup() to the Client API, to let users of the Integrity
Digest Cache search a digest in a digest cache and, in a subsequent patch,
to search it in the digest caches for each directory entry.
digest_cache_lookup() returns a digest cache reference that must be
released by calling digest_cache_put().
Finally, introduce digest_cache_hash_key() to compute the hash table key
from the first two bytes of the digest (modulo the number of slots).
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
include/linux/digest_cache.h | 39 ++++
security/integrity/digest_cache/Kconfig | 11 +
security/integrity/digest_cache/Makefile | 2 +-
security/integrity/digest_cache/htable.c | 251 +++++++++++++++++++++
security/integrity/digest_cache/internal.h | 36 +++
security/integrity/digest_cache/main.c | 3 +
6 files changed, 341 insertions(+), 1 deletion(-)
create mode 100644 security/integrity/digest_cache/htable.c
diff --git a/include/linux/digest_cache.h b/include/linux/digest_cache.h
index 1f88b61fb7cd..59a42c04cbb8 100644
--- a/include/linux/digest_cache.h
+++ b/include/linux/digest_cache.h
@@ -11,12 +11,25 @@
#define _LINUX_DIGEST_CACHE_H
#include <linux/fs.h>
+#include <crypto/hash_info.h>
#ifdef CONFIG_INTEGRITY_DIGEST_CACHE
/* Client API */
struct digest_cache *digest_cache_get(struct file *file);
void digest_cache_put(struct digest_cache *digest_cache);
bool digest_cache_opened_fd(struct file *file);
+struct digest_cache *digest_cache_lookup(struct dentry *dentry,
+ struct digest_cache *digest_cache,
+ u8 *digest, enum hash_algo algo);
+
+/* Parser API */
+int digest_cache_htable_init(struct digest_cache *digest_cache, u64 num_digests,
+ enum hash_algo algo);
+int digest_cache_htable_add(struct digest_cache *digest_cache, u8 *digest,
+ enum hash_algo algo);
+int digest_cache_htable_lookup(struct dentry *dentry,
+ struct digest_cache *digest_cache, u8 *digest,
+ enum hash_algo algo);
#else
static inline struct digest_cache *digest_cache_get(struct file *file)
@@ -33,5 +46,31 @@ static inline bool digest_cache_opened_fd(struct file *file)
return false;
}
+static inline struct digest_cache *
+digest_cache_lookup(struct dentry *dentry, struct digest_cache *digest_cache,
+ u8 *digest, enum hash_algo algo)
+{
+ return NULL;
+}
+
+static inline int digest_cache_htable_init(struct digest_cache *digest_cache,
+ u64 num_digests, enum hash_algo algo)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int digest_cache_htable_add(struct digest_cache *digest_cache,
+ u8 *digest, enum hash_algo algo)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int digest_cache_htable_lookup(struct dentry *dentry,
+ struct digest_cache *digest_cache,
+ u8 *digest, enum hash_algo algo)
+{
+ return -EOPNOTSUPP;
+}
+
#endif /* CONFIG_INTEGRITY_DIGEST_CACHE */
#endif /* _LINUX_DIGEST_CACHE_H */
diff --git a/security/integrity/digest_cache/Kconfig b/security/integrity/digest_cache/Kconfig
index a4bfa8287b8d..419011fb52c9 100644
--- a/security/integrity/digest_cache/Kconfig
+++ b/security/integrity/digest_cache/Kconfig
@@ -21,3 +21,14 @@ config DIGEST_LIST_DEFAULT_PATH
It can be changed at run-time, by writing the new path to the
securityfs interface. Digest caches created with the old path are
not affected by the change.
+
+config DIGEST_CACHE_HTABLE_DEPTH
+ int
+ default 30
+ help
+ Desired average depth of the collision list in the digest cache
+ hash tables.
+
+ A smaller number will increase the amount of hash table slots, and
+ make the search faster. A bigger number will decrease the number of
+ hash table slots, but make the search slower.
diff --git a/security/integrity/digest_cache/Makefile b/security/integrity/digest_cache/Makefile
index c351186d4e1e..0092c913979d 100644
--- a/security/integrity/digest_cache/Makefile
+++ b/security/integrity/digest_cache/Makefile
@@ -4,4 +4,4 @@
obj-$(CONFIG_INTEGRITY_DIGEST_CACHE) += digest_cache.o
-digest_cache-y := main.o secfs.o
+digest_cache-y := main.o secfs.o htable.o
diff --git a/security/integrity/digest_cache/htable.c b/security/integrity/digest_cache/htable.c
new file mode 100644
index 000000000000..8aa6d50a0cb5
--- /dev/null
+++ b/security/integrity/digest_cache/htable.c
@@ -0,0 +1,251 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023-2024 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * Implement hash table operations for the digest cache.
+ */
+
+#define pr_fmt(fmt) "digest_cache: "fmt
+#include "internal.h"
+
+/**
+ * digest_cache_hash_key - Compute hash key
+ * @digest: Digest cache
+ * @num_slots: Number of slots in the hash table
+ *
+ * This function computes a hash key based on the first two bytes of the
+ * digest and the number of slots of the hash table.
+ *
+ * Return: Hash key.
+ */
+static inline unsigned int digest_cache_hash_key(u8 *digest,
+ unsigned int num_slots)
+{
+ /* Same as ima_hash_key() but parametrized. */
+ return (digest[0] | digest[1] << 8) % num_slots;
+}
+
+/**
+ * lookup_htable - Lookup a hash table
+ * @digest_cache: Digest cache
+ * @algo: Algorithm of the desired hash table
+ *
+ * This function searches the hash table for a given algorithm in the digest
+ * cache.
+ *
+ * Return: A hash table if found, NULL otherwise.
+ */
+static struct htable *lookup_htable(struct digest_cache *digest_cache,
+ enum hash_algo algo)
+{
+ struct htable *h;
+
+ list_for_each_entry(h, &digest_cache->htables, next)
+ if (h->algo == algo)
+ return h;
+
+ return NULL;
+}
+
+/**
+ * digest_cache_htable_init - Allocate and initialize the hash table
+ * @digest_cache: Digest cache
+ * @num_digests: Number of digests to add to the hash table
+ * @algo: Algorithm of the digests
+ *
+ * This function allocates and initializes the hash table for a given algorithm.
+ * The number of slots depends on the number of digests to add to the digest
+ * cache, and the constant CONFIG_DIGEST_CACHE_HTABLE_DEPTH stating the desired
+ * average depth of the collision list.
+ *
+ * Return: Zero on success, a POSIX error code otherwise.
+ */
+int digest_cache_htable_init(struct digest_cache *digest_cache, u64 num_digests,
+ enum hash_algo algo)
+{
+ struct htable *h;
+ unsigned int i;
+
+ if (!num_digests)
+ return -EINVAL;
+
+ h = lookup_htable(digest_cache, algo);
+ if (h)
+ return 0;
+
+ h = kmalloc(sizeof(*h), GFP_KERNEL);
+ if (!h)
+ return -ENOMEM;
+
+ h->num_slots = DIV_ROUND_UP(num_digests,
+ CONFIG_DIGEST_CACHE_HTABLE_DEPTH);
+ h->slots = kmalloc_array(h->num_slots, sizeof(*h->slots), GFP_KERNEL);
+ if (!h->slots) {
+ kfree(h);
+ return -ENOMEM;
+ }
+
+ for (i = 0; i < h->num_slots; i++)
+ INIT_HLIST_HEAD(&h->slots[i]);
+
+ h->num_digests = 0;
+ h->algo = algo;
+
+ list_add_tail(&h->next, &digest_cache->htables);
+
+ pr_debug("Initialized hash table for digest list %s, digests: %llu, slots: %u, algo: %s\n",
+ digest_cache->path_str, num_digests, h->num_slots,
+ hash_algo_name[algo]);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(digest_cache_htable_init);
+
+/**
+ * digest_cache_htable_add - Add a new digest to the digest cache
+ * @digest_cache: Digest cache
+ * @digest: Digest to add
+ * @algo: Algorithm of the digest
+ *
+ * This function adds a digest extracted from a digest list to the digest cache.
+ *
+ * Return: Zero on success, a POSIX error code otherwise.
+ */
+int digest_cache_htable_add(struct digest_cache *digest_cache, u8 *digest,
+ enum hash_algo algo)
+{
+ struct htable *h;
+ struct digest_cache_entry *entry;
+ unsigned int key;
+ int digest_len;
+
+ h = lookup_htable(digest_cache, algo);
+ if (!h) {
+ pr_debug("No hash table for algorithm %s was found in digest cache %s, initialize one\n",
+ hash_algo_name[algo], digest_cache->path_str);
+ return -ENOENT;
+ }
+
+ digest_len = hash_digest_size[algo];
+
+ entry = kmalloc(sizeof(*entry) + digest_len, GFP_KERNEL);
+ if (!entry)
+ return -ENOMEM;
+
+ memcpy(entry->digest, digest, digest_len);
+
+ key = digest_cache_hash_key(digest, h->num_slots);
+ hlist_add_head(&entry->hnext, &h->slots[key]);
+ h->num_digests++;
+ pr_debug("Added digest %s:%*phN to digest cache %s, num of %s digests: %llu\n",
+ hash_algo_name[algo], digest_len, digest,
+ digest_cache->path_str, hash_algo_name[algo], h->num_digests);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(digest_cache_htable_add);
+
+/**
+ * digest_cache_htable_lookup - Search a digest in the digest cache
+ * @dentry: Dentry of the file whose digest is looked up
+ * @digest_cache: Digest cache
+ * @digest: Digest to search
+ * @algo: Algorithm of the digest to search
+ *
+ * This function searches the passed digest and algorithm in the digest cache.
+ *
+ * Return: Zero if the digest is found, a POSIX error code otherwise.
+ */
+int digest_cache_htable_lookup(struct dentry *dentry,
+ struct digest_cache *digest_cache, u8 *digest,
+ enum hash_algo algo)
+{
+ struct digest_cache_entry *entry;
+ struct htable *h;
+ unsigned int key;
+ int digest_len = hash_digest_size[algo];
+ int search_depth = 0, ret = -ENOENT;
+
+ h = lookup_htable(digest_cache, algo);
+ if (!h)
+ goto out;
+
+ key = digest_cache_hash_key(digest, h->num_slots);
+
+ hlist_for_each_entry(entry, &h->slots[key], hnext) {
+ if (!memcmp(entry->digest, digest, digest_len)) {
+ pr_debug("Cache hit at depth %d for file %s, digest %s:%*phN in digest cache %s\n",
+ search_depth, dentry->d_name.name,
+ hash_algo_name[algo], digest_len, digest,
+ digest_cache->path_str);
+
+ return 0;
+ }
+
+ search_depth++;
+ }
+out:
+ pr_debug("Cache miss for file %s, digest %s:%*phN not in digest cache %s\n",
+ dentry->d_name.name, hash_algo_name[algo], digest_len, digest,
+ digest_cache->path_str);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(digest_cache_htable_lookup);
+
+/**
+ * digest_cache_lookup - Search a digest in the digest cache
+ * @dentry: Dentry of the file whose digest is looked up
+ * @digest_cache: Digest cache
+ * @digest: Digest to search
+ * @algo: Algorithm of the digest to search
+ *
+ * This function calls digest_cache_htable_lookup() to search a digest in the
+ * passed digest cache, obtained with digest_cache_get().
+ *
+ * Return: A digest cache reference the digest is found, NULL if not.
+ */
+struct digest_cache *digest_cache_lookup(struct dentry *dentry,
+ struct digest_cache *digest_cache,
+ u8 *digest, enum hash_algo algo)
+{
+ int ret;
+
+ ret = digest_cache_htable_lookup(dentry, digest_cache, digest, algo);
+ if (ret < 0)
+ return NULL;
+
+ return digest_cache_ref(digest_cache);
+}
+EXPORT_SYMBOL_GPL(digest_cache_lookup);
+
+/**
+ * digest_cache_htable_free - Free the hash tables
+ * @digest_cache: Digest cache
+ *
+ * This function removes all digests from all hash tables in the digest cache,
+ * and frees the memory.
+ */
+void digest_cache_htable_free(struct digest_cache *digest_cache)
+{
+ struct htable *h, *h_tmp;
+ struct digest_cache_entry *p;
+ struct hlist_node *q;
+ unsigned int i;
+
+ list_for_each_entry_safe(h, h_tmp, &digest_cache->htables, next) {
+ for (i = 0; i < h->num_slots; i++) {
+ hlist_for_each_entry_safe(p, q, &h->slots[i], hnext) {
+ hlist_del(&p->hnext);
+ pr_debug("Removed digest %s:%*phN from digest cache %s\n",
+ hash_algo_name[h->algo],
+ hash_digest_size[h->algo], p->digest,
+ digest_cache->path_str);
+ kfree(p);
+ }
+ }
+
+ list_del(&h->next);
+ kfree(h->slots);
+ kfree(h);
+ }
+}
diff --git a/security/integrity/digest_cache/internal.h b/security/integrity/digest_cache/internal.h
index 82d9c894c8fc..e14343e96caa 100644
--- a/security/integrity/digest_cache/internal.h
+++ b/security/integrity/digest_cache/internal.h
@@ -18,8 +18,40 @@
#define INIT_STARTED 1 /* Digest cache init started. */
#define INVALID 2 /* Digest cache marked as invalid. */
+/**
+ * struct digest_cache_entry - Entry of a digest cache hash table
+ * @hnext: Pointer to the next element in the collision list
+ * @digest: Stored digest
+ *
+ * This structure represents an entry of a digest cache hash table, storing a
+ * digest.
+ */
+struct digest_cache_entry {
+ struct hlist_node hnext;
+ u8 digest[];
+};
+
+/**
+ * struct htable - Hash table
+ * @next: Next hash table in the linked list
+ * @slots: Hash table slots
+ * @num_slots: Number of slots
+ * @num_digests: Number of digests stored in the hash table
+ * @algo: Algorithm of the digests
+ *
+ * This structure is a hash table storing digests of file data or metadata.
+ */
+struct htable {
+ struct list_head next;
+ struct hlist_head *slots;
+ unsigned int num_slots;
+ u64 num_digests;
+ enum hash_algo algo;
+};
+
/**
* struct digest_cache - Digest cache
+ * @htables: Hash tables (one per algorithm)
* @ref_count: Number of references to the digest cache
* @path_str: Path of the digest list the digest cache was created from
* @flags: Control flags
@@ -27,6 +59,7 @@
* This structure represents a cache of digests extracted from a digest list.
*/
struct digest_cache {
+ struct list_head htables;
atomic_t ref_count;
char *path_str;
unsigned long flags;
@@ -125,4 +158,7 @@ int __init digest_cache_do_init(const struct lsm_id *lsm_id,
/* secfs.c */
int __init digest_cache_secfs_init(struct dentry *dir);
+/* htable.c */
+void digest_cache_htable_free(struct digest_cache *digest_cache);
+
#endif /* _DIGEST_CACHE_INTERNAL_H */
diff --git a/security/integrity/digest_cache/main.c b/security/integrity/digest_cache/main.c
index 6724471914da..ebc5dc09a62b 100644
--- a/security/integrity/digest_cache/main.c
+++ b/security/integrity/digest_cache/main.c
@@ -51,6 +51,7 @@ static struct digest_cache *digest_cache_alloc_init(char *path_str,
atomic_set(&digest_cache->ref_count, 1);
digest_cache->flags = 0UL;
+ INIT_LIST_HEAD(&digest_cache->htables);
pr_debug("New digest cache %s (ref count: %d)\n",
digest_cache->path_str, atomic_read(&digest_cache->ref_count));
@@ -66,6 +67,8 @@ static struct digest_cache *digest_cache_alloc_init(char *path_str,
*/
static void digest_cache_free(struct digest_cache *digest_cache)
{
+ digest_cache_htable_free(digest_cache);
+
pr_debug("Freed digest cache %s\n", digest_cache->path_str);
kfree(digest_cache->path_str);
kmem_cache_free(digest_cache_cache, digest_cache);
--
2.47.0.118.gfd3785337b
^ permalink raw reply related
* [PATCH v6 05/15] digest_cache: Add securityfs interface
From: Roberto Sassu @ 2024-11-19 10:49 UTC (permalink / raw)
To: zohar, dmitry.kasatkin, eric.snowberg, corbet, mcgrof, petr.pavlu,
samitolvanen, da.gomez, akpm, paul, jmorris, serge, shuah,
mcoquelin.stm32, alexandre.torgue
Cc: linux-integrity, linux-doc, linux-kernel, linux-api,
linux-modules, linux-security-module, linux-kselftest, wufan,
pbrobinson, zbyszek, hch, mjg59, pmatilai, jannh, dhowells, jikos,
mkoutny, ppavlu, petr.vorel, mzerqung, kgold, Roberto Sassu
In-Reply-To: <20241119104922.2772571-1-roberto.sassu@huaweicloud.com>
From: Roberto Sassu <roberto.sassu@huawei.com>
Create the digest_cache directory in <securityfs>/integrity, and add the
default_path file, to let root change/read the default path (file or
directory) from where digest lists are looked up.
An RW semaphore prevents the default path from changing while
digest_list_new() and read_default_path() are executed, so that those read
a stable value. Multiple digest_list_new() and read_default_path() calls,
instead, can be done in parallel, since they are the readers.
Changing the default path does not affect digest caches created with the
old path.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/digest_cache/Kconfig | 4 +
security/integrity/digest_cache/Makefile | 2 +-
security/integrity/digest_cache/internal.h | 4 +
security/integrity/digest_cache/main.c | 10 +-
security/integrity/digest_cache/secfs.c | 104 +++++++++++++++++++++
security/integrity/ima/ima_fs.c | 6 ++
6 files changed, 128 insertions(+), 2 deletions(-)
create mode 100644 security/integrity/digest_cache/secfs.c
diff --git a/security/integrity/digest_cache/Kconfig b/security/integrity/digest_cache/Kconfig
index 81f9d4ae749f..a4bfa8287b8d 100644
--- a/security/integrity/digest_cache/Kconfig
+++ b/security/integrity/digest_cache/Kconfig
@@ -17,3 +17,7 @@ config DIGEST_LIST_DEFAULT_PATH
help
Default path where the Integrity Digest Cache expects to find digest
lists.
+
+ It can be changed at run-time, by writing the new path to the
+ securityfs interface. Digest caches created with the old path are
+ not affected by the change.
diff --git a/security/integrity/digest_cache/Makefile b/security/integrity/digest_cache/Makefile
index 6a3f7cc6e106..c351186d4e1e 100644
--- a/security/integrity/digest_cache/Makefile
+++ b/security/integrity/digest_cache/Makefile
@@ -4,4 +4,4 @@
obj-$(CONFIG_INTEGRITY_DIGEST_CACHE) += digest_cache.o
-digest_cache-y := main.o
+digest_cache-y := main.o secfs.o
diff --git a/security/integrity/digest_cache/internal.h b/security/integrity/digest_cache/internal.h
index 29bf98a974f3..82d9c894c8fc 100644
--- a/security/integrity/digest_cache/internal.h
+++ b/security/integrity/digest_cache/internal.h
@@ -52,6 +52,7 @@ struct digest_cache_security {
extern loff_t inode_sec_offset;
extern loff_t file_sec_offset;
extern char *default_path_str;
+extern struct rw_semaphore default_path_sem;
static inline struct digest_cache_security *
digest_cache_get_security_from_blob(void *inode_security)
@@ -121,4 +122,7 @@ struct digest_cache *digest_cache_init(struct dentry *dentry,
int __init digest_cache_do_init(const struct lsm_id *lsm_id,
loff_t inode_offset, loff_t file_offset);
+/* secfs.c */
+int __init digest_cache_secfs_init(struct dentry *dir);
+
#endif /* _DIGEST_CACHE_INTERNAL_H */
diff --git a/security/integrity/digest_cache/main.c b/security/integrity/digest_cache/main.c
index 6e94cff2b0dc..6724471914da 100644
--- a/security/integrity/digest_cache/main.c
+++ b/security/integrity/digest_cache/main.c
@@ -21,6 +21,9 @@ loff_t file_sec_offset;
char *default_path_str = CONFIG_DIGEST_LIST_DEFAULT_PATH;
+/* Protects default_path_str. */
+struct rw_semaphore default_path_sem;
+
/**
* digest_cache_alloc_init - Allocate and initialize a new digest cache
* @path_str: Path string of the digest list
@@ -321,9 +324,12 @@ struct digest_cache *digest_cache_get(struct file *file)
/* Serialize accesses to inode for which the digest cache is used. */
mutex_lock(&dig_sec->dig_user_mutex);
- if (!dig_sec->dig_user)
+ if (!dig_sec->dig_user) {
+ down_read(&default_path_sem);
/* Consume extra reference from digest_cache_create(). */
dig_sec->dig_user = digest_cache_new(dentry, &digest_list_path);
+ up_read(&default_path_sem);
+ }
if (dig_sec->dig_user)
/* Increment ref. count for reference returned to the caller. */
@@ -439,6 +445,8 @@ static struct security_hook_list digest_cache_hooks[] __ro_after_init = {
int __init digest_cache_do_init(const struct lsm_id *lsm_id,
loff_t inode_offset, loff_t file_offset)
{
+ init_rwsem(&default_path_sem);
+
inode_sec_offset = inode_offset;
file_sec_offset = file_offset;
diff --git a/security/integrity/digest_cache/secfs.c b/security/integrity/digest_cache/secfs.c
new file mode 100644
index 000000000000..f158233f3492
--- /dev/null
+++ b/security/integrity/digest_cache/secfs.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023-2024 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * Implement the securityfs interface of the Integrity Digest Cache.
+ */
+
+#define pr_fmt(fmt) "digest_cache: "fmt
+#include <linux/security.h>
+
+#include "internal.h"
+
+static struct dentry *digest_cache_dir;
+static struct dentry *default_path_dentry;
+
+/**
+ * write_default_path - Write default path
+ * @file: File descriptor of the securityfs file
+ * @buf: User space buffer
+ * @datalen: Amount of data to write
+ * @ppos: Current position in the file
+ *
+ * This function sets the new default path where digest lists can be found.
+ * Can be either a regular file or a directory.
+ *
+ * Return: Length of path written on success, a POSIX error code otherwise.
+ */
+static ssize_t write_default_path(struct file *file, const char __user *buf,
+ size_t datalen, loff_t *ppos)
+{
+ char *new_default_path_str;
+
+ new_default_path_str = memdup_user_nul(buf, datalen);
+ if (IS_ERR(new_default_path_str))
+ return PTR_ERR(new_default_path_str);
+
+ down_write(&default_path_sem);
+ kfree_const(default_path_str);
+ default_path_str = new_default_path_str;
+ up_write(&default_path_sem);
+ return datalen;
+}
+
+/**
+ * read_default_path - Read default path
+ * @file: File descriptor of the securityfs file
+ * @buf: User space buffer
+ * @datalen: Amount of data to read
+ * @ppos: Current position in the file
+ *
+ * This function returns the current default path where digest lists can be
+ * found. Can be either a regular file or a directory.
+ *
+ * Return: Length of path read on success, a POSIX error code otherwise.
+ */
+static ssize_t read_default_path(struct file *file, char __user *buf,
+ size_t datalen, loff_t *ppos)
+{
+ int ret;
+
+ down_read(&default_path_sem);
+ ret = simple_read_from_buffer(buf, datalen, ppos, default_path_str,
+ strlen(default_path_str) + 1);
+ up_read(&default_path_sem);
+ return ret;
+}
+
+static const struct file_operations default_path_ops = {
+ .open = generic_file_open,
+ .write = write_default_path,
+ .read = read_default_path,
+ .llseek = generic_file_llseek,
+};
+
+/**
+ * digest_cache_secfs_init - Initialize the securityfs interface
+ * @dir: Directory entry provided by the calling LSM
+ *
+ * This function initializes the securityfs interfaces, for configuration
+ * by user space.
+ *
+ * It creates 'default_path', allowing user space to change the default
+ * directory where digest lists are searched.
+ *
+ * Return: Zero on success, a POSIX error code otherwise.
+ */
+int __init digest_cache_secfs_init(struct dentry *dir)
+{
+ digest_cache_dir = securityfs_create_dir("digest_cache", dir);
+ if (IS_ERR(digest_cache_dir))
+ return PTR_ERR(digest_cache_dir);
+
+ default_path_dentry = securityfs_create_file("default_path", 0660,
+ digest_cache_dir, NULL,
+ &default_path_ops);
+ if (IS_ERR(default_path_dentry)) {
+ securityfs_remove(digest_cache_dir);
+ return PTR_ERR(default_path_dentry);
+ }
+
+ return 0;
+}
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index e4a79a9b2d58..be9e374e2cef 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -614,6 +614,12 @@ int __init ima_fs_init(void)
goto out;
}
+ if (IS_ENABLED(CONFIG_INTEGRITY_DIGEST_CACHE)) {
+ ret = digest_cache_secfs_init(integrity_dir);
+ if (ret < 0)
+ goto out;
+ }
+
return 0;
out:
securityfs_remove(ima_policy);
--
2.47.0.118.gfd3785337b
^ permalink raw reply related
* [PATCH v6 04/15] digest_cache: Initialize digest caches
From: Roberto Sassu @ 2024-11-19 10:49 UTC (permalink / raw)
To: zohar, dmitry.kasatkin, eric.snowberg, corbet, mcgrof, petr.pavlu,
samitolvanen, da.gomez, akpm, paul, jmorris, serge, shuah,
mcoquelin.stm32, alexandre.torgue
Cc: linux-integrity, linux-doc, linux-kernel, linux-api,
linux-modules, linux-security-module, linux-kselftest, wufan,
pbrobinson, zbyszek, hch, mjg59, pmatilai, jannh, dhowells, jikos,
mkoutny, ppavlu, petr.vorel, mzerqung, kgold, Roberto Sassu
In-Reply-To: <20241119104922.2772571-1-roberto.sassu@huaweicloud.com>
From: Roberto Sassu <roberto.sassu@huawei.com>
Introduce digest_cache_init() to initialize created digest caches. Since
initialization happens after releasing both the dig_owner_mutex and
dig_user_mutex locks (to avoid a lock inversion with VFS locks), any caller
of digest_cache_get() can potentially be in charge of initializing them,
provided that it got the digest list path from digest_cache_new().
Introduce the INIT_STARTED flag, to atomically determine whether the digest
cache is being initialized and eventually do it if the flag is not yet set.
Introduce the INIT_IN_PROGRESS flag, for the other callers to wait until
the caller in charge of the initialization finishes initializing the digest
cache. Set INIT_IN_PROGRESS in digest_cache_create() and clear it in
digest_cache_init(). Finally, call clear_and_wake_up_bit() to wake up the
other callers.
Finally, introduce the INVALID flag, to let the callers which didn't
initialize the digest cache know that an error occurred during
initialization and, consequently, prevent them from using that digest
cache.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
security/integrity/digest_cache/internal.h | 8 ++++
security/integrity/digest_cache/main.c | 48 ++++++++++++++++++++++
2 files changed, 56 insertions(+)
diff --git a/security/integrity/digest_cache/internal.h b/security/integrity/digest_cache/internal.h
index fa76ab2672ea..29bf98a974f3 100644
--- a/security/integrity/digest_cache/internal.h
+++ b/security/integrity/digest_cache/internal.h
@@ -13,6 +13,11 @@
#include <linux/lsm_hooks.h>
#include <linux/digest_cache.h>
+/* Digest cache bits in flags. */
+#define INIT_IN_PROGRESS 0 /* Digest cache being initialized. */
+#define INIT_STARTED 1 /* Digest cache init started. */
+#define INVALID 2 /* Digest cache marked as invalid. */
+
/**
* struct digest_cache - Digest cache
* @ref_count: Number of references to the digest cache
@@ -110,6 +115,9 @@ struct digest_cache *digest_cache_create(struct dentry *dentry,
struct path *default_path,
struct path *digest_list_path,
char *path_str, char *filename);
+struct digest_cache *digest_cache_init(struct dentry *dentry,
+ struct path *digest_list_path,
+ struct digest_cache *digest_cache);
int __init digest_cache_do_init(const struct lsm_id *lsm_id,
loff_t inode_offset, loff_t file_offset);
diff --git a/security/integrity/digest_cache/main.c b/security/integrity/digest_cache/main.c
index c644cdc2ebd7..6e94cff2b0dc 100644
--- a/security/integrity/digest_cache/main.c
+++ b/security/integrity/digest_cache/main.c
@@ -156,6 +156,9 @@ struct digest_cache *digest_cache_create(struct dentry *dentry,
/* Increment ref. count for reference returned to the caller. */
digest_cache = digest_cache_ref(dig_sec->dig_owner);
+
+ /* Make other digest cache requestors wait until creation complete. */
+ set_bit(INIT_IN_PROGRESS, &digest_cache->flags);
out:
mutex_unlock(&dig_sec->dig_owner_mutex);
return digest_cache;
@@ -238,6 +241,47 @@ static struct digest_cache *digest_cache_new(struct dentry *dentry,
return digest_cache;
}
+/**
+ * digest_cache_init - Initialize a digest cache
+ * @dentry: Dentry of the inode for which the digest cache will be used
+ * @digest_list_path: Path structure of the digest list
+ * @digest_cache: Digest cache to initialize
+ *
+ * This function checks if the INIT_STARTED digest cache flag is set. If it is,
+ * or the caller didn't provide the digest list path, it waits until the caller
+ * that saw INIT_STARTED unset and had the path completes the initialization.
+ *
+ * The latter sets INIT_STARTED (atomically), performs the initialization,
+ * clears the INIT_IN_PROGRESS digest cache flag, and wakes up the other
+ * callers.
+ *
+ * Return: A valid and initialized digest cache on success, NULL otherwise.
+ */
+struct digest_cache *digest_cache_init(struct dentry *dentry,
+ struct path *digest_list_path,
+ struct digest_cache *digest_cache)
+{
+ /* Wait for digest cache initialization. */
+ if (!digest_list_path->dentry ||
+ test_and_set_bit(INIT_STARTED, &digest_cache->flags)) {
+ wait_on_bit(&digest_cache->flags, INIT_IN_PROGRESS,
+ TASK_UNINTERRUPTIBLE);
+ goto out;
+ }
+
+ /* Notify initialization complete. */
+ clear_and_wake_up_bit(INIT_IN_PROGRESS, &digest_cache->flags);
+out:
+ if (test_bit(INVALID, &digest_cache->flags)) {
+ pr_debug("Digest cache %s is invalid, don't return it\n",
+ digest_cache->path_str);
+ digest_cache_put(digest_cache);
+ digest_cache = NULL;
+ }
+
+ return digest_cache;
+}
+
/**
* digest_cache_get - Get a digest cache for a given inode
* @file: File descriptor of the inode for which the digest cache will be used
@@ -287,6 +331,10 @@ struct digest_cache *digest_cache_get(struct file *file)
mutex_unlock(&dig_sec->dig_user_mutex);
+ if (digest_cache)
+ digest_cache = digest_cache_init(dentry, &digest_list_path,
+ digest_cache);
+
if (digest_list_path.dentry)
path_put(&digest_list_path);
--
2.47.0.118.gfd3785337b
^ permalink raw reply related
* [PATCH v6 03/15] integrity: Introduce the Integrity Digest Cache
From: Roberto Sassu @ 2024-11-19 10:49 UTC (permalink / raw)
To: zohar, dmitry.kasatkin, eric.snowberg, corbet, mcgrof, petr.pavlu,
samitolvanen, da.gomez, akpm, paul, jmorris, serge, shuah,
mcoquelin.stm32, alexandre.torgue
Cc: linux-integrity, linux-doc, linux-kernel, linux-api,
linux-modules, linux-security-module, linux-kselftest, wufan,
pbrobinson, zbyszek, hch, mjg59, pmatilai, jannh, dhowells, jikos,
mkoutny, ppavlu, petr.vorel, mzerqung, kgold, Roberto Sassu
In-Reply-To: <20241119104922.2772571-1-roberto.sassu@huaweicloud.com>
From: Roberto Sassu <roberto.sassu@huawei.com>
Introduce the Integrity Digest Cache, to collect digests from various
sources (called digest lists), and to store them in kernel memory, in a set
of hash tables forming a digest cache. Extracted digests can be used as
reference values for integrity verification of file data or metadata.
The Integrity Digest Cache aims to be used by IMA and (later) by EVM to
verify accessed files. It is not an LSM on its own, and requires IMA (when
the feature is enabled) to reserve additional space in the inode and file
descriptor security blobs and to call digest_cache_do_init() for
registering additional LSM hooks.
The additional space in the inode security blob is used for storing the
digest_cache_security structure, which contains two digest cache pointers:
dig_owner, set if the digest cache was created from that inode; dig_user,
set if the digest cache was used for verifying that inode. An inode
security blob has both pointers set if digests are extracted from that
inode, and the inode itself is verified with another inode. Check and
assignment of those pointers are protected respectively with
dig_owner_mutex and dig_user_mutex.
The additional space in the file descriptor security blob is used to store
the digest cache pointer and to annotate the digest cache with the
integrity information the digest cache is created from. It also makes it
possible to identify from IMA hooks the file descriptors opened by the
Integrity Digest Cache and to properly nest the mutex, by calling the new
function digest_cache_opened_fd().
The only way for a user of the Integrity Digest Cache to obtain a digest
cache is to call digest_cache_get(). The latter first checks if dig_user in
the security blob of the passed inode is not NULL and, in that case,
immediately returns the pointer after incrementing the digest cache
reference count.
If dig_user is NULL, digest_cache_get() calls digest_cache_new(), which
determines from which path the digest cache should be retrieved. If the
default path (defined in the kernel config, or at run-time) is a file,
digest_cache_new() uses it as the final destination. If the default path is
a directory, the function attempts to read the security.digest_list xattr
and, if found, uses its value as last path component. Lastly, if the xattr
is not found or empty, it uses the directory as final destination. A
subsequent patch will allow the holders of the returned digest cache to
iteratively search a digest in each directory entry.
Finally, digest_cache_new() calls digest_cache_create(), which resolves the
inode from the path, and checks if the dig_owner pointer is set. If it is,
again, digest_cache_create() immediately returns after incrementing the
digest cache reference count. If not, it calls digest_cache_alloc_init() to
create a new digest cache. A subsequent patch will initialize it.
A holder of a digest cache can release its reference and consequently
decrement the digest cache reference count by calling digest_cache_put().
If the reference count becomes zero, digest_cache_put() also calls
digest_cache_free() to free the memory.
Inodes having either dig_owner and dig_user set are also holders of digest
caches. Their reference is released when they are evicted from memory, by
implementing the inode_free_security LSM hook. The inode_alloc_security LSM
hook, instead, initializes the digest_cache_security structure.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
include/linux/digest_cache.h | 37 ++
include/uapi/linux/xattr.h | 3 +
security/integrity/Kconfig | 1 +
security/integrity/Makefile | 1 +
security/integrity/digest_cache/Kconfig | 19 +
security/integrity/digest_cache/Makefile | 7 +
security/integrity/digest_cache/internal.h | 116 ++++++
security/integrity/digest_cache/main.c | 406 +++++++++++++++++++++
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_main.c | 10 +-
10 files changed, 600 insertions(+), 1 deletion(-)
create mode 100644 include/linux/digest_cache.h
create mode 100644 security/integrity/digest_cache/Kconfig
create mode 100644 security/integrity/digest_cache/Makefile
create mode 100644 security/integrity/digest_cache/internal.h
create mode 100644 security/integrity/digest_cache/main.c
diff --git a/include/linux/digest_cache.h b/include/linux/digest_cache.h
new file mode 100644
index 000000000000..1f88b61fb7cd
--- /dev/null
+++ b/include/linux/digest_cache.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023-2024 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * Public API of the Integrity Digest Cache.
+ */
+
+#ifndef _LINUX_DIGEST_CACHE_H
+#define _LINUX_DIGEST_CACHE_H
+
+#include <linux/fs.h>
+
+#ifdef CONFIG_INTEGRITY_DIGEST_CACHE
+/* Client API */
+struct digest_cache *digest_cache_get(struct file *file);
+void digest_cache_put(struct digest_cache *digest_cache);
+bool digest_cache_opened_fd(struct file *file);
+
+#else
+static inline struct digest_cache *digest_cache_get(struct file *file)
+{
+ return NULL;
+}
+
+static inline void digest_cache_put(struct digest_cache *digest_cache)
+{
+}
+
+static inline bool digest_cache_opened_fd(struct file *file)
+{
+ return false;
+}
+
+#endif /* CONFIG_INTEGRITY_DIGEST_CACHE */
+#endif /* _LINUX_DIGEST_CACHE_H */
diff --git a/include/uapi/linux/xattr.h b/include/uapi/linux/xattr.h
index 9463db2dfa9d..8a58cf4bce65 100644
--- a/include/uapi/linux/xattr.h
+++ b/include/uapi/linux/xattr.h
@@ -54,6 +54,9 @@
#define XATTR_IMA_SUFFIX "ima"
#define XATTR_NAME_IMA XATTR_SECURITY_PREFIX XATTR_IMA_SUFFIX
+#define XATTR_DIGEST_LIST_SUFFIX "digest_list"
+#define XATTR_NAME_DIGEST_LIST XATTR_SECURITY_PREFIX XATTR_DIGEST_LIST_SUFFIX
+
#define XATTR_SELINUX_SUFFIX "selinux"
#define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX
diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 3c45f4f3455f..55b1311a48fa 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -132,5 +132,6 @@ config INTEGRITY_AUDIT
source "security/integrity/ima/Kconfig"
source "security/integrity/evm/Kconfig"
+source "security/integrity/digest_cache/Kconfig"
endif # if INTEGRITY
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 92b63039c654..0fdabfc6c8ae 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -21,3 +21,4 @@ integrity-$(CONFIG_LOAD_PPC_KEYS) += platform_certs/efi_parser.o \
# The relative order of the 'ima' and 'evm' LSMs depends on the order below.
obj-$(CONFIG_IMA) += ima/
obj-$(CONFIG_EVM) += evm/
+obj-$(CONFIG_INTEGRITY_DIGEST_CACHE) += digest_cache/
diff --git a/security/integrity/digest_cache/Kconfig b/security/integrity/digest_cache/Kconfig
new file mode 100644
index 000000000000..81f9d4ae749f
--- /dev/null
+++ b/security/integrity/digest_cache/Kconfig
@@ -0,0 +1,19 @@
+# SPDX-License-Identifier: GPL-2.0
+config INTEGRITY_DIGEST_CACHE
+ bool "Integrity Digest Cache"
+ default n
+ help
+ This option enables a cache of reference digests (e.g. of file
+ data or metadata) to be used by IMA and (later) by EVM, to verify
+ accessed files.
+
+ Digests can be extracted from supported data formats (e.g. a
+ TLV-based format). Support for more formats can be added through
+ third-party kernel modules.
+
+config DIGEST_LIST_DEFAULT_PATH
+ string
+ default "/etc/digest_lists"
+ help
+ Default path where the Integrity Digest Cache expects to find digest
+ lists.
diff --git a/security/integrity/digest_cache/Makefile b/security/integrity/digest_cache/Makefile
new file mode 100644
index 000000000000..6a3f7cc6e106
--- /dev/null
+++ b/security/integrity/digest_cache/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for building the Integrity Digest Cache.
+
+obj-$(CONFIG_INTEGRITY_DIGEST_CACHE) += digest_cache.o
+
+digest_cache-y := main.o
diff --git a/security/integrity/digest_cache/internal.h b/security/integrity/digest_cache/internal.h
new file mode 100644
index 000000000000..fa76ab2672ea
--- /dev/null
+++ b/security/integrity/digest_cache/internal.h
@@ -0,0 +1,116 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023-2024 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * Internal header of the Integrity Digest Cache.
+ */
+
+#ifndef _DIGEST_CACHE_INTERNAL_H
+#define _DIGEST_CACHE_INTERNAL_H
+
+#include <linux/lsm_hooks.h>
+#include <linux/digest_cache.h>
+
+/**
+ * struct digest_cache - Digest cache
+ * @ref_count: Number of references to the digest cache
+ * @path_str: Path of the digest list the digest cache was created from
+ * @flags: Control flags
+ *
+ * This structure represents a cache of digests extracted from a digest list.
+ */
+struct digest_cache {
+ atomic_t ref_count;
+ char *path_str;
+ unsigned long flags;
+};
+
+/**
+ * struct digest_cache_security - Digest cache pointers in inode security blob
+ * @dig_owner: Digest cache created from this inode
+ * @dig_owner_mutex: Protects @dig_owner
+ * @dig_user: Digest cache requested for this inode
+ * @dig_user_mutex: Protects @dig_user
+ *
+ * This structure contains references to digest caches, protected by their
+ * respective mutex.
+ */
+struct digest_cache_security {
+ struct digest_cache *dig_owner;
+ struct mutex dig_owner_mutex;
+ struct digest_cache *dig_user;
+ struct mutex dig_user_mutex;
+};
+
+extern loff_t inode_sec_offset;
+extern loff_t file_sec_offset;
+extern char *default_path_str;
+
+static inline struct digest_cache_security *
+digest_cache_get_security_from_blob(void *inode_security)
+{
+ return inode_security + inode_sec_offset;
+}
+
+static inline struct digest_cache_security *
+digest_cache_get_security(const struct inode *inode)
+{
+ if (unlikely(!inode->i_security))
+ return NULL;
+
+ return digest_cache_get_security_from_blob(inode->i_security);
+}
+
+static inline struct digest_cache *
+digest_cache_ref(struct digest_cache *digest_cache)
+{
+ int ref_count = atomic_inc_return(&digest_cache->ref_count);
+
+ pr_debug("Ref (+) digest cache %s (ref count: %d)\n",
+ digest_cache->path_str, ref_count);
+ return digest_cache;
+}
+
+static inline struct digest_cache *
+digest_cache_unref(struct digest_cache *digest_cache)
+{
+ bool ref_is_zero;
+
+ /* Unreliable ref. count, but cannot decrement before print (UAF). */
+ pr_debug("Ref (-) digest cache %s (ref count: %d)\n",
+ digest_cache->path_str,
+ atomic_read(&digest_cache->ref_count) - 1);
+
+ ref_is_zero = atomic_dec_and_test(&digest_cache->ref_count);
+ return (ref_is_zero) ? digest_cache : NULL;
+}
+
+static inline void digest_cache_to_file_sec(const struct file *file,
+ struct digest_cache *digest_cache)
+{
+ struct digest_cache **digest_cache_sec;
+
+ digest_cache_sec = file->f_security + file_sec_offset;
+ *digest_cache_sec = digest_cache;
+}
+
+static inline struct digest_cache *
+digest_cache_from_file_sec(const struct file *file)
+{
+ struct digest_cache **digest_cache_sec;
+
+ digest_cache_sec = file->f_security + file_sec_offset;
+ return *digest_cache_sec;
+}
+
+/* main.c */
+struct digest_cache *digest_cache_create(struct dentry *dentry,
+ struct path *default_path,
+ struct path *digest_list_path,
+ char *path_str, char *filename);
+int __init digest_cache_do_init(const struct lsm_id *lsm_id,
+ loff_t inode_offset, loff_t file_offset);
+
+#endif /* _DIGEST_CACHE_INTERNAL_H */
diff --git a/security/integrity/digest_cache/main.c b/security/integrity/digest_cache/main.c
new file mode 100644
index 000000000000..c644cdc2ebd7
--- /dev/null
+++ b/security/integrity/digest_cache/main.c
@@ -0,0 +1,406 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023-2024 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * Implement the main code of the Integrity Digest Cache.
+ */
+
+#define pr_fmt(fmt) "digest_cache: "fmt
+#include <linux/namei.h>
+#include <linux/xattr.h>
+
+#include "internal.h"
+
+static int digest_cache_enabled __ro_after_init;
+static struct kmem_cache *digest_cache_cache __read_mostly;
+
+loff_t inode_sec_offset;
+loff_t file_sec_offset;
+
+char *default_path_str = CONFIG_DIGEST_LIST_DEFAULT_PATH;
+
+/**
+ * digest_cache_alloc_init - Allocate and initialize a new digest cache
+ * @path_str: Path string of the digest list
+ * @filename: Digest list file name (can be an empty string)
+ *
+ * This function allocates and initializes a new digest cache.
+ *
+ * Return: A new digest cache on success, NULL on error.
+ */
+static struct digest_cache *digest_cache_alloc_init(char *path_str,
+ char *filename)
+{
+ struct digest_cache *digest_cache;
+
+ digest_cache = kmem_cache_zalloc(digest_cache_cache, GFP_KERNEL);
+ if (!digest_cache)
+ return digest_cache;
+
+ digest_cache->path_str = kasprintf(GFP_KERNEL, "%s%s%s", path_str,
+ filename[0] ? "/" : "", filename);
+ if (!digest_cache->path_str) {
+ kmem_cache_free(digest_cache_cache, digest_cache);
+ return NULL;
+ }
+
+ atomic_set(&digest_cache->ref_count, 1);
+ digest_cache->flags = 0UL;
+
+ pr_debug("New digest cache %s (ref count: %d)\n",
+ digest_cache->path_str, atomic_read(&digest_cache->ref_count));
+
+ return digest_cache;
+}
+
+/**
+ * digest_cache_free - Free all memory occupied by the digest cache
+ * @digest_cache: Digest cache
+ *
+ * This function frees the memory occupied by the digest cache.
+ */
+static void digest_cache_free(struct digest_cache *digest_cache)
+{
+ pr_debug("Freed digest cache %s\n", digest_cache->path_str);
+ kfree(digest_cache->path_str);
+ kmem_cache_free(digest_cache_cache, digest_cache);
+}
+
+/**
+ * digest_cache_create - Create a digest cache
+ * @dentry: Dentry of the inode for which the digest cache will be used
+ * @default_path: Path structure of the default path
+ * @digest_list_path: Path structure of the digest list
+ * @path_str: Path string of the digest list
+ * @filename: Digest list file name (can be an empty string)
+ *
+ * This function first locates, from the passed path and filename, the digest
+ * list inode from which the digest cache will be created or retrieved (if it
+ * already exists).
+ *
+ * If dig_owner is NULL in the inode security blob, this function creates a
+ * new digest cache with reference count set to 1 (reference returned), sets
+ * it to dig_owner and consequently increments again the digest cache reference
+ * count.
+ *
+ * Otherwise, it simply increments the reference count of the existing
+ * dig_owner, since that reference is returned to the caller.
+ *
+ * This function locks dig_owner_mutex to protect against concurrent requests
+ * to obtain a digest cache from the same inode.
+ *
+ * Return: A digest cache on success, NULL on error.
+ */
+struct digest_cache *digest_cache_create(struct dentry *dentry,
+ struct path *default_path,
+ struct path *digest_list_path,
+ char *path_str, char *filename)
+{
+ struct digest_cache *digest_cache = NULL;
+ struct digest_cache_security *dig_sec;
+ struct inode *inode = d_backing_inode(default_path->dentry);
+ int ret;
+
+ if (S_ISDIR(inode->i_mode) && filename[0]) {
+ ret = vfs_path_lookup(default_path->dentry, default_path->mnt,
+ filename, LOOKUP_FOLLOW,
+ digest_list_path);
+ if (ret < 0) {
+ pr_debug("Cannot find digest list %s/%s\n", path_str,
+ filename);
+ return NULL;
+ }
+
+ inode = d_backing_inode(digest_list_path->dentry);
+
+ /* No support for nested directories. */
+ if (!S_ISREG(inode->i_mode)) {
+ pr_debug("%s is not a regular file (no support for nested directories)\n",
+ digest_list_path->dentry->d_name.name);
+ return NULL;
+ }
+ } else {
+ digest_list_path->dentry = default_path->dentry;
+ digest_list_path->mnt = default_path->mnt;
+ path_get(digest_list_path);
+ }
+
+ /*
+ * Cannot request a digest cache for the same inode the digest cache
+ * is populated from.
+ */
+ if (d_backing_inode(dentry) == inode) {
+ pr_debug("Cannot request a digest cache for %s and create the digest cache from it\n",
+ dentry->d_name.name);
+ return NULL;
+ }
+
+ dig_sec = digest_cache_get_security(inode);
+ if (unlikely(!dig_sec))
+ goto out;
+
+ /* Serialize check and assignment of dig_owner. */
+ mutex_lock(&dig_sec->dig_owner_mutex);
+ if (dig_sec->dig_owner) {
+ /* Increment ref. count for reference returned to the caller. */
+ digest_cache = digest_cache_ref(dig_sec->dig_owner);
+ goto out;
+ }
+
+ /* Ref. count is already 1 for this reference. */
+ dig_sec->dig_owner = digest_cache_alloc_init(path_str, filename);
+ if (!dig_sec->dig_owner)
+ goto out;
+
+ /* Increment ref. count for reference returned to the caller. */
+ digest_cache = digest_cache_ref(dig_sec->dig_owner);
+out:
+ mutex_unlock(&dig_sec->dig_owner_mutex);
+ return digest_cache;
+}
+
+/**
+ * digest_cache_new - Retrieve digest list file name and request digest cache
+ * @dentry: Dentry of the inode for which the digest cache will be used
+ * @digest_list_path: Path structure of the digest list (updated)
+ *
+ * This function locates the default path. If it is a file, it directly creates
+ * a digest cache from it. Otherwise, it reads the digest list file name from
+ * the security.digest_list xattr and requests the creation of a digest cache
+ * with that file name. If security.digest_list is empty or not found, this
+ * function requests the creation of a digest cache on the parent directory.
+ *
+ * This function passes to the caller the path of the digest list from which
+ * the digest cache was created (file or parent directory), so that
+ * digest_cache_init() can reuse the same path, and to prevent the inode from
+ * being evicted between creation and initialization.
+ *
+ * Return: A digest cache on success, NULL on error.
+ */
+static struct digest_cache *digest_cache_new(struct dentry *dentry,
+ struct path *digest_list_path)
+{
+ char filename[NAME_MAX + 1] = { 0 };
+ struct digest_cache *digest_cache = NULL;
+ struct path default_path;
+ int ret;
+
+ ret = kern_path(default_path_str, 0, &default_path);
+ if (ret < 0) {
+ pr_debug("Cannot find path %s\n", default_path_str);
+ return NULL;
+ }
+
+ /* The default path is a file, no need to get xattr. */
+ if (S_ISREG(d_backing_inode(default_path.dentry)->i_mode)) {
+ pr_debug("Default path %s is a file, not reading %s xattr\n",
+ default_path_str, XATTR_NAME_DIGEST_LIST);
+ goto create;
+ } else if (!S_ISDIR(d_backing_inode(default_path.dentry)->i_mode)) {
+ pr_debug("Default path %s must be either a file or a directory\n",
+ default_path_str);
+ goto out;
+ }
+
+ ret = vfs_getxattr(&nop_mnt_idmap, dentry, XATTR_NAME_DIGEST_LIST,
+ filename, sizeof(filename) - 1);
+ if (ret <= 0) {
+ if (ret && ret != -ENODATA && ret != -EOPNOTSUPP) {
+ pr_debug("Cannot get %s xattr from file %s, ret: %d\n",
+ XATTR_NAME_DIGEST_LIST, dentry->d_name.name,
+ ret);
+ goto out;
+ }
+
+ pr_debug("Digest list file name not found for file %s, using %s\n",
+ dentry->d_name.name, default_path_str);
+
+ goto create;
+ }
+
+ if (strchr(filename, '/')) {
+ pr_debug("%s xattr should contain only a file name, got: %s\n",
+ XATTR_NAME_DIGEST_LIST, filename);
+ goto out;
+ }
+
+ pr_debug("Found %s xattr in %s, default path: %s, digest list: %s\n",
+ XATTR_NAME_DIGEST_LIST, dentry->d_name.name, default_path_str,
+ filename);
+create:
+ digest_cache = digest_cache_create(dentry, &default_path,
+ digest_list_path, default_path_str,
+ filename);
+out:
+ path_put(&default_path);
+ return digest_cache;
+}
+
+/**
+ * digest_cache_get - Get a digest cache for a given inode
+ * @file: File descriptor of the inode for which the digest cache will be used
+ *
+ * This function tries to find a digest cache from the inode security blob from
+ * the passed file descriptor (dig_user field). If a digest cache was not found,
+ * it calls digest_cache_new() to create a new one. In both cases, it increments
+ * the digest cache reference count before returning the reference to the
+ * caller.
+ *
+ * The caller is responsible to call digest_cache_put() to release the digest
+ * cache reference returned.
+ *
+ * This function locks dig_user_mutex to protect against concurrent requests
+ * to obtain a digest cache for the same inode.
+ *
+ * Return: A digest cache on success, NULL otherwise.
+ */
+struct digest_cache *digest_cache_get(struct file *file)
+{
+ struct digest_cache_security *dig_sec;
+ struct digest_cache *digest_cache = NULL;
+ struct inode *inode = file_inode(file);
+ struct dentry *dentry = file_dentry(file);
+ struct path digest_list_path = { .dentry = NULL, .mnt = NULL };
+
+ if (!digest_cache_enabled)
+ return NULL;
+
+ /* Do not allow recursion for now. */
+ if (digest_cache_opened_fd(file))
+ return NULL;
+
+ dig_sec = digest_cache_get_security(inode);
+ if (unlikely(!dig_sec))
+ return NULL;
+
+ /* Serialize accesses to inode for which the digest cache is used. */
+ mutex_lock(&dig_sec->dig_user_mutex);
+ if (!dig_sec->dig_user)
+ /* Consume extra reference from digest_cache_create(). */
+ dig_sec->dig_user = digest_cache_new(dentry, &digest_list_path);
+
+ if (dig_sec->dig_user)
+ /* Increment ref. count for reference returned to the caller. */
+ digest_cache = digest_cache_ref(dig_sec->dig_user);
+
+ mutex_unlock(&dig_sec->dig_user_mutex);
+
+ if (digest_list_path.dentry)
+ path_put(&digest_list_path);
+
+ return digest_cache;
+}
+EXPORT_SYMBOL_GPL(digest_cache_get);
+
+/**
+ * digest_cache_put - Release a digest cache reference
+ * @digest_cache: Digest cache
+ *
+ * This function decrements the reference count of the digest cache passed as
+ * argument. If the reference count reaches zero, it calls digest_cache_free()
+ * to free the digest cache.
+ */
+void digest_cache_put(struct digest_cache *digest_cache)
+{
+ struct digest_cache *to_free;
+
+ to_free = digest_cache_unref(digest_cache);
+ if (!to_free)
+ return;
+
+ digest_cache_free(to_free);
+}
+EXPORT_SYMBOL_GPL(digest_cache_put);
+
+/**
+ * digest_cache_opened_fd - Determine if fd is opened by Digest Cache
+ * @file: File descriptor
+ *
+ * Determine whether or not the file descriptor was internally opened by the
+ * Integrity Digest Cache.
+ *
+ * Return: True if it is opened by us, false otherwise.
+ */
+bool digest_cache_opened_fd(struct file *file)
+{
+ struct digest_cache *digest_cache = digest_cache_from_file_sec(file);
+
+ return !!digest_cache;
+}
+EXPORT_SYMBOL_GPL(digest_cache_opened_fd);
+
+/**
+ * digest_cache_inode_alloc_security - Initialize inode security blob
+ * @inode: Inode for which the security blob is initialized
+ *
+ * This function initializes the digest_cache_security structure, directly
+ * stored in the inode security blob.
+ *
+ * Return: Zero.
+ */
+static int digest_cache_inode_alloc_security(struct inode *inode)
+{
+ struct digest_cache_security *dig_sec;
+
+ /* The inode security blob is always allocated here. */
+ dig_sec = digest_cache_get_security(inode);
+ mutex_init(&dig_sec->dig_owner_mutex);
+ mutex_init(&dig_sec->dig_user_mutex);
+ return 0;
+}
+
+/**
+ * digest_cache_inode_free_security_rcu - Release the digest cache references
+ * @inode_security: Inode security blob
+ *
+ * Since the inode is being evicted, this function releases the non-needed
+ * references to the digest caches stored in the digest_cache_security
+ * structure.
+ */
+static void digest_cache_inode_free_security_rcu(void *inode_security)
+{
+ struct digest_cache_security *dig_sec;
+
+ dig_sec = digest_cache_get_security_from_blob(inode_security);
+ mutex_destroy(&dig_sec->dig_owner_mutex);
+ mutex_destroy(&dig_sec->dig_user_mutex);
+ if (dig_sec->dig_owner)
+ digest_cache_put(dig_sec->dig_owner);
+ if (dig_sec->dig_user)
+ digest_cache_put(dig_sec->dig_user);
+}
+
+static struct security_hook_list digest_cache_hooks[] __ro_after_init = {
+ LSM_HOOK_INIT(inode_alloc_security, digest_cache_inode_alloc_security),
+ LSM_HOOK_INIT(inode_free_security_rcu,
+ digest_cache_inode_free_security_rcu),
+};
+
+/**
+ * digest_cache_do_init - Initialize the Integrity Digest Cache
+ * @lsm_id: ID of LSM registering the LSM hooks
+ * @inode_offset: Offset in the inode security blob
+ * @file_offset: Offset in the file security blob
+ *
+ * Initialize the Integrity Digest Cache, by instantiating a cache for the
+ * digest_cache structure and by registering the LSM hooks as part of the
+ * calling LSM.
+ */
+int __init digest_cache_do_init(const struct lsm_id *lsm_id,
+ loff_t inode_offset, loff_t file_offset)
+{
+ inode_sec_offset = inode_offset;
+ file_sec_offset = file_offset;
+
+ digest_cache_cache = kmem_cache_create("digest_cache_cache",
+ sizeof(struct digest_cache),
+ 0, SLAB_PANIC, NULL);
+
+ security_add_hooks(digest_cache_hooks, ARRAY_SIZE(digest_cache_hooks),
+ lsm_id);
+
+ digest_cache_enabled = 1;
+ return 0;
+}
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 3c323ca213d4..2eaa7f224da4 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -23,6 +23,7 @@
#include <crypto/hash_info.h>
#include "../integrity.h"
+#include "../digest_cache/internal.h"
enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN,
IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII };
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 06132cf47016..f6cc0c7a6244 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -1206,11 +1206,19 @@ static int __init init_ima_lsm(void)
ima_iintcache_init();
security_add_hooks(ima_hooks, ARRAY_SIZE(ima_hooks), &ima_lsmid);
init_ima_appraise_lsm(&ima_lsmid);
+ if (IS_ENABLED(CONFIG_INTEGRITY_DIGEST_CACHE))
+ digest_cache_do_init(&ima_lsmid, ima_blob_sizes.lbs_inode +
+ sizeof(struct ima_iint_cache *),
+ ima_blob_sizes.lbs_file);
return 0;
}
struct lsm_blob_sizes ima_blob_sizes __ro_after_init = {
- .lbs_inode = sizeof(struct ima_iint_cache *),
+ .lbs_inode = sizeof(struct ima_iint_cache *)
+#ifdef CONFIG_INTEGRITY_DIGEST_CACHE
+ + sizeof(struct digest_cache_security),
+ .lbs_file = sizeof(struct digest_cache *),
+#endif
};
DEFINE_LSM(ima) = {
--
2.47.0.118.gfd3785337b
^ permalink raw reply related
* [PATCH v6 02/15] module: Introduce ksys_finit_module()
From: Roberto Sassu @ 2024-11-19 10:49 UTC (permalink / raw)
To: zohar, dmitry.kasatkin, eric.snowberg, corbet, mcgrof, petr.pavlu,
samitolvanen, da.gomez, akpm, paul, jmorris, serge, shuah,
mcoquelin.stm32, alexandre.torgue
Cc: linux-integrity, linux-doc, linux-kernel, linux-api,
linux-modules, linux-security-module, linux-kselftest, wufan,
pbrobinson, zbyszek, hch, mjg59, pmatilai, jannh, dhowells, jikos,
mkoutny, ppavlu, petr.vorel, mzerqung, kgold, Roberto Sassu
In-Reply-To: <20241119104922.2772571-1-roberto.sassu@huaweicloud.com>
From: Roberto Sassu <roberto.sassu@huawei.com>
Introduce ksys_finit_module() to let kernel components request a kernel
module without requiring running modprobe.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
include/linux/syscalls.h | 10 ++++++++++
kernel/module/main.c | 43 ++++++++++++++++++++++++++++++----------
2 files changed, 43 insertions(+), 10 deletions(-)
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 5758104921e6..18bb346bb793 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -1233,6 +1233,16 @@ int ksys_ipc(unsigned int call, int first, unsigned long second,
int compat_ksys_ipc(u32 call, int first, int second,
u32 third, u32 ptr, u32 fifth);
+#ifdef CONFIG_MODULES
+int ksys_finit_module(struct file *f, const char *kargs, int flags);
+#else
+static inline int ksys_finit_module(struct file *f, const char *kargs,
+ int flags)
+{
+ return -EOPNOTSUPP;
+}
+#endif
+
/*
* The following kernel syscall equivalents are just wrappers to fs-internal
* functions. Therefore, provide stubs to be inlined at the callsites.
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 49b9bca9de12..81f79c9ea637 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2852,7 +2852,7 @@ static int early_mod_check(struct load_info *info, int flags)
* zero, and we rely on this for optional sections.
*/
static int load_module(struct load_info *info, const char __user *uargs,
- int flags)
+ const char *kargs, int flags)
{
struct module *mod;
bool module_allocated = false;
@@ -2953,7 +2953,13 @@ static int load_module(struct load_info *info, const char __user *uargs,
flush_module_icache(mod);
/* Now copy in args */
- mod->args = strndup_user(uargs, ~0UL >> 1);
+ if (kargs) {
+ mod->args = kstrndup(kargs, ~0UL >> 1, GFP_KERNEL);
+ if (!mod->args)
+ mod->args = ERR_PTR(-ENOMEM);
+ } else {
+ mod->args = strndup_user(uargs, ~0UL >> 1);
+ }
if (IS_ERR(mod->args)) {
err = PTR_ERR(mod->args);
goto free_arch_cleanup;
@@ -3083,7 +3089,7 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
return err;
}
- return load_module(&info, uargs, 0);
+ return load_module(&info, uargs, NULL, 0);
}
struct idempotent {
@@ -3170,7 +3176,8 @@ static int idempotent_wait_for_completion(struct idempotent *u)
return u->ret;
}
-static int init_module_from_file(struct file *f, const char __user * uargs, int flags)
+static int init_module_from_file(struct file *f, const char __user * uargs,
+ const char *kargs, int flags)
{
struct load_info info = { };
void *buf = NULL;
@@ -3195,10 +3202,11 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
info.len = len;
}
- return load_module(&info, uargs, flags);
+ return load_module(&info, uargs, kargs, flags);
}
-static int idempotent_init_module(struct file *f, const char __user * uargs, int flags)
+static int idempotent_init_module(struct file *f, const char __user * uargs,
+ const char *kargs, int flags)
{
struct idempotent idem;
@@ -3207,7 +3215,7 @@ static int idempotent_init_module(struct file *f, const char __user * uargs, int
/* Are we the winners of the race and get to do this? */
if (!idempotent(&idem, file_inode(f))) {
- int ret = init_module_from_file(f, uargs, flags);
+ int ret = init_module_from_file(f, uargs, kargs, flags);
return idempotent_complete(&idem, ret);
}
@@ -3217,15 +3225,16 @@ static int idempotent_init_module(struct file *f, const char __user * uargs, int
return idempotent_wait_for_completion(&idem);
}
-SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
+static int _ksys_finit_module(struct file *f, int fd, const char __user * uargs,
+ const char *kargs, int flags)
{
int err;
- struct fd f;
err = may_init_module();
if (err)
return err;
+ /* fd = -1 if called from the kernel. */
pr_debug("finit_module: fd=%d, uargs=%p, flags=%i\n", fd, uargs, flags);
if (flags & ~(MODULE_INIT_IGNORE_MODVERSIONS
@@ -3233,8 +3242,22 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
|MODULE_INIT_COMPRESSED_FILE))
return -EINVAL;
+ err = idempotent_init_module(f, uargs, kargs, flags);
+ return err;
+}
+
+int ksys_finit_module(struct file *f, const char *kargs, int flags)
+{
+ return _ksys_finit_module(f, -1, NULL, kargs, flags);
+}
+
+SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
+{
+ int err;
+ struct fd f;
+
f = fdget(fd);
- err = idempotent_init_module(fd_file(f), uargs, flags);
+ err = _ksys_finit_module(fd_file(f), fd, uargs, NULL, flags);
fdput(f);
return err;
}
--
2.47.0.118.gfd3785337b
^ permalink raw reply related
* [PATCH v6 00/15] integrity: Introduce the Integrity Digest Cache
From: Roberto Sassu @ 2024-11-19 10:49 UTC (permalink / raw)
To: zohar, dmitry.kasatkin, eric.snowberg, corbet, mcgrof, petr.pavlu,
samitolvanen, da.gomez, akpm, paul, jmorris, serge, shuah,
mcoquelin.stm32, alexandre.torgue
Cc: linux-integrity, linux-doc, linux-kernel, linux-api,
linux-modules, linux-security-module, linux-kselftest, wufan,
pbrobinson, zbyszek, hch, mjg59, pmatilai, jannh, dhowells, jikos,
mkoutny, ppavlu, petr.vorel, mzerqung, kgold, Roberto Sassu
From: Roberto Sassu <roberto.sassu@huawei.com>
Integrity detection and protection has long been a desirable feature, to
reach a large user base and mitigate the risk of flaws in the software
and attacks.
However, while solutions exist, they struggle to reach a large user base,
due to requiring higher than desired constraints on performance,
flexibility and configurability, that only security conscious people are
willing to accept.
For example, IMA measurement requires the target platform to collect
integrity measurements, and to protect them with the TPM, which introduces
a noticeable overhead (up to 10x slower in a microbenchmark) on frequently
used system calls, like the open().
IMA Appraisal currently requires individual files to be signed and
verified, and Linux distributions to rebuild all packages to include file
signatures (this approach has been adopted from Fedora 39+). Like a TPM,
also signature verification introduces a significant overhead, especially
if it is used to check the integrity of many files.
This is where the new Integrity Digest Cache comes into play, it offers
additional support for new and existing integrity solutions, to make
them faster and easier to deploy.
The Integrity Digest Cache can help IMA to reduce the number of TPM
operations and to make them happen in a deterministic way. If IMA knows
that a file comes from a Linux distribution, it can measure files in a
different way: measure the list of digests coming from the distribution
(e.g. RPM package headers), and subsequently measure a file if it is not
found in that list.
The performance improvement comes at the cost of IMA not reporting which
files from installed packages were accessed, and in which temporal
sequence. This approach might not be suitable for all use cases.
The Integrity Digest Cache can also help IMA for appraisal. IMA can simply
lookup the calculated digest of an accessed file in the list of digests
extracted from package headers, after verifying the header signature. It is
sufficient to verify only one signature for all files in the package, as
opposed to verifying a signature for each file.
The same approach can be followed by other LSMs, such as Integrity Policy
Enforcement (IPE), and BPF LSM.
The Integrity Digest Cache is not tied to a specific package format. The
kernel supports a TLV-based digest list format. More can be added through
third-party kernel modules. The TLV parser has been verified for memory
safety with the Frama-C static analyzer. The version with the Frama-C
assertions is available here:
https://github.com/robertosassu/rpm-formal/blob/main/validate_tlv.c
Integrating the Integrity Digest Cache in IMA brings significant
performance improvements: up to 67% and 79% for measurement respectively in
sequential and parallel file reads; up to 65% and 43% for appraisal
respectively in sequential and parallel file reads.
The performance can be further enhanced by using fsverity digests instead
of conventional file digests, which would make IMA verify only the portion
of the file to be read. However, at the moment, fsverity digests are not
included in RPM packages. In this case, once rpm is extended to include
them, Linux distributions still have to rebuild their packages.
The Integrity Digest Cache can support both digest types, so that the
functionality is immediately available without waiting for Linux
distributions to do the transition.
This patch set only includes the patches necessary to extract digests from
a TLV-based data format, and exposes an API for LSMs to query them. A
separate patch set will be provided to integrate it in IMA.
This patch set and the follow-up IMA integration can be tested by following
the instructions at:
https://github.com/linux-integrity/digest-cache-tools
This patch set applies on top of:
https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/log/?h=next-integrity
with commit 08ae3e5f5fc8 ("integrity: Use static_assert() to check struct
sizes").
Changelog
v5:
- Remove the RPM parser and selftests (suggested by Linus)
- Return digest cache pointer from digest_cache_lookup()
- Export new Parser API, and allow registration of third-party digest list
parsers (suggested by Mimi)
- Reduce sizes in TLV format and remove TLV header (suggested by Jani
Nikula)
- Introduce new DIGEST_LIST_NUM_ENTRIES TLV field
- Pass file descriptor instead of dentry in digest_cache_get() to properly
detect potential deadlocks
- Introduce digest_cache_opened_fd() to tell lockdep when it is safe to
nest a mutex if digest_cache_get() is called with that mutex held
- Add new patch to introduce ksys_finit_module()
- Make the TLV parser as configurable (Y/N/m) with Kconfig (suggested by
Mimi)
- Don't store the path structure in the digest cache and pass it between
creation and initialization of the digest cache
- Remove digest_cache_dir_update_dig_user() and keep the digest cache
retrieved during digest_cache_get()
- Fail with an error pointer in digest_cache_dir_lookup_digest() if the
current and passed directory digest cache don't match, or the digest
cache was reset
- Handle num_digest = 0 in digest_cache_htable_init()
- Accept -EOPNOTSUPP error in digest_cache_new()
- Implement inode_free_security_rcu LSM hook instead of inode_free_security
- Move reservation of file descriptor security blob inside the #ifdef in
init_ima_lsm()
- Add test file_reset_again to check the error pointer returned by
digest_cache_lookup()
- Remove TLV_FAILURE_HDR_LEN TLV error test
- Add missing MODULE_DESCRIPTION in kselftest kernel module (suggested by
Jeff Johnson)
- Replace dentry_open() with kernel_file_open() in populate.c and dir.c
- Skip affected tests when CONFIG_DYNAMIC_FTRACE_WITH_ARGS=n
v4:
- Rename digest_cache LSM to Integrity Digest Cache (suggested by Paul
Moore)
- Update documentation
- Remove forward declaration of struct digest_cache in
include/linux/digest_cache.h (suggested by Jarkko)
- Add DIGEST_CACHE_FREE digest cache event for notification
- Remove digest_cache_found_t typedef and use uintptr_t instead
- Add header callback in TLV parser and unexport tlv_parse_hdr() and
tlv_parse_data()
- Plug the Integrity Digest Cache into the 'ima' LSM
- Switch from constructor to zeroing the object cache
- Remove notifier and detect digest cache changes by comparing pointers
- Rename digest_cache_dir_create() to digest_cache_dir_add_entries()
- Introduce digest_cache_dir_create() to create and initialize a directory
digest cache
- Introduce digest_cache_dir_update_dig_user() to update dig_user with a
file digest cache on positive digest lookup
- Use up to date directory digest cache, to take into account possible
inode eviction for the old ones
- Introduce digest_cache_dir_prefetch() to prefetch digest lists
- Adjust component name in debug messages (suggested by Jarkko)
- Add FILE_PREFETCH and FILE_READ digest cache flags, remove RESET_USER
- Reintroduce spin lock for digest cache verification data (needed for the
selftests)
- Get inode and file descriptor security blob offsets from outside (IMA)
- Avoid user-after-free in digest_cache_unref() by decrementing the ref.
count after printing the debug message
- Check for digest list lookup loops also for the parent directory
- Put and clear dig_owner directly in digest_cache_reset_clear_owner()
- Move digest cache initialization code from digest_cache_create() to
digest_cache_init()
- Hold the digest list path until the digest cache is initialized (to avoid
premature inode eviction)
- Avoid race condition on setting DIR_PREFETCH in the directory digest
cache
- Introduce digest_cache_dir_prefetch() and do it between digest cache
creation and initialization (to avoid lock inversion)
- Avoid unnecessary length check in digest_list_parse_rpm()
- Declare arrays of strings in tlv parser as static
- Emit reset for parent directory on directory entry modification
- Rename digest_cache_reset_owner() to digest_cache_reset_clear_owner()
and digest_cache_reset_user() to digest_cache_clear_user()
- Execute digest_cache_file_release() either if FMODE_WRITE or
FMODE_CREATED are set in the file descriptor f_mode
- Determine in digest_cache_verif_set() which gfp flag to use depending on
verifier ID
- Update selftests
v3:
- Rewrite documentation, and remove the installation instructions since
they are now included in the README of digest-cache-tools
- Add digest cache event notifier
- Drop digest_cache_was_reset(), and send instead to asynchronous
notifications
- Fix digest_cache LSM Kconfig style issues (suggested by Randy Dunlap)
- Propagate digest cache reset to directory entries
- Destroy per directory entry mutex
- Introduce RESET_USER bit, to clear the dig_user pointer on
set/removexattr
- Replace 'file content' with 'file data' (suggested by Mimi)
- Introduce per digest cache mutex and replace verif_data_lock spinlock
- Track changes of security.digest_list xattr
- Stop tracking file_open and use file_release instead also for file writes
- Add error messages in digest_cache_create()
- Load/unload testing kernel module automatically during execution of test
- Add tests for digest cache event notifier
- Add test for ftruncate()
- Remove DIGEST_CACHE_RESET_PREFETCH_BUF command in test and clear the
buffer on read instead
v2:
- Include the TLV parser in this patch set (from user asymmetric keys and
signatures)
- Move from IMA and make an independent LSM
- Remove IMA-specific stuff from this patch set
- Add per algorithm hash table
- Expect all digest lists to be in the same directory and allow changing
the default directory
- Support digest lookup on directories, when there is no
security.digest_list xattr
- Add seq num to digest list file name, to impose ordering on directory
iteration
- Add a new data type DIGEST_LIST_ENTRY_DATA for the nested data in the
tlv digest list format
- Add the concept of verification data attached to digest caches
- Add the reset mechanism to track changes on digest lists and directory
containing the digest lists
- Add kernel selftests
v1:
- Add documentation in Documentation/security/integrity-digest-cache.rst
- Pass the mask of IMA actions to digest_cache_alloc()
- Add a reference count to the digest cache
- Remove the path parameter from digest_cache_get(), and rely on the
reference count to avoid the digest cache disappearing while being used
- Rename the dentry_to_check parameter of digest_cache_get() to dentry
- Rename digest_cache_get() to digest_cache_new() and add
digest_cache_get() to set the digest cache in the iint of the inode for
which the digest cache was requested
- Add dig_owner and dig_user to the iint, to distinguish from which inode
the digest cache was created from, and which is using it; consequently it
makes the digest cache usable to measure/appraise other digest caches
(support not yet enabled)
- Add dig_owner_mutex and dig_user_mutex to serialize accesses to dig_owner
and dig_user until they are initialized
- Enforce strong synchronization and make the contenders wait until
dig_owner and dig_user are assigned to the iint the first time
- Move checking IMA actions on the digest list earlier, and fail if no
action were performed (digest cache not usable)
- Remove digest_cache_put(), not needed anymore with the introduction of
the reference count
- Fail immediately in digest_cache_lookup() if the digest algorithm is
not set in the digest cache
- Use 64 bit mask for IMA actions on the digest list instead of 8 bit
- Return NULL in the inline version of digest_cache_get()
- Use list_add_tail() instead of list_add() in the iterator
- Copy the digest list path to a separate buffer in digest_cache_iter_dir()
- Use digest list parsers verified with Frama-C
- Explicitly disable (for now) the possibility in the IMA policy to use the
digest cache to measure/appraise other digest lists
- Replace exit(<value>) with return <value> in manage_digest_lists.c
Roberto Sassu (15):
lib: Add TLV parser
module: Introduce ksys_finit_module()
integrity: Introduce the Integrity Digest Cache
digest_cache: Initialize digest caches
digest_cache: Add securityfs interface
digest_cache: Add hash tables and operations
digest_cache: Allow registration of digest list parsers
digest_cache: Parse tlv digest lists
digest_cache: Populate the digest cache from a digest list
digest_cache: Add management of verification data
digest_cache: Add support for directories
digest cache: Prefetch digest lists if requested
digest_cache: Reset digest cache on file/directory change
selftests/digest_cache: Add selftests for the Integrity Digest Cache
docs: Add documentation of the Integrity Digest Cache
Documentation/security/digest_cache.rst | 850 ++++++++++++++++++
Documentation/security/index.rst | 1 +
MAINTAINERS | 10 +
include/linux/digest_cache.h | 131 +++
include/linux/kernel_read_file.h | 1 +
include/linux/syscalls.h | 10 +
include/linux/tlv_parser.h | 32 +
include/uapi/linux/tlv_digest_list.h | 47 +
include/uapi/linux/tlv_parser.h | 41 +
include/uapi/linux/xattr.h | 6 +
kernel/module/main.c | 43 +-
lib/Kconfig | 3 +
lib/Makefile | 2 +
lib/tlv_parser.c | 87 ++
lib/tlv_parser.h | 18 +
security/integrity/Kconfig | 1 +
security/integrity/Makefile | 1 +
security/integrity/digest_cache/Kconfig | 43 +
security/integrity/digest_cache/Makefile | 11 +
security/integrity/digest_cache/dir.c | 400 +++++++++
security/integrity/digest_cache/htable.c | 260 ++++++
security/integrity/digest_cache/internal.h | 283 ++++++
security/integrity/digest_cache/main.c | 597 ++++++++++++
security/integrity/digest_cache/modsig.c | 66 ++
security/integrity/digest_cache/parsers.c | 257 ++++++
security/integrity/digest_cache/parsers/tlv.c | 341 +++++++
security/integrity/digest_cache/populate.c | 104 +++
security/integrity/digest_cache/reset.c | 227 +++++
security/integrity/digest_cache/secfs.c | 104 +++
security/integrity/digest_cache/verif.c | 135 +++
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_fs.c | 6 +
security/integrity/ima/ima_main.c | 10 +-
tools/testing/selftests/Makefile | 1 +
.../testing/selftests/digest_cache/.gitignore | 3 +
tools/testing/selftests/digest_cache/Makefile | 24 +
.../testing/selftests/digest_cache/all_test.c | 769 ++++++++++++++++
tools/testing/selftests/digest_cache/common.c | 78 ++
tools/testing/selftests/digest_cache/common.h | 93 ++
.../selftests/digest_cache/common_user.c | 33 +
.../selftests/digest_cache/common_user.h | 15 +
tools/testing/selftests/digest_cache/config | 2 +
.../selftests/digest_cache/generators.c | 130 +++
.../selftests/digest_cache/generators.h | 16 +
.../selftests/digest_cache/testmod/Makefile | 16 +
.../selftests/digest_cache/testmod/kern.c | 551 ++++++++++++
46 files changed, 5849 insertions(+), 11 deletions(-)
create mode 100644 Documentation/security/digest_cache.rst
create mode 100644 include/linux/digest_cache.h
create mode 100644 include/linux/tlv_parser.h
create mode 100644 include/uapi/linux/tlv_digest_list.h
create mode 100644 include/uapi/linux/tlv_parser.h
create mode 100644 lib/tlv_parser.c
create mode 100644 lib/tlv_parser.h
create mode 100644 security/integrity/digest_cache/Kconfig
create mode 100644 security/integrity/digest_cache/Makefile
create mode 100644 security/integrity/digest_cache/dir.c
create mode 100644 security/integrity/digest_cache/htable.c
create mode 100644 security/integrity/digest_cache/internal.h
create mode 100644 security/integrity/digest_cache/main.c
create mode 100644 security/integrity/digest_cache/modsig.c
create mode 100644 security/integrity/digest_cache/parsers.c
create mode 100644 security/integrity/digest_cache/parsers/tlv.c
create mode 100644 security/integrity/digest_cache/populate.c
create mode 100644 security/integrity/digest_cache/reset.c
create mode 100644 security/integrity/digest_cache/secfs.c
create mode 100644 security/integrity/digest_cache/verif.c
create mode 100644 tools/testing/selftests/digest_cache/.gitignore
create mode 100644 tools/testing/selftests/digest_cache/Makefile
create mode 100644 tools/testing/selftests/digest_cache/all_test.c
create mode 100644 tools/testing/selftests/digest_cache/common.c
create mode 100644 tools/testing/selftests/digest_cache/common.h
create mode 100644 tools/testing/selftests/digest_cache/common_user.c
create mode 100644 tools/testing/selftests/digest_cache/common_user.h
create mode 100644 tools/testing/selftests/digest_cache/config
create mode 100644 tools/testing/selftests/digest_cache/generators.c
create mode 100644 tools/testing/selftests/digest_cache/generators.h
create mode 100644 tools/testing/selftests/digest_cache/testmod/Makefile
create mode 100644 tools/testing/selftests/digest_cache/testmod/kern.c
--
2.47.0.118.gfd3785337b
^ permalink raw reply
* A Proposal
From: William Cheung @ 2024-11-18 22:38 UTC (permalink / raw)
To: linux-api
I have a lucratuve proposal for you, reply for more info.
^ permalink raw reply
* Re: [PATCH net-next v2 12/12] net: homa: create Makefile and Kconfig
From: John Ousterhout @ 2024-11-18 21:23 UTC (permalink / raw)
To: kernel test robot; +Cc: netdev, linux-api, oe-kbuild-all
In-Reply-To: <202411132114.VB5yFmtR-lkp@intel.com>
I believe I have fixed all of the kernel test robot issues reported
under this subject line.
-John-
On Wed, Nov 13, 2024 at 5:53 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi John,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on net-next/main]
>
> url: https://github.com/intel-lab-lkp/linux/commits/John-Ousterhout/net-homa-define-user-visible-API-for-Homa/20241112-074535
> base: net-next/main
> patch link: https://lore.kernel.org/r/20241111234006.5942-13-ouster%40cs.stanford.edu
> patch subject: [PATCH net-next v2 12/12] net: homa: create Makefile and Kconfig
> config: riscv-randconfig-r112-20241113 (https://download.01.org/0day-ci/archive/20241113/202411132114.VB5yFmtR-lkp@intel.com/config)
> compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
> reproduce: (https://download.01.org/0day-ci/archive/20241113/202411132114.VB5yFmtR-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202411132114.VB5yFmtR-lkp@intel.com/
>
> sparse warnings: (new ones prefixed by >>)
> >> net/homa/homa_sock.c:201:31: sparse: sparse: cast removes address space '__rcu' of expression
> net/homa/homa_sock.c:248:17: sparse: sparse: context imbalance in 'homa_sock_shutdown' - different lock contexts for basic block
> net/homa/homa_sock.c:303:21: sparse: sparse: context imbalance in 'homa_sock_bind' - different lock contexts for basic block
>
> vim +/__rcu +201 net/homa/homa_sock.c
>
> 8ddf00265eb650 John Ousterhout 2024-11-11 183
> 8ddf00265eb650 John Ousterhout 2024-11-11 184 /*
> 8ddf00265eb650 John Ousterhout 2024-11-11 185 * homa_sock_unlink() - Unlinks a socket from its socktab and does
> 8ddf00265eb650 John Ousterhout 2024-11-11 186 * related cleanups. Once this method returns, the socket will not be
> 8ddf00265eb650 John Ousterhout 2024-11-11 187 * discoverable through the socktab.
> 8ddf00265eb650 John Ousterhout 2024-11-11 188 */
> 8ddf00265eb650 John Ousterhout 2024-11-11 189 void homa_sock_unlink(struct homa_sock *hsk)
> 8ddf00265eb650 John Ousterhout 2024-11-11 190 {
> 8ddf00265eb650 John Ousterhout 2024-11-11 191 struct homa_socktab *socktab = hsk->homa->port_map;
> 8ddf00265eb650 John Ousterhout 2024-11-11 192 struct homa_socktab_scan *scan;
> 8ddf00265eb650 John Ousterhout 2024-11-11 193
> 8ddf00265eb650 John Ousterhout 2024-11-11 194 /* If any scans refer to this socket, advance them to refer to
> 8ddf00265eb650 John Ousterhout 2024-11-11 195 * the next socket instead.
> 8ddf00265eb650 John Ousterhout 2024-11-11 196 */
> 8ddf00265eb650 John Ousterhout 2024-11-11 197 spin_lock_bh(&socktab->write_lock);
> 8ddf00265eb650 John Ousterhout 2024-11-11 198 list_for_each_entry(scan, &socktab->active_scans, scan_links) {
> 8ddf00265eb650 John Ousterhout 2024-11-11 199 if (!scan->next || scan->next->sock != hsk)
> 8ddf00265eb650 John Ousterhout 2024-11-11 200 continue;
> 8ddf00265eb650 John Ousterhout 2024-11-11 @201 scan->next = (struct homa_socktab_links *)hlist_next_rcu(&scan
> 8ddf00265eb650 John Ousterhout 2024-11-11 202 ->next->hash_links);
> 8ddf00265eb650 John Ousterhout 2024-11-11 203 }
> 8ddf00265eb650 John Ousterhout 2024-11-11 204 hlist_del_rcu(&hsk->socktab_links.hash_links);
> 8ddf00265eb650 John Ousterhout 2024-11-11 205 spin_unlock_bh(&socktab->write_lock);
> 8ddf00265eb650 John Ousterhout 2024-11-11 206 }
> 8ddf00265eb650 John Ousterhout 2024-11-11 207
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] statmount: add flag to retrieve unescaped options
From: Christian Brauner @ 2024-11-15 8:10 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Miklos Szeredi, linux-fsdevel, linux-api, Karel Zak,
Christian Brauner, Josef Bacik
In-Reply-To: <CAJfpegtLiOjbtP4np-WjJ_oyC-u3FwZ4BWQxGkSSmWxurBOQdA@mail.gmail.com>
On Thu, Nov 14, 2024 at 03:53:44PM +0100, Miklos Szeredi wrote:
> On Wed, 13 Nov 2024 at 17:31, Christian Brauner <brauner@kernel.org> wrote:
>
> > Please take a look at the top of #vfs.misc and tell me whether this is
> > ok with you.
>
> One more thing I'd do is:
>
> - if (seq->count == start)
> + if (seq->count <= start + 1)
>
> This would handle the (broken) case of just a single comma in the
> options. So it's not a bug fix per-se, but would make it clear that
> the loop will run at least once, and so the seq->count calculation at
> the end won't be skewed.
>
> The buf_start calculation could also be moved further down before the
> loop, where it's actually used.
>
> I don't find the variable naming much better, how about taking the
> naming from string_unescape:
>
> opt_start -> src - pointer to escaped string
> buf_start -> dst - pointer to de-escaped string
>
> The others make sense.
Fine by me. I'm about to send pull-requests so I would suggest you send
a cleanup that I can include post-rc1 so we don't have to rebase right
now.
^ permalink raw reply
* Re: [PATCH net 1/1] net/ipv6: Netlink flag for new IPv6 Default Routes
From: Matt Muggeridge @ 2024-11-14 20:42 UTC (permalink / raw)
To: idosch
Cc: Matt.Muggeridge, davem, dsahern, edumazet, horms, kuba, linux-api,
linux-kernel, netdev, pabeni, stable
In-Reply-To: <ZyyN2bSgrpbhbkpp@shredder>
Thank you for your review and feedback, Ido.
>> Without this flag, when there are mutliple default routers, the kernel
>> coalesces multiple default routes into an ECMP route. The ECMP route
>> ignores per-route REACHABILITY information. If one of the default
>> routers is unresponsive, with a Neighbor Cache entry of INCOMPLETE, then
>> it can still be selected as the nexthop for outgoing packets. This
>> results in an inability to communicate with remote hosts, even though
>> one of the default routers remains REACHABLE. This violates RFC4861
>> section 6.3.6, bullet 1.
>
>Do you have forwarding disabled (it causes RT6_LOOKUP_F_REACHABLE to be
>set)?
Yes, forwarding is disabled on our embedded system. Though, this needs to
work on systems regardless of the state of forwarding.
> Is the problem that fib6_table_lookup() chooses a reachable
>nexthop and then fib6_select_path() overrides it with an unreachable
>one?
I'm afraid I don't know.
The objective is to allow IPv6 Netlink clients to be able to create default
routes from RAs in the same way the kernel creates default routes from RAs.
Essentially, I'm trying to have Netlink and Kernel behaviors match.
My analysis led me to the need for Netlink clients to set the kernel's
fib6_config flags RTF_RA_ROUTER, where:
#define RTF_RA_ROUTER (RTF_ADDRCONF | RTF_DEFAULT)
>> + if (rtm->rtm_flags & RTM_F_RA_ROUTER)
>> + cfg->fc_flags |= RTF_RA_ROUTER;
>> +
>
> It is possible there are user space programs out there that set this bit
> (knowingly or not) when sending requests to the kernel and this change
> will result in a behavior change for them. So, if we were to continue in
> this path, this would need to be converted to a new netlink attribute to
> avoid such potential problems.
>
Is this a mandated approach to implementing unspecified bits in a flag?
I'm a little surprised by this consideration. If we account for poorly
written buggy user-programs, doesn't this open any API to an explosion
of new attributes or other odd extensions? I'd imagine the same argument
would be applicable to ioctl flags, socket flags, and so on. Why would we
treat implementing unspecified Netlink bits differently to implementing
unspecified ioctl bits, etc.
Naturally, if this is the mandated approach, then I'll reimplement it with
a new Netlink attribute. I'm just trying to understand what is the
Linux-lore, here?
> BTW, you can avoid the coalescing problem by using the nexthop API (man
> ip-nexthop).
I'm not sure how that would help in this case. We need the nexthop to be
determined according to its REACHABILITY and other considerations described
in RFC4861.
Kind regards,
Matt.
^ permalink raw reply
* Re: [PATCH net-next v2 00/12] Begin upstreaming Homa transport protocol
From: Randy Dunlap @ 2024-11-14 19:36 UTC (permalink / raw)
To: John Ousterhout, Cong Wang; +Cc: netdev, linux-api
In-Reply-To: <CAGXJAmyGTwjFo6fGoROY=hQFXbR5RdpmpkEc9Zm6DOoD2nbwNA@mail.gmail.com>
On 11/14/24 8:59 AM, John Ousterhout wrote:
> On Wed, Nov 13, 2024 at 9:08 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>
>> On Mon, Nov 11, 2024 at 03:39:53PM -0800, John Ousterhout wrote:
>>> MAINTAINERS | 7 +
>>> include/uapi/linux/homa.h | 165 ++++++
>>> net/Kconfig | 1 +
>>> net/Makefile | 1 +
>>> net/homa/Kconfig | 19 +
>>> net/homa/Makefile | 14 +
>>> net/homa/homa_impl.h | 767 ++++++++++++++++++++++++++
>>> net/homa/homa_incoming.c | 1076 +++++++++++++++++++++++++++++++++++++
>>> net/homa/homa_outgoing.c | 854 +++++++++++++++++++++++++++++
>>> net/homa/homa_peer.c | 319 +++++++++++
>>> net/homa/homa_peer.h | 234 ++++++++
>>> net/homa/homa_plumbing.c | 965 +++++++++++++++++++++++++++++++++
>>> net/homa/homa_pool.c | 420 +++++++++++++++
>>> net/homa/homa_pool.h | 152 ++++++
>>> net/homa/homa_rpc.c | 488 +++++++++++++++++
>>> net/homa/homa_rpc.h | 446 +++++++++++++++
>>> net/homa/homa_sock.c | 380 +++++++++++++
>>> net/homa/homa_sock.h | 426 +++++++++++++++
>>> net/homa/homa_stub.h | 80 +++
>>> net/homa/homa_timer.c | 156 ++++++
>>> net/homa/homa_utils.c | 150 ++++++
>>> net/homa/homa_wire.h | 378 +++++++++++++
>>
>> Hi John,
>>
>> Thanks for your efforts to push them upstream!
>>
>> Just some very high-level comments:
>>
>> 1. Please run scripts/checkpatch.pl to make sure the coding style is
>> aligned with upstream, since I noticed there are still some C++ style
>> comments in your patchset.
>
> I have been running checkpatch.pl, but it didn't complain about C++
> style comments. Those comments really shouldn't be there, though: they
> are for pieces of code I've temporarily commented out, and those
> chunks shouldn't be in the upstream version of Homa. I'll fix things
> so they don't appear in the future.
>
John, you should be OK using // comments if that's what you want to use.
https://lkml.iu.edu/hypermail/linux/kernel/1607.1/00627.html
or
https://lore.kernel.org/lkml/CA+55aFyQYJerovMsSoSKS7PessZBr4vNp-3QUUwhqk4A4_jcbg@mail.gmail.com/
are still current AFAIK.
--
~Randy
^ permalink raw reply
* Re: [PATCH net-next v2 00/12] Begin upstreaming Homa transport protocol
From: John Ousterhout @ 2024-11-14 16:59 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, linux-api
In-Reply-To: <ZzTcx8nmEKIJpaCR@pop-os.localdomain>
On Wed, Nov 13, 2024 at 9:08 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Mon, Nov 11, 2024 at 03:39:53PM -0800, John Ousterhout wrote:
> > MAINTAINERS | 7 +
> > include/uapi/linux/homa.h | 165 ++++++
> > net/Kconfig | 1 +
> > net/Makefile | 1 +
> > net/homa/Kconfig | 19 +
> > net/homa/Makefile | 14 +
> > net/homa/homa_impl.h | 767 ++++++++++++++++++++++++++
> > net/homa/homa_incoming.c | 1076 +++++++++++++++++++++++++++++++++++++
> > net/homa/homa_outgoing.c | 854 +++++++++++++++++++++++++++++
> > net/homa/homa_peer.c | 319 +++++++++++
> > net/homa/homa_peer.h | 234 ++++++++
> > net/homa/homa_plumbing.c | 965 +++++++++++++++++++++++++++++++++
> > net/homa/homa_pool.c | 420 +++++++++++++++
> > net/homa/homa_pool.h | 152 ++++++
> > net/homa/homa_rpc.c | 488 +++++++++++++++++
> > net/homa/homa_rpc.h | 446 +++++++++++++++
> > net/homa/homa_sock.c | 380 +++++++++++++
> > net/homa/homa_sock.h | 426 +++++++++++++++
> > net/homa/homa_stub.h | 80 +++
> > net/homa/homa_timer.c | 156 ++++++
> > net/homa/homa_utils.c | 150 ++++++
> > net/homa/homa_wire.h | 378 +++++++++++++
>
> Hi John,
>
> Thanks for your efforts to push them upstream!
>
> Just some very high-level comments:
>
> 1. Please run scripts/checkpatch.pl to make sure the coding style is
> aligned with upstream, since I noticed there are still some C++ style
> comments in your patchset.
I have been running checkpatch.pl, but it didn't complain about C++
style comments. Those comments really shouldn't be there, though: they
are for pieces of code I've temporarily commented out, and those
chunks shouldn't be in the upstream version of Homa. I'll fix things
so they don't appear in the future.
> 2. Please consider adding socket diagnostics, see net/ipv4/inet_diag.c.
I wasn't familiar with them before your email; I'll take a look.
> 3. Please consider adding test cases, you can pretty much follow
> tools/testing/vsock/vsock_test.c.
Homa has extensive unit tests in the GitHub repo, but I thought I'd
wait to try upstreaming them until all of Homa has been upstreamed
(they are based on a modified version of kselftest.h, so they may be a
bit controversial). One way or another, though, I'm committed to
having good unit tests for Homa.
Thank you for your feedback.
-John-
^ permalink raw reply
* Re: [PATCH] statmount: add flag to retrieve unescaped options
From: Miklos Szeredi @ 2024-11-14 14:53 UTC (permalink / raw)
To: Christian Brauner
Cc: Miklos Szeredi, linux-fsdevel, linux-api, Karel Zak,
Christian Brauner, Josef Bacik
In-Reply-To: <20241113-wandmalerei-haben-9b19b61e5118@brauner>
On Wed, 13 Nov 2024 at 17:31, Christian Brauner <brauner@kernel.org> wrote:
> Please take a look at the top of #vfs.misc and tell me whether this is
> ok with you.
One more thing I'd do is:
- if (seq->count == start)
+ if (seq->count <= start + 1)
This would handle the (broken) case of just a single comma in the
options. So it's not a bug fix per-se, but would make it clear that
the loop will run at least once, and so the seq->count calculation at
the end won't be skewed.
The buf_start calculation could also be moved further down before the
loop, where it's actually used.
I don't find the variable naming much better, how about taking the
naming from string_unescape:
opt_start -> src - pointer to escaped string
buf_start -> dst - pointer to de-escaped string
The others make sense.
Thanks,
Miklos
^ permalink raw reply
* Re: [PATCH net-next v2 00/12] Begin upstreaming Homa transport protocol
From: John Ousterhout @ 2024-11-13 18:18 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, linux-api
In-Reply-To: <20241112174834.43231a32@kernel.org>
On Tue, Nov 12, 2024 at 5:48 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 11 Nov 2024 15:39:53 -0800 John Ousterhout wrote:
> > This patch series begins the process of upstreaming the Homa transport
> > protocol. Homa is an alternative to TCP for use in datacenter
> > environments. It provides 10-100x reductions in tail latency for short
> > messages relative to TCP. Its benefits are greatest for mixed workloads
> > containing both short and long messages running under high network loads.
> > Homa is not API-compatible with TCP: it is connectionless and message-
> > oriented (but still reliable and flow-controlled). Homa's new API not
> > only contributes to its performance gains, but it also eliminates the
> > massive amount of connection state required by TCP for highly connected
> > datacenter workloads.
>
> Hi John!
>
> Thanks for pushing forward! I wanted to give you a heads up that we're
> operating at 50% maintainer capacity for the next 2 weeks so the
> reviews may be more muted than usual. Don't hesitate to post new
> versions (typically ~once a week) if you want to address any incoming
> feedback and/or the build issues.
Thanks for letting me know.
-John-
^ permalink raw reply
* Re: [PATCH net-next v2 00/12] Begin upstreaming Homa transport protocol
From: Cong Wang @ 2024-11-13 17:07 UTC (permalink / raw)
To: John Ousterhout; +Cc: netdev, linux-api
In-Reply-To: <20241111234006.5942-1-ouster@cs.stanford.edu>
On Mon, Nov 11, 2024 at 03:39:53PM -0800, John Ousterhout wrote:
> MAINTAINERS | 7 +
> include/uapi/linux/homa.h | 165 ++++++
> net/Kconfig | 1 +
> net/Makefile | 1 +
> net/homa/Kconfig | 19 +
> net/homa/Makefile | 14 +
> net/homa/homa_impl.h | 767 ++++++++++++++++++++++++++
> net/homa/homa_incoming.c | 1076 +++++++++++++++++++++++++++++++++++++
> net/homa/homa_outgoing.c | 854 +++++++++++++++++++++++++++++
> net/homa/homa_peer.c | 319 +++++++++++
> net/homa/homa_peer.h | 234 ++++++++
> net/homa/homa_plumbing.c | 965 +++++++++++++++++++++++++++++++++
> net/homa/homa_pool.c | 420 +++++++++++++++
> net/homa/homa_pool.h | 152 ++++++
> net/homa/homa_rpc.c | 488 +++++++++++++++++
> net/homa/homa_rpc.h | 446 +++++++++++++++
> net/homa/homa_sock.c | 380 +++++++++++++
> net/homa/homa_sock.h | 426 +++++++++++++++
> net/homa/homa_stub.h | 80 +++
> net/homa/homa_timer.c | 156 ++++++
> net/homa/homa_utils.c | 150 ++++++
> net/homa/homa_wire.h | 378 +++++++++++++
Hi John,
Thanks for your efforts to push them upstream!
Just some very high-level comments:
1. Please run scripts/checkpatch.pl to make sure the coding style is
aligned with upstream, since I noticed there are still some C++ style
comments in your patchset.
2. Please consider adding socket diagnostics, see net/ipv4/inet_diag.c.
3. Please consider adding test cases, you can pretty much follow
tools/testing/vsock/vsock_test.c.
Regards,
Cong
^ permalink raw reply
* Re: [PATCH] statmount: add flag to retrieve unescaped options
From: Christian Brauner @ 2024-11-13 16:30 UTC (permalink / raw)
To: Miklos Szeredi
Cc: linux-fsdevel, linux-api, Karel Zak, Christian Brauner,
Josef Bacik
In-Reply-To: <20241113-unbeobachtet-unvollendet-36c5443a042d@brauner>
On Wed, Nov 13, 2024 at 01:27:08PM +0100, Christian Brauner wrote:
> On Tue, Nov 12, 2024 at 11:10:04AM +0100, Miklos Szeredi wrote:
> > Filesystem options can be retrieved with STATMOUNT_MNT_OPTS, which
> > returns a string of comma separated options, where some characters are
> > escaped using the \OOO notation.
> >
> > Add a new flag, STATMOUNT_OPT_ARRAY, which instead returns the raw
> > option values separated with '\0' charaters.
> >
> > Since escaped charaters are rare, this inteface is preferable for
> > non-libmount users which likley don't want to deal with option
> > de-escaping.
> >
> > Example code:
> >
> > if (st->mask & STATMOUNT_OPT_ARRAY) {
> > const char *opt = st->str + st->opt_array;
> >
> > for (unsigned int i = 0; i < st->opt_num; i++) {
> > printf("opt_array[%i]: <%s>\n", i, opt);
> > opt += strlen(opt) + 1;
> > }
> > }
> >
> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > ---
>
> I'm likely going to snatch this for v6.13 still. I just need to reflow
> the code as the formatting is broken and maybe rename a few variables
> and need to wrap my head around the parsing. I'm testing this now.
Please take a look at the top of #vfs.misc and tell me whether this is
ok with you.
^ permalink raw reply
* Re: [PATCH net-next v2 12/12] net: homa: create Makefile and Kconfig
From: kernel test robot @ 2024-11-13 13:52 UTC (permalink / raw)
To: John Ousterhout, netdev, linux-api; +Cc: oe-kbuild-all, John Ousterhout
In-Reply-To: <20241111234006.5942-13-ouster@cs.stanford.edu>
Hi John,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/John-Ousterhout/net-homa-define-user-visible-API-for-Homa/20241112-074535
base: net-next/main
patch link: https://lore.kernel.org/r/20241111234006.5942-13-ouster%40cs.stanford.edu
patch subject: [PATCH net-next v2 12/12] net: homa: create Makefile and Kconfig
config: riscv-randconfig-r112-20241113 (https://download.01.org/0day-ci/archive/20241113/202411132114.VB5yFmtR-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce: (https://download.01.org/0day-ci/archive/20241113/202411132114.VB5yFmtR-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411132114.VB5yFmtR-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> net/homa/homa_sock.c:201:31: sparse: sparse: cast removes address space '__rcu' of expression
net/homa/homa_sock.c:248:17: sparse: sparse: context imbalance in 'homa_sock_shutdown' - different lock contexts for basic block
net/homa/homa_sock.c:303:21: sparse: sparse: context imbalance in 'homa_sock_bind' - different lock contexts for basic block
vim +/__rcu +201 net/homa/homa_sock.c
8ddf00265eb650 John Ousterhout 2024-11-11 183
8ddf00265eb650 John Ousterhout 2024-11-11 184 /*
8ddf00265eb650 John Ousterhout 2024-11-11 185 * homa_sock_unlink() - Unlinks a socket from its socktab and does
8ddf00265eb650 John Ousterhout 2024-11-11 186 * related cleanups. Once this method returns, the socket will not be
8ddf00265eb650 John Ousterhout 2024-11-11 187 * discoverable through the socktab.
8ddf00265eb650 John Ousterhout 2024-11-11 188 */
8ddf00265eb650 John Ousterhout 2024-11-11 189 void homa_sock_unlink(struct homa_sock *hsk)
8ddf00265eb650 John Ousterhout 2024-11-11 190 {
8ddf00265eb650 John Ousterhout 2024-11-11 191 struct homa_socktab *socktab = hsk->homa->port_map;
8ddf00265eb650 John Ousterhout 2024-11-11 192 struct homa_socktab_scan *scan;
8ddf00265eb650 John Ousterhout 2024-11-11 193
8ddf00265eb650 John Ousterhout 2024-11-11 194 /* If any scans refer to this socket, advance them to refer to
8ddf00265eb650 John Ousterhout 2024-11-11 195 * the next socket instead.
8ddf00265eb650 John Ousterhout 2024-11-11 196 */
8ddf00265eb650 John Ousterhout 2024-11-11 197 spin_lock_bh(&socktab->write_lock);
8ddf00265eb650 John Ousterhout 2024-11-11 198 list_for_each_entry(scan, &socktab->active_scans, scan_links) {
8ddf00265eb650 John Ousterhout 2024-11-11 199 if (!scan->next || scan->next->sock != hsk)
8ddf00265eb650 John Ousterhout 2024-11-11 200 continue;
8ddf00265eb650 John Ousterhout 2024-11-11 @201 scan->next = (struct homa_socktab_links *)hlist_next_rcu(&scan
8ddf00265eb650 John Ousterhout 2024-11-11 202 ->next->hash_links);
8ddf00265eb650 John Ousterhout 2024-11-11 203 }
8ddf00265eb650 John Ousterhout 2024-11-11 204 hlist_del_rcu(&hsk->socktab_links.hash_links);
8ddf00265eb650 John Ousterhout 2024-11-11 205 spin_unlock_bh(&socktab->write_lock);
8ddf00265eb650 John Ousterhout 2024-11-11 206 }
8ddf00265eb650 John Ousterhout 2024-11-11 207
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] statmount: add flag to retrieve unescaped options
From: Christian Brauner @ 2024-11-13 12:27 UTC (permalink / raw)
To: Miklos Szeredi
Cc: linux-fsdevel, linux-api, Karel Zak, Christian Brauner,
Josef Bacik
In-Reply-To: <20241112101006.30715-1-mszeredi@redhat.com>
On Tue, Nov 12, 2024 at 11:10:04AM +0100, Miklos Szeredi wrote:
> Filesystem options can be retrieved with STATMOUNT_MNT_OPTS, which
> returns a string of comma separated options, where some characters are
> escaped using the \OOO notation.
>
> Add a new flag, STATMOUNT_OPT_ARRAY, which instead returns the raw
> option values separated with '\0' charaters.
>
> Since escaped charaters are rare, this inteface is preferable for
> non-libmount users which likley don't want to deal with option
> de-escaping.
>
> Example code:
>
> if (st->mask & STATMOUNT_OPT_ARRAY) {
> const char *opt = st->str + st->opt_array;
>
> for (unsigned int i = 0; i < st->opt_num; i++) {
> printf("opt_array[%i]: <%s>\n", i, opt);
> opt += strlen(opt) + 1;
> }
> }
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
I'm likely going to snatch this for v6.13 still. I just need to reflow
the code as the formatting is broken and maybe rename a few variables
and need to wrap my head around the parsing. I'm testing this now.
^ permalink raw reply
* Re: [PATCH net-next v2 00/12] Begin upstreaming Homa transport protocol
From: Jakub Kicinski @ 2024-11-13 1:48 UTC (permalink / raw)
To: John Ousterhout; +Cc: netdev, linux-api
In-Reply-To: <20241111234006.5942-1-ouster@cs.stanford.edu>
On Mon, 11 Nov 2024 15:39:53 -0800 John Ousterhout wrote:
> This patch series begins the process of upstreaming the Homa transport
> protocol. Homa is an alternative to TCP for use in datacenter
> environments. It provides 10-100x reductions in tail latency for short
> messages relative to TCP. Its benefits are greatest for mixed workloads
> containing both short and long messages running under high network loads.
> Homa is not API-compatible with TCP: it is connectionless and message-
> oriented (but still reliable and flow-controlled). Homa's new API not
> only contributes to its performance gains, but it also eliminates the
> massive amount of connection state required by TCP for highly connected
> datacenter workloads.
Hi John!
Thanks for pushing forward! I wanted to give you a heads up that we're
operating at 50% maintainer capacity for the next 2 weeks so the
reviews may be more muted than usual. Don't hesitate to post new
versions (typically ~once a week) if you want to address any incoming
feedback and/or the build issues.
HTH
^ permalink raw reply
* [PATCH v21 3/6] selftests/exec: Add 32 tests for AT_EXECVE_CHECK and exec securebits
From: Mickaël Salaün @ 2024-11-12 19:18 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, Adhemerval Zanella Netto,
Alejandro Colomar, Aleksa Sarai, Andrew Morton, Andy Lutomirski,
Arnd Bergmann, Casey Schaufler, Christian Heimes, Dmitry Vyukov,
Elliott Hughes, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Linus Torvalds, Luca Boccassi, Luis Chamberlain,
Madhavan T . Venkataraman, Matt Bobrowski, Matthew Garrett,
Matthew Wilcox, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Scott Shell, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Theodore Ts'o, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20241112191858.162021-1-mic@digikod.net>
Test that checks performed by execveat(..., AT_EXECVE_CHECK) are
consistent with noexec mount points and file execute permissions.
Test that SECBIT_EXEC_RESTRICT_FILE and SECBIT_EXEC_DENY_INTERACTIVE are
inherited by child processes and that they can be pinned with the
appropriate SECBIT_EXEC_RESTRICT_FILE_LOCKED and
SECBIT_EXEC_DENY_INTERACTIVE_LOCKED bits.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241112191858.162021-4-mic@digikod.net
---
Changes since v20:
* Rename AT_CHECK to AT_EXECVE_CHECK.
Changes since v19:
* Rename securebits.
* Rename test file.
Changes since v18:
* Rewrite tests with the new design: execveat/AT_CHECK and securebits.
* Simplify the capability dropping and improve it with the NOROOT
securebits.
* Replace most ASSERT with EXPECT.
* Fix NULL execve's argv to avoid kernel warning.
* Move tests to exec/
* Build a "false" static binary to test full execution path.
Changes since v14:
* Add Reviewed-by Kees Cook.
Changes since v13:
* Move -I to CFLAGS (suggested by Kees Cook).
* Update sysctl name.
Changes since v12:
* Fix Makefile's license.
Changes since v10:
* Update selftest Makefile.
Changes since v9:
* Rename the syscall and the sysctl.
* Update tests for enum trusted_for_usage
Changes since v8:
* Update with the dedicated syscall introspect_access(2) and the renamed
fs.introspection_policy sysctl.
* Remove check symlink which can't be use as is anymore.
* Use socketpair(2) to test UNIX socket.
Changes since v7:
* Update tests with faccessat2/AT_INTERPRETED, including new ones to
check that setting R_OK or W_OK returns EINVAL.
* Add tests for memfd, pipefs and nsfs.
* Rename and move back tests to a standalone directory.
Changes since v6:
* Add full combination tests for all file types, including block
devices, character devices, fifos, sockets and symlinks.
* Properly save and restore initial sysctl value for all tests.
Changes since v5:
* Refactor with FIXTURE_VARIANT, which make the tests much more easy to
read and maintain.
* Save and restore initial sysctl value (suggested by Kees Cook).
* Test with a sysctl value of 0.
* Check errno in sysctl_access_write test.
* Update tests for the CAP_SYS_ADMIN switch.
* Update tests to check -EISDIR (replacing -EACCES).
* Replace FIXTURE_DATA() with FIXTURE() (spotted by Kees Cook).
* Use global const strings.
Changes since v3:
* Replace RESOLVE_MAYEXEC with O_MAYEXEC.
* Add tests to check that O_MAYEXEC is ignored by open(2) and openat(2).
Changes since v2:
* Move tests from exec/ to openat2/ .
* Replace O_MAYEXEC with RESOLVE_MAYEXEC from openat2(2).
* Cleanup tests.
Changes since v1:
* Move tests from yama/ to exec/ .
* Fix _GNU_SOURCE in kselftest_harness.h .
* Add a new test sysctl_access_write to check if CAP_MAC_ADMIN is taken
into account.
* Test directory execution which is always forbidden since commit
73601ea5b7b1 ("fs/open.c: allow opening only regular files during
execve()"), and also check that even the root user can not bypass file
execution checks.
* Make sure delete_workspace() always as enough right to succeed.
* Cosmetic cleanup.
---
tools/testing/selftests/exec/.gitignore | 2 +
tools/testing/selftests/exec/Makefile | 7 +
tools/testing/selftests/exec/check-exec.c | 448 ++++++++++++++++++++++
tools/testing/selftests/exec/config | 2 +
tools/testing/selftests/exec/false.c | 5 +
5 files changed, 464 insertions(+)
create mode 100644 tools/testing/selftests/exec/check-exec.c
create mode 100644 tools/testing/selftests/exec/config
create mode 100644 tools/testing/selftests/exec/false.c
diff --git a/tools/testing/selftests/exec/.gitignore b/tools/testing/selftests/exec/.gitignore
index a0dc5d4bf733..a32c63bb4df1 100644
--- a/tools/testing/selftests/exec/.gitignore
+++ b/tools/testing/selftests/exec/.gitignore
@@ -9,6 +9,8 @@ execveat.ephemeral
execveat.denatured
non-regular
null-argv
+/check-exec
+/false
/load_address.*
!load_address.c
/recursion-depth
diff --git a/tools/testing/selftests/exec/Makefile b/tools/testing/selftests/exec/Makefile
index ba012bc5aab9..8713d1c862ae 100644
--- a/tools/testing/selftests/exec/Makefile
+++ b/tools/testing/selftests/exec/Makefile
@@ -1,6 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS = -Wall
CFLAGS += -Wno-nonnull
+CFLAGS += $(KHDR_INCLUDES)
+
+LDLIBS += -lcap
ALIGNS := 0x1000 0x200000 0x1000000
ALIGN_PIES := $(patsubst %,load_address.%,$(ALIGNS))
@@ -9,12 +12,14 @@ ALIGNMENT_TESTS := $(ALIGN_PIES) $(ALIGN_STATIC_PIES)
TEST_PROGS := binfmt_script.py
TEST_GEN_PROGS := execveat non-regular $(ALIGNMENT_TESTS)
+TEST_GEN_PROGS_EXTENDED := false
TEST_GEN_FILES := execveat.symlink execveat.denatured script subdir
# Makefile is a run-time dependency, since it's accessed by the execveat test
TEST_FILES := Makefile
TEST_GEN_PROGS += recursion-depth
TEST_GEN_PROGS += null-argv
+TEST_GEN_PROGS += check-exec
EXTRA_CLEAN := $(OUTPUT)/subdir.moved $(OUTPUT)/execveat.moved $(OUTPUT)/xxxxx* \
$(OUTPUT)/S_I*.test
@@ -38,3 +43,5 @@ $(OUTPUT)/load_address.0x%: load_address.c
$(OUTPUT)/load_address.static.0x%: load_address.c
$(CC) $(CFLAGS) $(LDFLAGS) -Wl,-z,max-page-size=$(lastword $(subst ., ,$@)) \
-fPIE -static-pie $< -o $@
+$(OUTPUT)/false: false.c
+ $(CC) $(CFLAGS) $(LDFLAGS) -static $< -o $@
diff --git a/tools/testing/selftests/exec/check-exec.c b/tools/testing/selftests/exec/check-exec.c
new file mode 100644
index 000000000000..c3aa046d8d68
--- /dev/null
+++ b/tools/testing/selftests/exec/check-exec.c
@@ -0,0 +1,448 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test execveat(2) with AT_EXECVE_CHECK, and prctl(2) with
+ * SECBIT_EXEC_RESTRICT_FILE, SECBIT_EXEC_DENY_INTERACTIVE, and their locked
+ * counterparts.
+ *
+ * Copyright © 2018-2020 ANSSI
+ * Copyright © 2024 Microsoft Corporation
+ *
+ * Author: Mickaël Salaün <mic@digikod.net>
+ */
+
+#include <asm-generic/unistd.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/prctl.h>
+#include <linux/securebits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/capability.h>
+#include <sys/mount.h>
+#include <sys/prctl.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/sysmacros.h>
+#include <unistd.h>
+
+/* Defines AT_EXECVE_CHECK without type conflicts. */
+#define _ASM_GENERIC_FCNTL_H
+#include <linux/fcntl.h>
+
+#include "../kselftest_harness.h"
+
+static void drop_privileges(struct __test_metadata *const _metadata)
+{
+ const unsigned int noroot = SECBIT_NOROOT | SECBIT_NOROOT_LOCKED;
+ cap_t cap_p;
+
+ if ((cap_get_secbits() & noroot) != noroot)
+ EXPECT_EQ(0, cap_set_secbits(noroot));
+
+ cap_p = cap_get_proc();
+ EXPECT_NE(NULL, cap_p);
+ EXPECT_NE(-1, cap_clear(cap_p));
+
+ /*
+ * Drops everything, especially CAP_SETPCAP, CAP_DAC_OVERRIDE, and
+ * CAP_DAC_READ_SEARCH.
+ */
+ EXPECT_NE(-1, cap_set_proc(cap_p));
+ EXPECT_NE(-1, cap_free(cap_p));
+}
+
+static int test_secbits_set(const unsigned int secbits)
+{
+ int err;
+
+ err = prctl(PR_SET_SECUREBITS, secbits);
+ if (err)
+ return errno;
+ return 0;
+}
+
+FIXTURE(access)
+{
+ int memfd, pipefd;
+ int pipe_fds[2], socket_fds[2];
+};
+
+FIXTURE_VARIANT(access)
+{
+ const bool mount_exec;
+ const bool file_exec;
+};
+
+FIXTURE_VARIANT_ADD(access, mount_exec_file_exec){
+ .mount_exec = true,
+ .file_exec = true,
+};
+
+FIXTURE_VARIANT_ADD(access, mount_exec_file_noexec){
+ .mount_exec = true,
+ .file_exec = false,
+};
+
+FIXTURE_VARIANT_ADD(access, mount_noexec_file_exec){
+ .mount_exec = false,
+ .file_exec = true,
+};
+
+FIXTURE_VARIANT_ADD(access, mount_noexec_file_noexec){
+ .mount_exec = false,
+ .file_exec = false,
+};
+
+static const char binary_path[] = "./false";
+static const char workdir_path[] = "./test-mount";
+static const char reg_file_path[] = "./test-mount/regular_file";
+static const char dir_path[] = "./test-mount/directory";
+static const char block_dev_path[] = "./test-mount/block_device";
+static const char char_dev_path[] = "./test-mount/character_device";
+static const char fifo_path[] = "./test-mount/fifo";
+
+FIXTURE_SETUP(access)
+{
+ int procfd_path_size;
+ static const char path_template[] = "/proc/self/fd/%d";
+ char procfd_path[sizeof(path_template) + 10];
+
+ /* Makes sure we are not already restricted nor locked. */
+ EXPECT_EQ(0, test_secbits_set(0));
+
+ /*
+ * Cleans previous workspace if any error previously happened (don't
+ * check errors).
+ */
+ umount(workdir_path);
+ rmdir(workdir_path);
+
+ /* Creates a clean mount point. */
+ ASSERT_EQ(0, mkdir(workdir_path, 00700));
+ ASSERT_EQ(0, mount("test", workdir_path, "tmpfs",
+ MS_MGC_VAL | (variant->mount_exec ? 0 : MS_NOEXEC),
+ "mode=0700,size=9m"));
+
+ /* Creates a regular file. */
+ ASSERT_EQ(0, mknod(reg_file_path,
+ S_IFREG | (variant->file_exec ? 0700 : 0600), 0));
+ /* Creates a directory. */
+ ASSERT_EQ(0, mkdir(dir_path, variant->file_exec ? 0700 : 0600));
+ /* Creates a character device: /dev/null. */
+ ASSERT_EQ(0, mknod(char_dev_path, S_IFCHR | 0400, makedev(1, 3)));
+ /* Creates a block device: /dev/loop0 */
+ ASSERT_EQ(0, mknod(block_dev_path, S_IFBLK | 0400, makedev(7, 0)));
+ /* Creates a fifo. */
+ ASSERT_EQ(0, mknod(fifo_path, S_IFIFO | 0600, 0));
+
+ /* Creates a regular file without user mount point. */
+ self->memfd = memfd_create("test-exec-probe", MFD_CLOEXEC);
+ ASSERT_LE(0, self->memfd);
+ /* Sets mode, which must be ignored by the exec check. */
+ ASSERT_EQ(0, fchmod(self->memfd, variant->file_exec ? 0700 : 0600));
+
+ /* Creates a pipefs file descriptor. */
+ ASSERT_EQ(0, pipe(self->pipe_fds));
+ procfd_path_size = snprintf(procfd_path, sizeof(procfd_path),
+ path_template, self->pipe_fds[0]);
+ ASSERT_LT(procfd_path_size, sizeof(procfd_path));
+ self->pipefd = open(procfd_path, O_RDWR | O_CLOEXEC);
+ ASSERT_LE(0, self->pipefd);
+ ASSERT_EQ(0, fchmod(self->pipefd, variant->file_exec ? 0700 : 0600));
+
+ /* Creates a socket file descriptor. */
+ ASSERT_EQ(0, socketpair(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC, 0,
+ self->socket_fds));
+}
+
+FIXTURE_TEARDOWN_PARENT(access)
+{
+ /* There is no need to unlink the test files. */
+ EXPECT_EQ(0, umount(workdir_path));
+ EXPECT_EQ(0, rmdir(workdir_path));
+}
+
+static void fill_exec_fd(struct __test_metadata *_metadata, const int fd_out)
+{
+ char buf[1024];
+ size_t len;
+ int fd_in;
+
+ fd_in = open(binary_path, O_CLOEXEC | O_RDONLY);
+ ASSERT_LE(0, fd_in);
+ /* Cannot use copy_file_range(2) because of EXDEV. */
+ len = read(fd_in, buf, sizeof(buf));
+ EXPECT_LE(0, len);
+ while (len > 0) {
+ EXPECT_EQ(len, write(fd_out, buf, len))
+ {
+ TH_LOG("Failed to write: %s (%d)", strerror(errno),
+ errno);
+ }
+ len = read(fd_in, buf, sizeof(buf));
+ EXPECT_LE(0, len);
+ }
+ EXPECT_EQ(0, close(fd_in));
+}
+
+static void fill_exec_path(struct __test_metadata *_metadata,
+ const char *const path)
+{
+ int fd_out;
+
+ fd_out = open(path, O_CLOEXEC | O_WRONLY);
+ ASSERT_LE(0, fd_out)
+ {
+ TH_LOG("Failed to open %s: %s", path, strerror(errno));
+ }
+ fill_exec_fd(_metadata, fd_out);
+ EXPECT_EQ(0, close(fd_out));
+}
+
+static void test_exec_fd(struct __test_metadata *_metadata, const int fd,
+ const int err_code)
+{
+ char *const argv[] = { "", NULL };
+ int access_ret, access_errno;
+
+ /*
+ * If we really execute fd, filled with the "false" binary, the current
+ * thread will exits with an error, which will be interpreted by the
+ * test framework as an error. With AT_EXECVE_CHECK, we only check a
+ * potential successful execution.
+ */
+ access_ret =
+ execveat(fd, "", argv, NULL, AT_EMPTY_PATH | AT_EXECVE_CHECK);
+ access_errno = errno;
+ if (err_code) {
+ EXPECT_EQ(-1, access_ret);
+ EXPECT_EQ(err_code, access_errno)
+ {
+ TH_LOG("Wrong error for execveat(2): %s (%d)",
+ strerror(access_errno), errno);
+ }
+ } else {
+ EXPECT_EQ(0, access_ret)
+ {
+ TH_LOG("Access denied: %s", strerror(access_errno));
+ }
+ }
+}
+
+static void test_exec_path(struct __test_metadata *_metadata,
+ const char *const path, const int err_code)
+{
+ int flags = O_CLOEXEC;
+ int fd;
+
+ /* Do not block on pipes. */
+ if (path == fifo_path)
+ flags |= O_NONBLOCK;
+
+ fd = open(path, flags | O_RDONLY);
+ ASSERT_LE(0, fd)
+ {
+ TH_LOG("Failed to open %s: %s", path, strerror(errno));
+ }
+ test_exec_fd(_metadata, fd, err_code);
+ EXPECT_EQ(0, close(fd));
+}
+
+/* Tests that we don't get ENOEXEC. */
+TEST_F(access, regular_file_empty)
+{
+ const int exec = variant->mount_exec && variant->file_exec;
+
+ test_exec_path(_metadata, reg_file_path, exec ? 0 : EACCES);
+
+ drop_privileges(_metadata);
+ test_exec_path(_metadata, reg_file_path, exec ? 0 : EACCES);
+}
+
+TEST_F(access, regular_file_elf)
+{
+ const int exec = variant->mount_exec && variant->file_exec;
+
+ fill_exec_path(_metadata, reg_file_path);
+
+ test_exec_path(_metadata, reg_file_path, exec ? 0 : EACCES);
+
+ drop_privileges(_metadata);
+ test_exec_path(_metadata, reg_file_path, exec ? 0 : EACCES);
+}
+
+/* Tests that we don't get ENOEXEC. */
+TEST_F(access, memfd_empty)
+{
+ const int exec = variant->file_exec;
+
+ test_exec_fd(_metadata, self->memfd, exec ? 0 : EACCES);
+
+ drop_privileges(_metadata);
+ test_exec_fd(_metadata, self->memfd, exec ? 0 : EACCES);
+}
+
+TEST_F(access, memfd_elf)
+{
+ const int exec = variant->file_exec;
+
+ fill_exec_fd(_metadata, self->memfd);
+
+ test_exec_fd(_metadata, self->memfd, exec ? 0 : EACCES);
+
+ drop_privileges(_metadata);
+ test_exec_fd(_metadata, self->memfd, exec ? 0 : EACCES);
+}
+
+TEST_F(access, non_regular_files)
+{
+ test_exec_path(_metadata, dir_path, EACCES);
+ test_exec_path(_metadata, block_dev_path, EACCES);
+ test_exec_path(_metadata, char_dev_path, EACCES);
+ test_exec_path(_metadata, fifo_path, EACCES);
+ test_exec_fd(_metadata, self->socket_fds[0], EACCES);
+ test_exec_fd(_metadata, self->pipefd, EACCES);
+}
+
+/* clang-format off */
+FIXTURE(secbits) {};
+/* clang-format on */
+
+FIXTURE_VARIANT(secbits)
+{
+ const bool is_privileged;
+ const int error;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(secbits, priv) {
+ /* clang-format on */
+ .is_privileged = true,
+ .error = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(secbits, unpriv) {
+ /* clang-format on */
+ .is_privileged = false,
+ .error = EPERM,
+};
+
+FIXTURE_SETUP(secbits)
+{
+ /* Makes sure no exec bits are set. */
+ EXPECT_EQ(0, test_secbits_set(0));
+ EXPECT_EQ(0, prctl(PR_GET_SECUREBITS));
+
+ if (!variant->is_privileged)
+ drop_privileges(_metadata);
+}
+
+FIXTURE_TEARDOWN(secbits)
+{
+}
+
+TEST_F(secbits, legacy)
+{
+ EXPECT_EQ(variant->error, test_secbits_set(0));
+}
+
+#define CHILD(...) \
+ do { \
+ pid_t child = vfork(); \
+ EXPECT_LE(0, child); \
+ if (child == 0) { \
+ __VA_ARGS__; \
+ _exit(0); \
+ } \
+ } while (0)
+
+TEST_F(secbits, exec)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_EXEC_RESTRICT_FILE;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS));
+ CHILD(EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS)));
+
+ secbits |= SECBIT_EXEC_DENY_INTERACTIVE;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS));
+ CHILD(EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS)));
+
+ secbits &= ~(SECBIT_EXEC_RESTRICT_FILE | SECBIT_EXEC_DENY_INTERACTIVE);
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS));
+ CHILD(EXPECT_EQ(secbits, prctl(PR_GET_SECUREBITS)));
+}
+
+TEST_F(secbits, check_locked_set)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_EXEC_RESTRICT_FILE;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ secbits |= SECBIT_EXEC_RESTRICT_FILE_LOCKED;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+
+ /* Checks lock set but unchanged. */
+ EXPECT_EQ(variant->error, test_secbits_set(secbits));
+ CHILD(EXPECT_EQ(variant->error, test_secbits_set(secbits)));
+
+ secbits &= ~SECBIT_EXEC_RESTRICT_FILE;
+ EXPECT_EQ(EPERM, test_secbits_set(0));
+ CHILD(EXPECT_EQ(EPERM, test_secbits_set(0)));
+}
+
+TEST_F(secbits, check_locked_unset)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_EXEC_RESTRICT_FILE_LOCKED;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+
+ /* Checks lock unset but unchanged. */
+ EXPECT_EQ(variant->error, test_secbits_set(secbits));
+ CHILD(EXPECT_EQ(variant->error, test_secbits_set(secbits)));
+
+ secbits &= ~SECBIT_EXEC_RESTRICT_FILE;
+ EXPECT_EQ(EPERM, test_secbits_set(0));
+ CHILD(EXPECT_EQ(EPERM, test_secbits_set(0)));
+}
+
+TEST_F(secbits, restrict_locked_set)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_EXEC_DENY_INTERACTIVE;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+ secbits |= SECBIT_EXEC_DENY_INTERACTIVE_LOCKED;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+
+ /* Checks lock set but unchanged. */
+ EXPECT_EQ(variant->error, test_secbits_set(secbits));
+ CHILD(EXPECT_EQ(variant->error, test_secbits_set(secbits)));
+
+ secbits &= ~SECBIT_EXEC_DENY_INTERACTIVE;
+ EXPECT_EQ(EPERM, test_secbits_set(0));
+ CHILD(EXPECT_EQ(EPERM, test_secbits_set(0)));
+}
+
+TEST_F(secbits, restrict_locked_unset)
+{
+ unsigned int secbits = prctl(PR_GET_SECUREBITS);
+
+ secbits |= SECBIT_EXEC_DENY_INTERACTIVE_LOCKED;
+ EXPECT_EQ(0, test_secbits_set(secbits));
+
+ /* Checks lock unset but unchanged. */
+ EXPECT_EQ(variant->error, test_secbits_set(secbits));
+ CHILD(EXPECT_EQ(variant->error, test_secbits_set(secbits)));
+
+ secbits &= ~SECBIT_EXEC_DENY_INTERACTIVE;
+ EXPECT_EQ(EPERM, test_secbits_set(0));
+ CHILD(EXPECT_EQ(EPERM, test_secbits_set(0)));
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/exec/config b/tools/testing/selftests/exec/config
new file mode 100644
index 000000000000..c308079867b3
--- /dev/null
+++ b/tools/testing/selftests/exec/config
@@ -0,0 +1,2 @@
+CONFIG_BLK_DEV=y
+CONFIG_BLK_DEV_LOOP=y
diff --git a/tools/testing/selftests/exec/false.c b/tools/testing/selftests/exec/false.c
new file mode 100644
index 000000000000..104383ec3a79
--- /dev/null
+++ b/tools/testing/selftests/exec/false.c
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+int main(void)
+{
+ return 1;
+}
--
2.47.0
^ permalink raw reply related
* [PATCH v21 5/6] samples/check-exec: Add set-exec
From: Mickaël Salaün @ 2024-11-12 19:18 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, Adhemerval Zanella Netto,
Alejandro Colomar, Aleksa Sarai, Andrew Morton, Andy Lutomirski,
Arnd Bergmann, Casey Schaufler, Christian Heimes, Dmitry Vyukov,
Elliott Hughes, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Linus Torvalds, Luca Boccassi, Luis Chamberlain,
Madhavan T . Venkataraman, Matt Bobrowski, Matthew Garrett,
Matthew Wilcox, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Scott Shell, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Theodore Ts'o, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20241112191858.162021-1-mic@digikod.net>
Add a simple tool to set SECBIT_EXEC_RESTRICT_FILE or
SECBIT_EXEC_DENY_INTERACTIVE before executing a command. This is useful
to easily test against enlighten script interpreters.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241112191858.162021-6-mic@digikod.net
---
Changes since v19:
* Rename file and directory.
* Update securebits and related arguments.
* Remove useless call to prctl() when securebits are unchanged.
---
samples/Kconfig | 7 +++
samples/Makefile | 1 +
samples/check-exec/.gitignore | 1 +
samples/check-exec/Makefile | 14 ++++++
samples/check-exec/set-exec.c | 85 +++++++++++++++++++++++++++++++++++
5 files changed, 108 insertions(+)
create mode 100644 samples/check-exec/.gitignore
create mode 100644 samples/check-exec/Makefile
create mode 100644 samples/check-exec/set-exec.c
diff --git a/samples/Kconfig b/samples/Kconfig
index b288d9991d27..efa28ceadc42 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -291,6 +291,13 @@ config SAMPLE_CGROUP
help
Build samples that demonstrate the usage of the cgroup API.
+config SAMPLE_CHECK_EXEC
+ bool "Exec secure bits examples"
+ depends on CC_CAN_LINK && HEADERS_INSTALL
+ help
+ Build a tool to easily configure SECBIT_EXEC_RESTRICT_FILE and
+ SECBIT_EXEC_DENY_INTERACTIVE.
+
source "samples/rust/Kconfig"
endif # SAMPLES
diff --git a/samples/Makefile b/samples/Makefile
index b85fa64390c5..f988202f3a30 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -3,6 +3,7 @@
subdir-$(CONFIG_SAMPLE_AUXDISPLAY) += auxdisplay
subdir-$(CONFIG_SAMPLE_ANDROID_BINDERFS) += binderfs
+subdir-$(CONFIG_SAMPLE_CHECK_EXEC) += check-exec
subdir-$(CONFIG_SAMPLE_CGROUP) += cgroup
obj-$(CONFIG_SAMPLE_CONFIGFS) += configfs/
obj-$(CONFIG_SAMPLE_CONNECTOR) += connector/
diff --git a/samples/check-exec/.gitignore b/samples/check-exec/.gitignore
new file mode 100644
index 000000000000..3f8119112ccf
--- /dev/null
+++ b/samples/check-exec/.gitignore
@@ -0,0 +1 @@
+/set-exec
diff --git a/samples/check-exec/Makefile b/samples/check-exec/Makefile
new file mode 100644
index 000000000000..d9f976e3ff98
--- /dev/null
+++ b/samples/check-exec/Makefile
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: BSD-3-Clause
+
+userprogs-always-y := \
+ set-exec
+
+userccflags += -I usr/include
+
+.PHONY: all clean
+
+all:
+ $(MAKE) -C ../.. samples/check-exec/
+
+clean:
+ $(MAKE) -C ../.. M=samples/check-exec/ clean
diff --git a/samples/check-exec/set-exec.c b/samples/check-exec/set-exec.c
new file mode 100644
index 000000000000..ba86a60a20dd
--- /dev/null
+++ b/samples/check-exec/set-exec.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Simple tool to set SECBIT_EXEC_RESTRICT_FILE, SECBIT_EXEC_DENY_INTERACTIVE,
+ * before executing a command.
+ *
+ * Copyright © 2024 Microsoft Corporation
+ */
+
+#define _GNU_SOURCE
+#define __SANE_USERSPACE_TYPES__
+#include <errno.h>
+#include <linux/prctl.h>
+#include <linux/securebits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/prctl.h>
+#include <unistd.h>
+
+static void print_usage(const char *argv0)
+{
+ fprintf(stderr, "usage: %s -f|-i -- <cmd> [args]...\n\n", argv0);
+ fprintf(stderr, "Execute a command with\n");
+ fprintf(stderr, "- SECBIT_EXEC_RESTRICT_FILE set: -f\n");
+ fprintf(stderr, "- SECBIT_EXEC_DENY_INTERACTIVE set: -i\n");
+}
+
+int main(const int argc, char *const argv[], char *const *const envp)
+{
+ const char *cmd_path;
+ char *const *cmd_argv;
+ int opt, secbits_cur, secbits_new;
+ bool has_policy = false;
+
+ secbits_cur = prctl(PR_GET_SECUREBITS);
+ if (secbits_cur == -1) {
+ /*
+ * This should never happen, except with a buggy seccomp
+ * filter.
+ */
+ perror("ERROR: Failed to get securebits");
+ return 1;
+ }
+
+ secbits_new = secbits_cur;
+ while ((opt = getopt(argc, argv, "fi")) != -1) {
+ switch (opt) {
+ case 'f':
+ secbits_new |= SECBIT_EXEC_RESTRICT_FILE |
+ SECBIT_EXEC_RESTRICT_FILE_LOCKED;
+ has_policy = true;
+ break;
+ case 'i':
+ secbits_new |= SECBIT_EXEC_DENY_INTERACTIVE |
+ SECBIT_EXEC_DENY_INTERACTIVE_LOCKED;
+ has_policy = true;
+ break;
+ default:
+ print_usage(argv[0]);
+ return 1;
+ }
+ }
+
+ if (!argv[optind] || !has_policy) {
+ print_usage(argv[0]);
+ return 1;
+ }
+
+ if (secbits_cur != secbits_new &&
+ prctl(PR_SET_SECUREBITS, secbits_new)) {
+ perror("Failed to set secure bit(s).");
+ fprintf(stderr,
+ "Hint: The running kernel may not support this feature.\n");
+ return 1;
+ }
+
+ cmd_path = argv[optind];
+ cmd_argv = argv + optind;
+ fprintf(stderr, "Executing command...\n");
+ execvpe(cmd_path, cmd_argv, envp);
+ fprintf(stderr, "Failed to execute \"%s\": %s\n", cmd_path,
+ strerror(errno));
+ return 1;
+}
--
2.47.0
^ permalink raw reply related
* [PATCH v21 4/6] selftests/landlock: Add tests for execveat + AT_EXECVE_CHECK
From: Mickaël Salaün @ 2024-11-12 19:18 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, Adhemerval Zanella Netto,
Alejandro Colomar, Aleksa Sarai, Andrew Morton, Andy Lutomirski,
Arnd Bergmann, Casey Schaufler, Christian Heimes, Dmitry Vyukov,
Elliott Hughes, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Linus Torvalds, Luca Boccassi, Luis Chamberlain,
Madhavan T . Venkataraman, Matt Bobrowski, Matthew Garrett,
Matthew Wilcox, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Scott Shell, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Theodore Ts'o, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module, Günther Noack
In-Reply-To: <20241112191858.162021-1-mic@digikod.net>
Extend layout1.execute with the new AT_EXECVE_CHECK flag. The semantic
with AT_EXECVE_CHECK is the same as with a simple execve(2),
LANDLOCK_ACCESS_FS_EXECUTE is enforced the same way.
Cc: Günther Noack <gnoack@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241112191858.162021-5-mic@digikod.net
---
Changes since v20:
* Rename AT_CHECK to AT_EXECVE_CHECK.
---
tools/testing/selftests/landlock/fs_test.c | 27 ++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 6788762188fe..cd66901be612 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -37,6 +37,10 @@
#include <linux/fs.h>
#include <linux/mount.h>
+/* Defines AT_EXECVE_CHECK without type conflicts. */
+#define _ASM_GENERIC_FCNTL_H
+#include <linux/fcntl.h>
+
#include "common.h"
#ifndef renameat2
@@ -2008,6 +2012,22 @@ static void test_execute(struct __test_metadata *const _metadata, const int err,
};
}
+static void test_check_exec(struct __test_metadata *const _metadata,
+ const int err, const char *const path)
+{
+ int ret;
+ char *const argv[] = { (char *)path, NULL };
+
+ ret = execveat(AT_FDCWD, path, argv, NULL,
+ AT_EMPTY_PATH | AT_EXECVE_CHECK);
+ if (err) {
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(errno, err);
+ } else {
+ EXPECT_EQ(0, ret);
+ }
+}
+
TEST_F_FORK(layout1, execute)
{
const struct rule rules[] = {
@@ -2025,20 +2045,27 @@ TEST_F_FORK(layout1, execute)
copy_binary(_metadata, file1_s1d2);
copy_binary(_metadata, file1_s1d3);
+ /* Checks before file1_s1d1 being denied. */
+ test_execute(_metadata, 0, file1_s1d1);
+ test_check_exec(_metadata, 0, file1_s1d1);
+
enforce_ruleset(_metadata, ruleset_fd);
ASSERT_EQ(0, close(ruleset_fd));
ASSERT_EQ(0, test_open(dir_s1d1, O_RDONLY));
ASSERT_EQ(0, test_open(file1_s1d1, O_RDONLY));
test_execute(_metadata, EACCES, file1_s1d1);
+ test_check_exec(_metadata, EACCES, file1_s1d1);
ASSERT_EQ(0, test_open(dir_s1d2, O_RDONLY));
ASSERT_EQ(0, test_open(file1_s1d2, O_RDONLY));
test_execute(_metadata, 0, file1_s1d2);
+ test_check_exec(_metadata, 0, file1_s1d2);
ASSERT_EQ(0, test_open(dir_s1d3, O_RDONLY));
ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY));
test_execute(_metadata, 0, file1_s1d3);
+ test_check_exec(_metadata, 0, file1_s1d3);
}
TEST_F_FORK(layout1, link)
--
2.47.0
^ permalink raw reply related
* [PATCH v21 6/6] samples/check-exec: Add an enlighten "inc" interpreter and 28 tests
From: Mickaël Salaün @ 2024-11-12 19:18 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, Adhemerval Zanella Netto,
Alejandro Colomar, Aleksa Sarai, Andrew Morton, Andy Lutomirski,
Arnd Bergmann, Casey Schaufler, Christian Heimes, Dmitry Vyukov,
Elliott Hughes, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Linus Torvalds, Luca Boccassi, Luis Chamberlain,
Madhavan T . Venkataraman, Matt Bobrowski, Matthew Garrett,
Matthew Wilcox, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Scott Shell, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Theodore Ts'o, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20241112191858.162021-1-mic@digikod.net>
Add a very simple script interpreter called "inc" that can evaluate two
different commands (one per line):
- "?" to initialize a counter from user's input;
- "+" to increment the counter (which is set to 0 by default).
It is enlighten to only interpret executable files according to
AT_EXECVE_CHECK and the related securebits:
# Executing a script with RESTRICT_FILE is only allowed if the script
# is executable:
./set-exec -f -- ./inc script-exec.inc # Allowed
./set-exec -f -- ./inc script-noexec.inc # Denied
# Executing stdin with DENY_INTERACTIVE is only allowed if stdin is an
# executable regular file:
./set-exec -i -- ./inc -i < script-exec.inc # Allowed
./set-exec -i -- ./inc -i < script-noexec.inc # Denied
# However, a pipe is not executable and it is then denied:
cat script-noexec.inc | ./set-exec -i -- ./inc -i # Denied
# Executing raw data (e.g. command argument) with DENY_INTERACTIVE is
# always denied.
./set-exec -i -- ./inc -c "+" # Denied
./inc -c "$(<script-ask.inc)" # Allowed
# To directly execute a script, we can update $PATH (used by `env`):
PATH="${PATH}:." ./script-exec.inc
# To execute several commands passed as argument:
Add a complete test suite to check the script interpreter against all
possible execution cases:
make TARGETS=exec kselftest-install
./tools/testing/selftests/kselftest_install/run_kselftest.sh
Fix ktap_helpers.sh to gracefully ignore optional argument.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241112191858.162021-7-mic@digikod.net
---
Changes since v20:
* Rename AT_CHECK to AT_EXECVE_CHECK.
Changes since v19:
* New patch.
---
samples/Kconfig | 4 +-
samples/check-exec/.gitignore | 1 +
samples/check-exec/Makefile | 1 +
samples/check-exec/inc.c | 205 ++++++++++++++++++
samples/check-exec/run-script-ask.inc | 8 +
samples/check-exec/script-ask.inc | 4 +
samples/check-exec/script-exec.inc | 3 +
samples/check-exec/script-noexec.inc | 3 +
tools/testing/selftests/exec/.gitignore | 2 +
tools/testing/selftests/exec/Makefile | 14 +-
.../selftests/exec/check-exec-tests.sh | 205 ++++++++++++++++++
.../selftests/kselftest/ktap_helpers.sh | 2 +-
12 files changed, 448 insertions(+), 4 deletions(-)
create mode 100644 samples/check-exec/inc.c
create mode 100755 samples/check-exec/run-script-ask.inc
create mode 100755 samples/check-exec/script-ask.inc
create mode 100755 samples/check-exec/script-exec.inc
create mode 100644 samples/check-exec/script-noexec.inc
create mode 100755 tools/testing/selftests/exec/check-exec-tests.sh
diff --git a/samples/Kconfig b/samples/Kconfig
index efa28ceadc42..84a9d4e8d947 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -296,7 +296,9 @@ config SAMPLE_CHECK_EXEC
depends on CC_CAN_LINK && HEADERS_INSTALL
help
Build a tool to easily configure SECBIT_EXEC_RESTRICT_FILE and
- SECBIT_EXEC_DENY_INTERACTIVE.
+ SECBIT_EXEC_DENY_INTERACTIVE, and a simple script interpreter to
+ demonstrate how they should be used with execveat(2) +
+ AT_EXECVE_CHECK.
source "samples/rust/Kconfig"
diff --git a/samples/check-exec/.gitignore b/samples/check-exec/.gitignore
index 3f8119112ccf..cd759a19dacd 100644
--- a/samples/check-exec/.gitignore
+++ b/samples/check-exec/.gitignore
@@ -1 +1,2 @@
+/inc
/set-exec
diff --git a/samples/check-exec/Makefile b/samples/check-exec/Makefile
index d9f976e3ff98..c4f08ad0f8e3 100644
--- a/samples/check-exec/Makefile
+++ b/samples/check-exec/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: BSD-3-Clause
userprogs-always-y := \
+ inc \
set-exec
userccflags += -I usr/include
diff --git a/samples/check-exec/inc.c b/samples/check-exec/inc.c
new file mode 100644
index 000000000000..94b87569d2a2
--- /dev/null
+++ b/samples/check-exec/inc.c
@@ -0,0 +1,205 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Very simple script interpreter that can evaluate two different commands (one
+ * per line):
+ * - "?" to initialize a counter from user's input;
+ * - "+" to increment the counter (which is set to 0 by default).
+ *
+ * See tools/testing/selftests/exec/check-exec-tests.sh and
+ * Documentation/userspace-api/check_exec.rst
+ *
+ * Copyright © 2024 Microsoft Corporation
+ */
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <linux/fcntl.h>
+#include <linux/prctl.h>
+#include <linux/securebits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/prctl.h>
+#include <unistd.h>
+
+/* Returns 1 on error, 0 otherwise. */
+static int interpret_buffer(char *buffer, size_t buffer_size)
+{
+ char *line, *saveptr = NULL;
+ long long number = 0;
+
+ /* Each command is the first character of a line. */
+ saveptr = NULL;
+ line = strtok_r(buffer, "\n", &saveptr);
+ while (line) {
+ if (*line != '#' && strlen(line) != 1) {
+ fprintf(stderr, "# ERROR: Unknown string\n");
+ return 1;
+ }
+ switch (*line) {
+ case '#':
+ /* Skips shebang and comments. */
+ break;
+ case '+':
+ /* Increments and prints the number. */
+ number++;
+ printf("%lld\n", number);
+ break;
+ case '?':
+ /* Reads integer from stdin. */
+ fprintf(stderr, "> Enter new number: \n");
+ if (scanf("%lld", &number) != 1) {
+ fprintf(stderr,
+ "# WARNING: Failed to read number from stdin\n");
+ }
+ break;
+ default:
+ fprintf(stderr, "# ERROR: Unknown character '%c'\n",
+ *line);
+ return 1;
+ }
+ line = strtok_r(NULL, "\n", &saveptr);
+ }
+ return 0;
+}
+
+/* Returns 1 on error, 0 otherwise. */
+static int interpret_stream(FILE *script, char *const script_name,
+ char *const *const envp, const bool restrict_stream)
+{
+ int err;
+ char *const script_argv[] = { script_name, NULL };
+ char buf[128] = {};
+ size_t buf_size = sizeof(buf);
+
+ /*
+ * We pass a valid argv and envp to the kernel to emulate a native
+ * script execution. We must use the script file descriptor instead of
+ * the script path name to avoid race conditions.
+ */
+ err = execveat(fileno(script), "", script_argv, envp,
+ AT_EMPTY_PATH | AT_EXECVE_CHECK);
+ if (err && restrict_stream) {
+ perror("ERROR: Script execution check");
+ return 1;
+ }
+
+ /* Reads script. */
+ buf_size = fread(buf, 1, buf_size - 1, script);
+ return interpret_buffer(buf, buf_size);
+}
+
+static void print_usage(const char *argv0)
+{
+ fprintf(stderr, "usage: %s <script.inc> | -i | -c <command>\n\n",
+ argv0);
+ fprintf(stderr, "Example:\n");
+ fprintf(stderr, " ./set-exec -fi -- ./inc -i < script-exec.inc\n");
+}
+
+int main(const int argc, char *const argv[], char *const *const envp)
+{
+ int opt;
+ char *cmd = NULL;
+ char *script_name = NULL;
+ bool interpret_stdin = false;
+ FILE *script_file = NULL;
+ int secbits;
+ bool deny_interactive, restrict_file;
+ size_t arg_nb;
+
+ secbits = prctl(PR_GET_SECUREBITS);
+ if (secbits == -1) {
+ /*
+ * This should never happen, except with a buggy seccomp
+ * filter.
+ */
+ perror("ERROR: Failed to get securebits");
+ return 1;
+ }
+
+ deny_interactive = !!(secbits & SECBIT_EXEC_DENY_INTERACTIVE);
+ restrict_file = !!(secbits & SECBIT_EXEC_RESTRICT_FILE);
+
+ while ((opt = getopt(argc, argv, "c:i")) != -1) {
+ switch (opt) {
+ case 'c':
+ if (cmd) {
+ fprintf(stderr, "ERROR: Command already set");
+ return 1;
+ }
+ cmd = optarg;
+ break;
+ case 'i':
+ interpret_stdin = true;
+ break;
+ default:
+ print_usage(argv[0]);
+ return 1;
+ }
+ }
+
+ /* Checks that only one argument is used, or read stdin. */
+ arg_nb = !!cmd + !!interpret_stdin;
+ if (arg_nb == 0 && argc == 2) {
+ script_name = argv[1];
+ } else if (arg_nb != 1) {
+ print_usage(argv[0]);
+ return 1;
+ }
+
+ if (cmd) {
+ /*
+ * Other kind of interactive interpretations should be denied
+ * as well (e.g. CLI arguments passing script snippets,
+ * environment variables interpreted as script). However, any
+ * way to pass script files should only be restricted according
+ * to restrict_file.
+ */
+ if (deny_interactive) {
+ fprintf(stderr,
+ "ERROR: Interactive interpretation denied.\n");
+ return 1;
+ }
+
+ return interpret_buffer(cmd, strlen(cmd));
+ }
+
+ if (interpret_stdin && !script_name) {
+ script_file = stdin;
+ /*
+ * As for any execve(2) call, this path may be logged by the
+ * kernel.
+ */
+ script_name = "/proc/self/fd/0";
+ /*
+ * When stdin is used, it can point to a regular file or a
+ * pipe. Restrict stdin execution according to
+ * SECBIT_EXEC_DENY_INTERACTIVE but always allow executable
+ * files (which are not considered as interactive inputs).
+ */
+ return interpret_stream(script_file, script_name, envp,
+ deny_interactive);
+ } else if (script_name && !interpret_stdin) {
+ /*
+ * In this sample, we don't pass any argument to scripts, but
+ * otherwise we would have to forge an argv with such
+ * arguments.
+ */
+ script_file = fopen(script_name, "r");
+ if (!script_file) {
+ perror("ERROR: Failed to open script");
+ return 1;
+ }
+ /*
+ * Restricts file execution according to
+ * SECBIT_EXEC_RESTRICT_FILE.
+ */
+ return interpret_stream(script_file, script_name, envp,
+ restrict_file);
+ }
+
+ print_usage(argv[0]);
+ return 1;
+}
diff --git a/samples/check-exec/run-script-ask.inc b/samples/check-exec/run-script-ask.inc
new file mode 100755
index 000000000000..3ea3e15fbd5a
--- /dev/null
+++ b/samples/check-exec/run-script-ask.inc
@@ -0,0 +1,8 @@
+#!/usr/bin/env sh
+
+DIR="$(dirname -- "$0")"
+
+PATH="${PATH}:${DIR}"
+
+set -x
+"${DIR}/script-ask.inc"
diff --git a/samples/check-exec/script-ask.inc b/samples/check-exec/script-ask.inc
new file mode 100755
index 000000000000..f48252ab07c1
--- /dev/null
+++ b/samples/check-exec/script-ask.inc
@@ -0,0 +1,4 @@
+#!/usr/bin/env inc
+
+?
++
diff --git a/samples/check-exec/script-exec.inc b/samples/check-exec/script-exec.inc
new file mode 100755
index 000000000000..525e958e1c20
--- /dev/null
+++ b/samples/check-exec/script-exec.inc
@@ -0,0 +1,3 @@
+#!/usr/bin/env inc
+
++
diff --git a/samples/check-exec/script-noexec.inc b/samples/check-exec/script-noexec.inc
new file mode 100644
index 000000000000..525e958e1c20
--- /dev/null
+++ b/samples/check-exec/script-noexec.inc
@@ -0,0 +1,3 @@
+#!/usr/bin/env inc
+
++
diff --git a/tools/testing/selftests/exec/.gitignore b/tools/testing/selftests/exec/.gitignore
index a32c63bb4df1..7f3d1ae762ec 100644
--- a/tools/testing/selftests/exec/.gitignore
+++ b/tools/testing/selftests/exec/.gitignore
@@ -11,9 +11,11 @@ non-regular
null-argv
/check-exec
/false
+/inc
/load_address.*
!load_address.c
/recursion-depth
+/set-exec
xxxxxxxx*
pipe
S_I*.test
diff --git a/tools/testing/selftests/exec/Makefile b/tools/testing/selftests/exec/Makefile
index 8713d1c862ae..45a3cfc435cf 100644
--- a/tools/testing/selftests/exec/Makefile
+++ b/tools/testing/selftests/exec/Makefile
@@ -10,9 +10,9 @@ ALIGN_PIES := $(patsubst %,load_address.%,$(ALIGNS))
ALIGN_STATIC_PIES := $(patsubst %,load_address.static.%,$(ALIGNS))
ALIGNMENT_TESTS := $(ALIGN_PIES) $(ALIGN_STATIC_PIES)
-TEST_PROGS := binfmt_script.py
+TEST_PROGS := binfmt_script.py check-exec-tests.sh
TEST_GEN_PROGS := execveat non-regular $(ALIGNMENT_TESTS)
-TEST_GEN_PROGS_EXTENDED := false
+TEST_GEN_PROGS_EXTENDED := false inc set-exec script-exec.inc script-noexec.inc
TEST_GEN_FILES := execveat.symlink execveat.denatured script subdir
# Makefile is a run-time dependency, since it's accessed by the execveat test
TEST_FILES := Makefile
@@ -26,6 +26,8 @@ EXTRA_CLEAN := $(OUTPUT)/subdir.moved $(OUTPUT)/execveat.moved $(OUTPUT)/xxxxx*
include ../lib.mk
+CHECK_EXEC_SAMPLES := $(top_srcdir)/samples/check-exec
+
$(OUTPUT)/subdir:
mkdir -p $@
$(OUTPUT)/script: Makefile
@@ -45,3 +47,11 @@ $(OUTPUT)/load_address.static.0x%: load_address.c
-fPIE -static-pie $< -o $@
$(OUTPUT)/false: false.c
$(CC) $(CFLAGS) $(LDFLAGS) -static $< -o $@
+$(OUTPUT)/inc: $(CHECK_EXEC_SAMPLES)/inc.c
+ $(CC) $(CFLAGS) $(LDFLAGS) $< -o $@
+$(OUTPUT)/set-exec: $(CHECK_EXEC_SAMPLES)/set-exec.c
+ $(CC) $(CFLAGS) $(LDFLAGS) $< -o $@
+$(OUTPUT)/script-exec.inc: $(CHECK_EXEC_SAMPLES)/script-exec.inc
+ cp $< $@
+$(OUTPUT)/script-noexec.inc: $(CHECK_EXEC_SAMPLES)/script-noexec.inc
+ cp $< $@
diff --git a/tools/testing/selftests/exec/check-exec-tests.sh b/tools/testing/selftests/exec/check-exec-tests.sh
new file mode 100755
index 000000000000..87102906ae3c
--- /dev/null
+++ b/tools/testing/selftests/exec/check-exec-tests.sh
@@ -0,0 +1,205 @@
+#!/usr/bin/env bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Test the "inc" interpreter.
+#
+# See include/uapi/linux/securebits.h, include/uapi/linux/fcntl.h and
+# samples/check-exec/inc.c
+#
+# Copyright © 2024 Microsoft Corporation
+
+set -u -e -o pipefail
+
+EXPECTED_OUTPUT="1"
+exec 2>/dev/null
+
+DIR="$(dirname $(readlink -f "$0"))"
+source "${DIR}"/../kselftest/ktap_helpers.sh
+
+exec_direct() {
+ local expect="$1"
+ local script="$2"
+ shift 2
+ local ret=0
+ local out
+
+ # Updates PATH for `env` to execute the `inc` interpreter.
+ out="$(PATH="." "$@" "${script}")" || ret=$?
+
+ if [[ ${ret} -ne ${expect} ]]; then
+ echo "ERROR: Wrong expectation for direct file execution: ${ret}"
+ return 1
+ fi
+ if [[ ${ret} -eq 0 && "${out}" != "${EXPECTED_OUTPUT}" ]]; then
+ echo "ERROR: Wrong output for direct file execution: ${out}"
+ return 1
+ fi
+}
+
+exec_indirect() {
+ local expect="$1"
+ local script="$2"
+ shift 2
+ local ret=0
+ local out
+
+ # Script passed as argument.
+ out="$("$@" ./inc "${script}")" || ret=$?
+
+ if [[ ${ret} -ne ${expect} ]]; then
+ echo "ERROR: Wrong expectation for indirect file execution: ${ret}"
+ return 1
+ fi
+ if [[ ${ret} -eq 0 && "${out}" != "${EXPECTED_OUTPUT}" ]]; then
+ echo "ERROR: Wrong output for indirect file execution: ${out}"
+ return 1
+ fi
+}
+
+exec_stdin_reg() {
+ local expect="$1"
+ local script="$2"
+ shift 2
+ local ret=0
+ local out
+
+ # Executing stdin must be allowed if the related file is executable.
+ out="$("$@" ./inc -i < "${script}")" || ret=$?
+
+ if [[ ${ret} -ne ${expect} ]]; then
+ echo "ERROR: Wrong expectation for stdin regular file execution: ${ret}"
+ return 1
+ fi
+ if [[ ${ret} -eq 0 && "${out}" != "${EXPECTED_OUTPUT}" ]]; then
+ echo "ERROR: Wrong output for stdin regular file execution: ${out}"
+ return 1
+ fi
+}
+
+exec_stdin_pipe() {
+ local expect="$1"
+ shift
+ local ret=0
+ local out
+
+ # A pipe is not executable.
+ out="$(cat script-exec.inc | "$@" ./inc -i)" || ret=$?
+
+ if [[ ${ret} -ne ${expect} ]]; then
+ echo "ERROR: Wrong expectation for stdin pipe execution: ${ret}"
+ return 1
+ fi
+}
+
+exec_argument() {
+ local expect="$1"
+ local ret=0
+ shift
+ local out
+
+ # Script not coming from a file must not be executed.
+ out="$("$@" ./inc -c "$(< script-exec.inc)")" || ret=$?
+
+ if [[ ${ret} -ne ${expect} ]]; then
+ echo "ERROR: Wrong expectation for arbitrary argument execution: ${ret}"
+ return 1
+ fi
+ if [[ ${ret} -eq 0 && "${out}" != "${EXPECTED_OUTPUT}" ]]; then
+ echo "ERROR: Wrong output for arbitrary argument execution: ${out}"
+ return 1
+ fi
+}
+
+exec_interactive() {
+ exec_stdin_pipe "$@"
+ exec_argument "$@"
+}
+
+ktap_test() {
+ ktap_test_result "$*" "$@"
+}
+
+ktap_print_header
+ktap_set_plan 28
+
+# Without secbit configuration, nothing is changed.
+
+ktap_print_msg "By default, executable scripts are allowed to be interpreted and executed."
+ktap_test exec_direct 0 script-exec.inc
+ktap_test exec_indirect 0 script-exec.inc
+
+ktap_print_msg "By default, executable stdin is allowed to be interpreted."
+ktap_test exec_stdin_reg 0 script-exec.inc
+
+ktap_print_msg "By default, non-executable scripts are allowed to be interpreted, but not directly executed."
+# We get 126 because of direct execution by Bash.
+ktap_test exec_direct 126 script-noexec.inc
+ktap_test exec_indirect 0 script-noexec.inc
+
+ktap_print_msg "By default, non-executable stdin is allowed to be interpreted."
+ktap_test exec_stdin_reg 0 script-noexec.inc
+
+ktap_print_msg "By default, interactive commands are allowed to be interpreted."
+ktap_test exec_interactive 0
+
+# With only file restriction: protect non-malicious users from inadvertent errors (e.g. python ~/Downloads/*.py).
+
+ktap_print_msg "With -f, executable scripts are allowed to be interpreted and executed."
+ktap_test exec_direct 0 script-exec.inc ./set-exec -f --
+ktap_test exec_indirect 0 script-exec.inc ./set-exec -f --
+
+ktap_print_msg "With -f, executable stdin is allowed to be interpreted."
+ktap_test exec_stdin_reg 0 script-exec.inc ./set-exec -f --
+
+ktap_print_msg "With -f, non-executable scripts are not allowed to be executed nor interpreted."
+# Direct execution of non-executable script is alwayse denied by the kernel.
+ktap_test exec_direct 1 script-noexec.inc ./set-exec -f --
+ktap_test exec_indirect 1 script-noexec.inc ./set-exec -f --
+
+ktap_print_msg "With -f, non-executable stdin is allowed to be interpreted."
+ktap_test exec_stdin_reg 0 script-noexec.inc ./set-exec -f --
+
+ktap_print_msg "With -f, interactive commands are allowed to be interpreted."
+ktap_test exec_interactive 0 ./set-exec -f --
+
+# With only denied interactive commands: check or monitor script content (e.g. with LSM).
+
+ktap_print_msg "With -i, executable scripts are allowed to be interpreted and executed."
+ktap_test exec_direct 0 script-exec.inc ./set-exec -i --
+ktap_test exec_indirect 0 script-exec.inc ./set-exec -i --
+
+ktap_print_msg "With -i, executable stdin is allowed to be interpreted."
+ktap_test exec_stdin_reg 0 script-exec.inc ./set-exec -i --
+
+ktap_print_msg "With -i, non-executable scripts are allowed to be interpreted, but not directly executed."
+# Direct execution of non-executable script is alwayse denied by the kernel.
+ktap_test exec_direct 1 script-noexec.inc ./set-exec -i --
+ktap_test exec_indirect 0 script-noexec.inc ./set-exec -i --
+
+ktap_print_msg "With -i, non-executable stdin is not allowed to be interpreted."
+ktap_test exec_stdin_reg 1 script-noexec.inc ./set-exec -i --
+
+ktap_print_msg "With -i, interactive commands are not allowed to be interpreted."
+ktap_test exec_interactive 1 ./set-exec -i --
+
+# With both file restriction and denied interactive commands: only allow executable scripts.
+
+ktap_print_msg "With -fi, executable scripts are allowed to be interpreted and executed."
+ktap_test exec_direct 0 script-exec.inc ./set-exec -fi --
+ktap_test exec_indirect 0 script-exec.inc ./set-exec -fi --
+
+ktap_print_msg "With -fi, executable stdin is allowed to be interpreted."
+ktap_test exec_stdin_reg 0 script-exec.inc ./set-exec -fi --
+
+ktap_print_msg "With -fi, non-executable scripts are not allowed to be interpreted nor executed."
+# Direct execution of non-executable script is alwayse denied by the kernel.
+ktap_test exec_direct 1 script-noexec.inc ./set-exec -fi --
+ktap_test exec_indirect 1 script-noexec.inc ./set-exec -fi --
+
+ktap_print_msg "With -fi, non-executable stdin is not allowed to be interpreted."
+ktap_test exec_stdin_reg 1 script-noexec.inc ./set-exec -fi --
+
+ktap_print_msg "With -fi, interactive commands are not allowed to be interpreted."
+ktap_test exec_interactive 1 ./set-exec -fi --
+
+ktap_finished
diff --git a/tools/testing/selftests/kselftest/ktap_helpers.sh b/tools/testing/selftests/kselftest/ktap_helpers.sh
index 79a125eb24c2..14e7f3ec3f84 100644
--- a/tools/testing/selftests/kselftest/ktap_helpers.sh
+++ b/tools/testing/selftests/kselftest/ktap_helpers.sh
@@ -40,7 +40,7 @@ ktap_skip_all() {
__ktap_test() {
result="$1"
description="$2"
- directive="$3" # optional
+ directive="${3:-}" # optional
local directive_str=
[ ! -z "$directive" ] && directive_str="# $directive"
--
2.47.0
^ permalink raw reply related
* [PATCH v21 2/6] security: Add EXEC_RESTRICT_FILE and EXEC_DENY_INTERACTIVE securebits
From: Mickaël Salaün @ 2024-11-12 19:18 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, Adhemerval Zanella Netto,
Alejandro Colomar, Aleksa Sarai, Andrew Morton, Andy Lutomirski,
Arnd Bergmann, Casey Schaufler, Christian Heimes, Dmitry Vyukov,
Elliott Hughes, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Linus Torvalds, Luca Boccassi, Luis Chamberlain,
Madhavan T . Venkataraman, Matt Bobrowski, Matthew Garrett,
Matthew Wilcox, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Scott Shell, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Theodore Ts'o, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module, Andy Lutomirski
In-Reply-To: <20241112191858.162021-1-mic@digikod.net>
The new SECBIT_EXEC_RESTRICT_FILE, SECBIT_EXEC_DENY_INTERACTIVE, and
their *_LOCKED counterparts are designed to be set by processes setting
up an execution environment, such as a user session, a container, or a
security sandbox. Unlike other securebits, these ones can be set by
unprivileged processes. Like seccomp filters or Landlock domains, the
securebits are inherited across processes.
When SECBIT_EXEC_RESTRICT_FILE is set, programs interpreting code should
control executable resources according to execveat(2) + AT_EXECVE_CHECK
(see previous commit).
When SECBIT_EXEC_DENY_INTERACTIVE is set, a process should deny
execution of user interactive commands (which excludes executable
regular files).
Being able to configure each of these securebits enables system
administrators or owner of image containers to gradually validate the
related changes and to identify potential issues (e.g. with interpreter
or audit logs).
It should be noted that unlike other security bits, the
SECBIT_EXEC_RESTRICT_FILE and SECBIT_EXEC_DENY_INTERACTIVE bits are
dedicated to user space willing to restrict itself. Because of that,
they only make sense in the context of a trusted environment (e.g.
sandbox, container, user session, full system) where the process
changing its behavior (according to these bits) and all its parent
processes are trusted. Otherwise, any parent process could just execute
its own malicious code (interpreting a script or not), or even enforce a
seccomp filter to mask these bits.
Such a secure environment can be achieved with an appropriate access
control (e.g. mount's noexec option, file access rights, LSM policy) and
an enlighten ld.so checking that libraries are allowed for execution
e.g., to protect against illegitimate use of LD_PRELOAD.
Ptrace restrictions according to these securebits would not make sense
because of the processes' trust assumption.
Scripts may need some changes to deal with untrusted data (e.g. stdin,
environment variables), but that is outside the scope of the kernel.
See chromeOS's documentation about script execution control and the
related threat model:
https://www.chromium.org/chromium-os/developer-library/guides/security/noexec-shell-scripts/
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241112191858.162021-3-mic@digikod.net
---
Changes since v20:
* Move UAPI documentation to a dedicated RST file and format it.
Changes since v19:
* Replace SECBIT_SHOULD_EXEC_CHECK and SECBIT_SHOULD_EXEC_RESTRICT with
SECBIT_EXEC_RESTRICT_FILE and SECBIT_EXEC_DENY_INTERACTIVE:
https://lore.kernel.org/all/20240710.eiKohpa4Phai@digikod.net/
* Remove the ptrace restrictions, suggested by Andy.
* Improve documentation according to the discussion with Jeff.
New design since v18:
https://lore.kernel.org/r/20220104155024.48023-3-mic@digikod.net
---
Documentation/userspace-api/check_exec.rst | 97 ++++++++++++++++++++++
include/uapi/linux/securebits.h | 24 +++++-
security/commoncap.c | 29 +++++--
3 files changed, 143 insertions(+), 7 deletions(-)
diff --git a/Documentation/userspace-api/check_exec.rst b/Documentation/userspace-api/check_exec.rst
index ad1aeaa5f6c0..1df5c7534af9 100644
--- a/Documentation/userspace-api/check_exec.rst
+++ b/Documentation/userspace-api/check_exec.rst
@@ -2,6 +2,21 @@
Executability check
===================
+The ``AT_EXECVE_CHECK`` :manpage:`execveat(2)` flag, and the
+``SECBIT_EXEC_RESTRICT_FILE`` and ``SECBIT_EXEC_DENY_INTERACTIVE`` securebits
+are intended for script interpreters and dynamic linkers to enforce a
+consistent execution security policy handled by the kernel. See the
+`samples/check-exec/inc.c`_ example.
+
+Whether an interpreter should check these securebits or not depends on the
+security risk of running malicious scripts with respect to the execution
+environment, and whether the kernel can check if a script is trustworthy or
+not. For instance, Python scripts running on a server can use arbitrary
+syscalls and access arbitrary files. Such interpreters should then be
+enlighten to use these securebits and let users define their security policy.
+However, a JavaScript engine running in a web browser should already be
+sandboxed and then should not be able to harm the user's environment.
+
AT_EXECVE_CHECK
===============
@@ -32,3 +47,85 @@ be executable, which also requires integrity guarantees.
To avoid race conditions leading to time-of-check to time-of-use issues,
``AT_EXECVE_CHECK`` should be used with ``AT_EMPTY_PATH`` to check against a
file descriptor instead of a path.
+
+SECBIT_EXEC_RESTRICT_FILE and SECBIT_EXEC_DENY_INTERACTIVE
+==========================================================
+
+When ``SECBIT_EXEC_RESTRICT_FILE`` is set, a process should only interpret or
+execute a file if a call to :manpage:`execveat(2)` with the related file
+descriptor and the ``AT_EXECVE_CHECK`` flag succeed.
+
+This secure bit may be set by user session managers, service managers,
+container runtimes, sandboxer tools... Except for test environments, the
+related ``SECBIT_EXEC_RESTRICT_FILE_LOCKED`` bit should also be set.
+
+Programs should only enforce consistent restrictions according to the
+securebits but without relying on any other user-controlled configuration.
+Indeed, the use case for these securebits is to only trust executable code
+vetted by the system configuration (through the kernel), so we should be
+careful to not let untrusted users control this configuration.
+
+However, script interpreters may still use user configuration such as
+environment variables as long as it is not a way to disable the securebits
+checks. For instance, the ``PATH`` and ``LD_PRELOAD`` variables can be set by
+a script's caller. Changing these variables may lead to unintended code
+executions, but only from vetted executable programs, which is OK. For this to
+make sense, the system should provide a consistent security policy to avoid
+arbitrary code execution e.g., by enforcing a write xor execute policy.
+
+When ``SECBIT_EXEC_DENY_INTERACTIVE`` is set, a process should never interpret
+interactive user commands (e.g. scripts). However, if such commands are passed
+through a file descriptor (e.g. stdin), its content should be interpreted if a
+call to :manpage:`execveat(2)` with the related file descriptor and the
+``AT_EXECVE_CHECK`` flag succeed.
+
+For instance, script interpreters called with a script snippet as argument
+should always deny such execution if ``SECBIT_EXEC_DENY_INTERACTIVE`` is set.
+
+This secure bit may be set by user session managers, service managers,
+container runtimes, sandboxer tools... Except for test environments, the
+related ``SECBIT_EXEC_DENY_INTERACTIVE_LOCKED`` bit should also be set.
+
+Here is the expected behavior for a script interpreter according to combination
+of any exec securebits:
+
+1. ``SECBIT_EXEC_RESTRICT_FILE=0`` and ``SECBIT_EXEC_DENY_INTERACTIVE=0``
+
+ Always interpret scripts, and allow arbitrary user commands (default).
+
+ No threat, everyone and everything is trusted, but we can get ahead of
+ potential issues thanks to the call to :manpage:`execveat(2)` with
+ ``AT_EXECVE_CHECK`` which should always be performed but ignored by the
+ script interpreter. Indeed, this check is still important to enable systems
+ administrators to verify requests (e.g. with audit) and prepare for
+ migration to a secure mode.
+
+2. ``SECBIT_EXEC_RESTRICT_FILE=1`` and ``SECBIT_EXEC_DENY_INTERACTIVE=0``
+
+ Deny script interpretation if they are not executable, but allow
+ arbitrary user commands.
+
+ The threat is (potential) malicious scripts run by trusted (and not fooled)
+ users. That can protect against unintended script executions (e.g. ``sh
+ /tmp/*.sh``). This makes sense for (semi-restricted) user sessions.
+
+3. ``SECBIT_EXEC_RESTRICT_FILE=0`` and ``SECBIT_EXEC_DENY_INTERACTIVE=1``
+
+ Always interpret scripts, but deny arbitrary user commands.
+
+ This use case may be useful for secure services (i.e. without interactive
+ user session) where scripts' integrity is verified (e.g. with IMA/EVM or
+ dm-verity/IPE) but where access rights might not be ready yet. Indeed,
+ arbitrary interactive commands would be much more difficult to check.
+
+4. ``SECBIT_EXEC_RESTRICT_FILE=1`` and ``SECBIT_EXEC_DENY_INTERACTIVE=1``
+
+ Deny script interpretation if they are not executable, and also deny
+ any arbitrary user commands.
+
+ The threat is malicious scripts run by untrusted users (but trusted code).
+ This makes sense for system services that may only execute trusted scripts.
+
+.. Links
+.. _samples/check-exec/inc.c:
+ https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/samples/check-exec/inc.c
diff --git a/include/uapi/linux/securebits.h b/include/uapi/linux/securebits.h
index d6d98877ff1a..3fba30dbd68b 100644
--- a/include/uapi/linux/securebits.h
+++ b/include/uapi/linux/securebits.h
@@ -52,10 +52,32 @@
#define SECBIT_NO_CAP_AMBIENT_RAISE_LOCKED \
(issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE_LOCKED))
+/* See Documentation/userspace-api/check_exec.rst */
+#define SECURE_EXEC_RESTRICT_FILE 8
+#define SECURE_EXEC_RESTRICT_FILE_LOCKED 9 /* make bit-8 immutable */
+
+#define SECBIT_EXEC_RESTRICT_FILE (issecure_mask(SECURE_EXEC_RESTRICT_FILE))
+#define SECBIT_EXEC_RESTRICT_FILE_LOCKED \
+ (issecure_mask(SECURE_EXEC_RESTRICT_FILE_LOCKED))
+
+/* See Documentation/userspace-api/check_exec.rst */
+#define SECURE_EXEC_DENY_INTERACTIVE 10
+#define SECURE_EXEC_DENY_INTERACTIVE_LOCKED 11 /* make bit-10 immutable */
+
+#define SECBIT_EXEC_DENY_INTERACTIVE \
+ (issecure_mask(SECURE_EXEC_DENY_INTERACTIVE))
+#define SECBIT_EXEC_DENY_INTERACTIVE_LOCKED \
+ (issecure_mask(SECURE_EXEC_DENY_INTERACTIVE_LOCKED))
+
#define SECURE_ALL_BITS (issecure_mask(SECURE_NOROOT) | \
issecure_mask(SECURE_NO_SETUID_FIXUP) | \
issecure_mask(SECURE_KEEP_CAPS) | \
- issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE))
+ issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE) | \
+ issecure_mask(SECURE_EXEC_RESTRICT_FILE) | \
+ issecure_mask(SECURE_EXEC_DENY_INTERACTIVE))
#define SECURE_ALL_LOCKS (SECURE_ALL_BITS << 1)
+#define SECURE_ALL_UNPRIVILEGED (issecure_mask(SECURE_EXEC_RESTRICT_FILE) | \
+ issecure_mask(SECURE_EXEC_DENY_INTERACTIVE))
+
#endif /* _UAPI_LINUX_SECUREBITS_H */
diff --git a/security/commoncap.c b/security/commoncap.c
index cefad323a0b1..52ea01acb453 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1302,21 +1302,38 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
& (old->securebits ^ arg2)) /*[1]*/
|| ((old->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/
|| (arg2 & ~(SECURE_ALL_LOCKS | SECURE_ALL_BITS)) /*[3]*/
- || (cap_capable(current_cred(),
- current_cred()->user_ns,
- CAP_SETPCAP,
- CAP_OPT_NONE) != 0) /*[4]*/
/*
* [1] no changing of bits that are locked
* [2] no unlocking of locks
* [3] no setting of unsupported bits
- * [4] doing anything requires privilege (go read about
- * the "sendmail capabilities bug")
*/
)
/* cannot change a locked bit */
return -EPERM;
+ /*
+ * Doing anything requires privilege (go read about the
+ * "sendmail capabilities bug"), except for unprivileged bits.
+ * Indeed, the SECURE_ALL_UNPRIVILEGED bits are not
+ * restrictions enforced by the kernel but by user space on
+ * itself.
+ */
+ if (cap_capable(current_cred(), current_cred()->user_ns,
+ CAP_SETPCAP, CAP_OPT_NONE) != 0) {
+ const unsigned long unpriv_and_locks =
+ SECURE_ALL_UNPRIVILEGED |
+ SECURE_ALL_UNPRIVILEGED << 1;
+ const unsigned long changed = old->securebits ^ arg2;
+
+ /* For legacy reason, denies non-change. */
+ if (!changed)
+ return -EPERM;
+
+ /* Denies privileged changes. */
+ if (changed & ~unpriv_and_locks)
+ return -EPERM;
+ }
+
new = prepare_creds();
if (!new)
return -ENOMEM;
--
2.47.0
^ permalink raw reply related
* [PATCH v21 1/6] exec: Add a new AT_EXECVE_CHECK flag to execveat(2)
From: Mickaël Salaün @ 2024-11-12 19:18 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, Adhemerval Zanella Netto,
Alejandro Colomar, Aleksa Sarai, Andrew Morton, Andy Lutomirski,
Arnd Bergmann, Casey Schaufler, Christian Heimes, Dmitry Vyukov,
Elliott Hughes, Eric Biggers, Eric Chiang, Fan Wu, Florian Weimer,
Geert Uytterhoeven, James Morris, Jan Kara, Jann Horn, Jeff Xu,
Jonathan Corbet, Jordan R Abrahams, Lakshmi Ramasubramanian,
Linus Torvalds, Luca Boccassi, Luis Chamberlain,
Madhavan T . Venkataraman, Matt Bobrowski, Matthew Garrett,
Matthew Wilcox, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Scott Shell, Shuah Khan, Stephen Rothwell, Steve Dower,
Steve Grubb, Theodore Ts'o, Thibaut Sautereau,
Vincent Strubel, Xiaoming Ni, Yin Fengwei, kernel-hardening,
linux-api, linux-fsdevel, linux-integrity, linux-kernel,
linux-security-module
In-Reply-To: <20241112191858.162021-1-mic@digikod.net>
Add a new AT_EXECVE_CHECK flag to execveat(2) to check if a file would
be allowed for execution. The main use case is for script interpreters
and dynamic linkers to check execution permission according to the
kernel's security policy. Another use case is to add context to access
logs e.g., which script (instead of interpreter) accessed a file. As
any executable code, scripts could also use this check [1].
This is different from faccessat(2) + X_OK which only checks a subset of
access rights (i.e. inode permission and mount options for regular
files), but not the full context (e.g. all LSM access checks). The main
use case for access(2) is for SUID processes to (partially) check access
on behalf of their caller. The main use case for execveat(2) +
AT_EXECVE_CHECK is to check if a script execution would be allowed,
according to all the different restrictions in place. Because the use
of AT_EXECVE_CHECK follows the exact kernel semantic as for a real
execution, user space gets the same error codes.
An interesting point of using execveat(2) instead of openat2(2) is that
it decouples the check from the enforcement. Indeed, the security check
can be logged (e.g. with audit) without blocking an execution
environment not yet ready to enforce a strict security policy.
LSMs can control or log execution requests with
security_bprm_creds_for_exec(). However, to enforce a consistent and
complete access control (e.g. on binary's dependencies) LSMs should
restrict file executability, or mesure executed files, with
security_file_open() by checking file->f_flags & __FMODE_EXEC.
Because AT_EXECVE_CHECK is dedicated to user space interpreters, it
doesn't make sense for the kernel to parse the checked files, look for
interpreters known to the kernel (e.g. ELF, shebang), and return ENOEXEC
if the format is unknown. Because of that, security_bprm_check() is
never called when AT_EXECVE_CHECK is used.
It should be noted that script interpreters cannot directly use
execveat(2) (without this new AT_EXECVE_CHECK flag) because this could
lead to unexpected behaviors e.g., `python script.sh` could lead to Bash
being executed to interpret the script. Unlike the kernel, script
interpreters may just interpret the shebang as a simple comment, which
should not change for backward compatibility reasons.
Because scripts or libraries files might not currently have the
executable permission set, or because we might want specific users to be
allowed to run arbitrary scripts, the following patch provides a dynamic
configuration mechanism with the SECBIT_EXEC_RESTRICT_FILE and
SECBIT_EXEC_DENY_INTERACTIVE securebits.
This is a redesign of the CLIP OS 4's O_MAYEXEC:
https://github.com/clipos-archive/src_platform_clip-patches/blob/f5cb330d6b684752e403b4e41b39f7004d88e561/1901_open_mayexec.patch
This patch has been used for more than a decade with customized script
interpreters. Some examples can be found here:
https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Link: https://docs.python.org/3/library/io.html#io.open_code [1]
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20241112191858.162021-2-mic@digikod.net
---
Changes since v20:
* Rename AT_CHECK to AT_EXECVE_CHECK, requested by Amir Goldstein and
Serge Hallyn.
* Move the UAPI documentation to a dedicated RST file.
* Add Reviewed-by: Serge Hallyn
Changes since v19:
* Remove mention of "role transition" as suggested by Andy.
* Highlight the difference between security_bprm_creds_for_exec() and
the __FMODE_EXEC check for LSMs (in commit message and LSM's hooks) as
discussed with Jeff.
* Improve documentation both in UAPI comments and kernel comments
(requested by Kees).
New design since v18:
https://lore.kernel.org/r/20220104155024.48023-3-mic@digikod.net
---
Documentation/userspace-api/check_exec.rst | 34 ++++++++++++++++++++++
Documentation/userspace-api/index.rst | 1 +
fs/exec.c | 20 +++++++++++--
include/linux/binfmts.h | 7 ++++-
include/uapi/linux/fcntl.h | 4 +++
kernel/audit.h | 1 +
kernel/auditsc.c | 1 +
security/security.c | 10 +++++++
8 files changed, 75 insertions(+), 3 deletions(-)
create mode 100644 Documentation/userspace-api/check_exec.rst
diff --git a/Documentation/userspace-api/check_exec.rst b/Documentation/userspace-api/check_exec.rst
new file mode 100644
index 000000000000..ad1aeaa5f6c0
--- /dev/null
+++ b/Documentation/userspace-api/check_exec.rst
@@ -0,0 +1,34 @@
+===================
+Executability check
+===================
+
+AT_EXECVE_CHECK
+===============
+
+Passing the ``AT_EXECVE_CHECK`` flag to :manpage:`execveat(2)` only performs a
+check on a regular file and returns 0 if execution of this file would be
+allowed, ignoring the file format and then the related interpreter dependencies
+(e.g. ELF libraries, script's shebang).
+
+Programs should always perform this check to apply kernel-level checks against
+files that are not directly executed by the kernel but passed to a user space
+interpreter instead. All files that contain executable code, from the point of
+view of the interpreter, should be checked. However the result of this check
+should only be enforced according to ``SECBIT_EXEC_RESTRICT_FILE`` or
+``SECBIT_EXEC_DENY_INTERACTIVE.``.
+
+The main purpose of this flag is to improve the security and consistency of an
+execution environment to ensure that direct file execution (e.g.
+``./script.sh``) and indirect file execution (e.g. ``sh script.sh``) lead to
+the same result. For instance, this can be used to check if a file is
+trustworthy according to the caller's environment.
+
+In a secure environment, libraries and any executable dependencies should also
+be checked. For instance, dynamic linking should make sure that all libraries
+are allowed for execution to avoid trivial bypass (e.g. using ``LD_PRELOAD``).
+For such secure execution environment to make sense, only trusted code should
+be executable, which also requires integrity guarantees.
+
+To avoid race conditions leading to time-of-check to time-of-use issues,
+``AT_EXECVE_CHECK`` should be used with ``AT_EMPTY_PATH`` to check against a
+file descriptor instead of a path.
diff --git a/Documentation/userspace-api/index.rst b/Documentation/userspace-api/index.rst
index 274cc7546efc..6272bcf11296 100644
--- a/Documentation/userspace-api/index.rst
+++ b/Documentation/userspace-api/index.rst
@@ -35,6 +35,7 @@ Security-related interfaces
mfd_noexec
spec_ctrl
tee
+ check_exec
Devices and I/O
===============
diff --git a/fs/exec.c b/fs/exec.c
index 6c53920795c2..bb83b6a39530 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -891,7 +891,8 @@ static struct file *do_open_execat(int fd, struct filename *name, int flags)
.lookup_flags = LOOKUP_FOLLOW,
};
- if ((flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+ if ((flags &
+ ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH | AT_EXECVE_CHECK)) != 0)
return ERR_PTR(-EINVAL);
if (flags & AT_SYMLINK_NOFOLLOW)
open_exec_flags.lookup_flags &= ~LOOKUP_FOLLOW;
@@ -1545,6 +1546,21 @@ static struct linux_binprm *alloc_bprm(int fd, struct filename *filename, int fl
}
bprm->interp = bprm->filename;
+ /*
+ * At this point, security_file_open() has already been called (with
+ * __FMODE_EXEC) and access control checks for AT_EXECVE_CHECK will
+ * stop just after the security_bprm_creds_for_exec() call in
+ * bprm_execve(). Indeed, the kernel should not try to parse the
+ * content of the file with exec_binprm() nor change the calling
+ * thread, which means that the following security functions will be
+ * not called:
+ * - security_bprm_check()
+ * - security_bprm_creds_from_file()
+ * - security_bprm_committing_creds()
+ * - security_bprm_committed_creds()
+ */
+ bprm->is_check = !!(flags & AT_EXECVE_CHECK);
+
retval = bprm_mm_init(bprm);
if (!retval)
return bprm;
@@ -1839,7 +1855,7 @@ static int bprm_execve(struct linux_binprm *bprm)
/* Set the unchanging part of bprm->cred */
retval = security_bprm_creds_for_exec(bprm);
- if (retval)
+ if (retval || bprm->is_check)
goto out;
retval = exec_binprm(bprm);
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index e6c00e860951..8ff0eb3644a1 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -42,7 +42,12 @@ struct linux_binprm {
* Set when errors can no longer be returned to the
* original userspace.
*/
- point_of_no_return:1;
+ point_of_no_return:1,
+ /*
+ * Set by user space to check executability according to the
+ * caller's environment.
+ */
+ is_check:1;
struct file *executable; /* Executable to pass to the interpreter */
struct file *interpreter;
struct file *file;
diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
index 87e2dec79fea..2e87f2e3a79f 100644
--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
@@ -154,6 +154,10 @@
usable with open_by_handle_at(2). */
#define AT_HANDLE_MNT_ID_UNIQUE 0x001 /* Return the u64 unique mount ID. */
+/* Flags for execveat2(2). */
+#define AT_EXECVE_CHECK 0x10000 /* Only perform a check if execution
+ would be allowed. */
+
#if defined(__KERNEL__)
#define AT_GETATTR_NOSEC 0x80000000
#endif
diff --git a/kernel/audit.h b/kernel/audit.h
index a60d2840559e..8ebdabd2ab81 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -197,6 +197,7 @@ struct audit_context {
struct open_how openat2;
struct {
int argc;
+ bool is_check;
} execve;
struct {
char *name;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index cd57053b4a69..8d9ba5600cf2 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2662,6 +2662,7 @@ void __audit_bprm(struct linux_binprm *bprm)
context->type = AUDIT_EXECVE;
context->execve.argc = bprm->argc;
+ context->execve.is_check = bprm->is_check;
}
diff --git a/security/security.c b/security/security.c
index c5981e558bc2..456361ec249d 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1249,6 +1249,12 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages)
* to 1 if AT_SECURE should be set to request libc enable secure mode. @bprm
* contains the linux_binprm structure.
*
+ * If execveat(2) is called with the AT_EXECVE_CHECK flag, bprm->is_check is
+ * set. The result must be the same as without this flag even if the execution
+ * will never really happen and @bprm will always be dropped.
+ *
+ * This hook must not change current->cred, only @bprm->cred.
+ *
* Return: Returns 0 if the hook is successful and permission is granted.
*/
int security_bprm_creds_for_exec(struct linux_binprm *bprm)
@@ -3100,6 +3106,10 @@ int security_file_receive(struct file *file)
* Save open-time permission checking state for later use upon file_permission,
* and recheck access if anything has changed since inode_permission.
*
+ * We can check if a file is opened for execution (e.g. execve(2) call), either
+ * directly or indirectly (e.g. ELF's ld.so) by checking file->f_flags &
+ * __FMODE_EXEC .
+ *
* Return: Returns 0 if permission is granted.
*/
int security_file_open(struct file *file)
--
2.47.0
^ 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