From: Roberto Sassu <[email protected]>
This patch set depends on:
- https://lore.kernel.org/linux-integrity/[email protected]/ (there will be a v8 shortly)
- https://lore.kernel.org/linux-security-module/[email protected]/
IMA and EVM are not effectively LSMs, especially due the fact that in the
past they could not provide a security blob while there is another LSM
active.
That changed in the recent years, the LSM stacking feature now makes it
possible to stack together multiple LSMs, and allows them to provide a
security blob for most kernel objects. While the LSM stacking feature has
some limitations being worked out, it is already suitable to make IMA and
EVM as LSMs.
In short, while this patch set is big, it does not make any functional
change to IMA and EVM. IMA and EVM functions are called by the LSM
infrastructure in the same places as before (except ima_post_path_mknod()),
rather being hardcoded calls, and the inode metadata pointer is directly
stored in the inode security blob rather than in a separate rbtree.
More specifically, patches 1-13 make IMA and EVM functions suitable to
be registered to the LSM infrastructure, by aligning function parameters.
Patches 14-22 add new LSM hooks in the same places where IMA and EVM
functions are called, if there is no LSM hook already.
Patch 23 adds the 'last' ordering strategy for LSMs, so that IMA and EVM
functions are called in the same order as of today. Also, like with the
'first' strategy, LSMs using it are always enabled, so IMA and EVM
functions will be always called (if IMA and EVM are compiled built-in).
Patches 24-27 do the bulk of the work, remove hardcoded calls to IMA and
EVM functions, register those functions in the LSM infrastructure, and let
the latter call them. In addition, they also reserve one slot for EVM to
supply an xattr to the inode_init_security hook.
Finally, patch 28 removes the rbtree used to bind metadata to the inodes,
and instead reserve a space in the inode security blob to store the pointer
to metadata. This also brings performance improvements due to retrieving
metadata in constant time, as opposed to logarithmic.
Roberto Sassu (28):
ima: Align ima_inode_post_setattr() definition with LSM infrastructure
ima: Align ima_post_path_mknod() definition with LSM infrastructure
ima: Align ima_post_create_tmpfile() definition with LSM
infrastructure
ima: Align ima_file_mprotect() definition with LSM infrastructure
ima: Align ima_inode_setxattr() definition with LSM infrastructure
ima: Align ima_inode_removexattr() definition with LSM infrastructure
ima: Align ima_post_read_file() definition with LSM infrastructure
evm: Align evm_inode_post_setattr() definition with LSM infrastructure
evm: Align evm_inode_setxattr() definition with LSM infrastructure
evm: Align evm_inode_post_setxattr() definition with LSM
infrastructure
evm: Complete description of evm_inode_setattr()
fs: Fix description of vfs_tmpfile()
security: Align inode_setattr hook definition with EVM
security: Introduce inode_post_setattr hook
security: Introduce inode_post_removexattr hook
security: Introduce file_post_open hook
security: Introduce file_pre_free_security hook
security: Introduce path_post_mknod hook
security: Introduce inode_post_create_tmpfile hook
security: Introduce inode_post_set_acl hook
security: Introduce inode_post_remove_acl hook
security: Introduce key_post_create_or_update hook
security: Introduce LSM_ORDER_LAST
ima: Move to LSM infrastructure
ima: Move IMA-Appraisal to LSM infrastructure
evm: Move to LSM infrastructure
integrity: Move integrity functions to the LSM infrastructure
integrity: Switch from rbtree to LSM-managed blob for
integrity_iint_cache
fs/attr.c | 5 +-
fs/file_table.c | 3 +-
fs/namei.c | 13 +-
fs/nfsd/vfs.c | 3 +-
fs/open.c | 1 -
fs/posix_acl.c | 5 +-
fs/xattr.c | 3 +-
include/linux/evm.h | 112 -----------
include/linux/ima.h | 142 -------------
include/linux/integrity.h | 26 ---
include/linux/lsm_hook_defs.h | 21 +-
include/linux/lsm_hooks.h | 1 +
include/linux/security.h | 65 ++++++
security/integrity/evm/evm_main.c | 109 ++++++++--
security/integrity/iint.c | 90 +++------
security/integrity/ima/ima.h | 12 ++
security/integrity/ima/ima_appraise.c | 38 +++-
security/integrity/ima/ima_main.c | 77 +++++--
security/integrity/integrity.h | 44 +++-
security/keys/key.c | 10 +-
security/security.c | 276 ++++++++++++++++----------
security/selinux/hooks.c | 3 +-
security/smack/smack_lsm.c | 4 +-
23 files changed, 550 insertions(+), 513 deletions(-)
--
2.25.1
From: Roberto Sassu <[email protected]>
Change ima_post_create_tmpfile() definition, so that it can be registered
as implementation of the post_create_tmpfile hook.
Signed-off-by: Roberto Sassu <[email protected]>
---
fs/namei.c | 2 +-
include/linux/ima.h | 7 +++++--
security/integrity/ima/ima_main.c | 8 ++++++--
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index b5a1ec29193..57727a1ae38 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3622,7 +3622,7 @@ static int vfs_tmpfile(struct mnt_idmap *idmap,
inode->i_state |= I_LINKABLE;
spin_unlock(&inode->i_lock);
}
- ima_post_create_tmpfile(idmap, inode);
+ ima_post_create_tmpfile(idmap, dir, file_dentry(file), mode);
return 0;
}
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 179ce52013b..7535686a403 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -19,7 +19,8 @@ extern enum hash_algo ima_get_current_hash_algo(void);
extern int ima_bprm_check(struct linux_binprm *bprm);
extern int ima_file_check(struct file *file, int mask);
extern void ima_post_create_tmpfile(struct mnt_idmap *idmap,
- struct inode *inode);
+ struct inode *dir, struct dentry *dentry,
+ umode_t mode);
extern void ima_file_free(struct file *file);
extern int ima_file_mmap(struct file *file, unsigned long reqprot,
unsigned long prot, unsigned long flags);
@@ -69,7 +70,9 @@ static inline int ima_file_check(struct file *file, int mask)
}
static inline void ima_post_create_tmpfile(struct mnt_idmap *idmap,
- struct inode *inode)
+ struct inode *dir,
+ struct dentry *dentry,
+ umode_t mode)
{
}
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 8941305376b..4a3d0c8bcba 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -659,16 +659,20 @@ EXPORT_SYMBOL_GPL(ima_inode_hash);
/**
* ima_post_create_tmpfile - mark newly created tmpfile as new
* @idmap: idmap of the mount the inode was found from
- * @inode: inode of the newly created tmpfile
+ * @dir: inode structure of the parent of the new file
+ * @dentry: dentry structure of the new file
+ * @mode: mode of the new file
*
* No measuring, appraising or auditing of newly created tmpfiles is needed.
* Skip calling process_measurement(), but indicate which newly, created
* tmpfiles are in policy.
*/
void ima_post_create_tmpfile(struct mnt_idmap *idmap,
- struct inode *inode)
+ struct inode *dir, struct dentry *dentry,
+ umode_t mode)
{
struct integrity_iint_cache *iint;
+ struct inode *inode = dentry->d_inode;
int must_appraise;
if (!ima_policy_flag || !S_ISREG(inode->i_mode))
--
2.25.1
From: Roberto Sassu <[email protected]>
Change evm_inode_setxattr() definition, so that it can be registered as
implementation of the inode_setxattr hook.
Signed-off-by: Roberto Sassu <[email protected]>
---
include/linux/evm.h | 4 ++--
security/integrity/evm/evm_main.c | 3 ++-
security/security.c | 2 +-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/linux/evm.h b/include/linux/evm.h
index b41f98791a7..2f8c5a3983f 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -27,7 +27,7 @@ extern void evm_inode_post_setattr(struct mnt_idmap *idmap,
struct dentry *dentry, int ia_valid);
extern int evm_inode_setxattr(struct mnt_idmap *idmap,
struct dentry *dentry, const char *name,
- const void *value, size_t size);
+ const void *value, size_t size, int flags);
extern void evm_inode_post_setxattr(struct dentry *dentry,
const char *xattr_name,
const void *xattr_value,
@@ -105,7 +105,7 @@ static inline void evm_inode_post_setattr(struct mnt_idmap *idmap,
static inline int evm_inode_setxattr(struct mnt_idmap *idmap,
struct dentry *dentry, const char *name,
- const void *value, size_t size)
+ const void *value, size_t size, int flags)
{
return 0;
}
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 0cb63dfc998..c884534f3b7 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -558,6 +558,7 @@ static int evm_protect_xattr(struct mnt_idmap *idmap,
* @xattr_name: pointer to the affected extended attribute name
* @xattr_value: pointer to the new extended attribute value
* @xattr_value_len: pointer to the new extended attribute value length
+ * @flags: flags to pass into filesystem operations
*
* Before allowing the 'security.evm' protected xattr to be updated,
* verify the existing value is valid. As only the kernel should have
@@ -567,7 +568,7 @@ static int evm_protect_xattr(struct mnt_idmap *idmap,
*/
int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
const char *xattr_name, const void *xattr_value,
- size_t xattr_value_len)
+ size_t xattr_value_len, int flags)
{
const struct evm_ima_xattr_data *xattr_data = xattr_value;
diff --git a/security/security.c b/security/security.c
index 29b4bebba5e..dcb98901eb9 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2224,7 +2224,7 @@ int security_inode_setxattr(struct mnt_idmap *idmap,
ret = ima_inode_setxattr(idmap, dentry, name, value, size, flags);
if (ret)
return ret;
- return evm_inode_setxattr(idmap, dentry, name, value, size);
+ return evm_inode_setxattr(idmap, dentry, name, value, size, flags);
}
/**
--
2.25.1
From: Roberto Sassu <[email protected]>
Change ima_inode_post_setattr() definition, so that it can be registered as
implementation of the inode_post_setattr hook.
Signed-off-by: Roberto Sassu <[email protected]>
---
fs/attr.c | 2 +-
include/linux/ima.h | 4 ++--
security/integrity/ima/ima_appraise.c | 3 ++-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/attr.c b/fs/attr.c
index aca9ff7aed3..5050ab4cc45 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -485,7 +485,7 @@ int notify_change(struct mnt_idmap *idmap, struct dentry *dentry,
if (!error) {
fsnotify_change(dentry, ia_valid);
- ima_inode_post_setattr(idmap, dentry);
+ ima_inode_post_setattr(idmap, dentry, ia_valid);
evm_inode_post_setattr(dentry, ia_valid);
}
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 86b57757c7b..910a2f11a90 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -186,7 +186,7 @@ static inline void ima_post_key_create_or_update(struct key *keyring,
#ifdef CONFIG_IMA_APPRAISE
extern bool is_ima_appraise_enabled(void);
extern void ima_inode_post_setattr(struct mnt_idmap *idmap,
- struct dentry *dentry);
+ struct dentry *dentry, int ia_valid);
extern int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
const void *xattr_value, size_t xattr_value_len);
extern int ima_inode_set_acl(struct mnt_idmap *idmap,
@@ -206,7 +206,7 @@ static inline bool is_ima_appraise_enabled(void)
}
static inline void ima_inode_post_setattr(struct mnt_idmap *idmap,
- struct dentry *dentry)
+ struct dentry *dentry, int ia_valid)
{
return;
}
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 491c1aca0b1..6b032bce4fe 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -627,6 +627,7 @@ void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file)
* ima_inode_post_setattr - reflect file metadata changes
* @idmap: idmap of the mount the inode was found from
* @dentry: pointer to the affected dentry
+ * @ia_valid: for the UID and GID status
*
* Changes to a dentry's metadata might result in needing to appraise.
*
@@ -634,7 +635,7 @@ void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file)
* to lock the inode's i_mutex.
*/
void ima_inode_post_setattr(struct mnt_idmap *idmap,
- struct dentry *dentry)
+ struct dentry *dentry, int ia_valid)
{
struct inode *inode = d_backing_inode(dentry);
struct integrity_iint_cache *iint;
--
2.25.1
From: Roberto Sassu <[email protected]>
Change ima_post_read_file() definition, so that it can be registered as
implementation of the post_read_file hook.
Signed-off-by: Roberto Sassu <[email protected]>
---
include/linux/ima.h | 4 ++--
security/integrity/ima/ima_main.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/ima.h b/include/linux/ima.h
index fb1d4699949..f56c6280667 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -31,7 +31,7 @@ extern int ima_post_load_data(char *buf, loff_t size,
enum kernel_load_data_id id, char *description);
extern int ima_read_file(struct file *file, enum kernel_read_file_id id,
bool contents);
-extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
+extern int ima_post_read_file(struct file *file, char *buf, loff_t size,
enum kernel_read_file_id id);
extern void ima_post_path_mknod(struct mnt_idmap *idmap,
const struct path *dir, struct dentry *dentry,
@@ -112,7 +112,7 @@ static inline int ima_read_file(struct file *file, enum kernel_read_file_id id,
return 0;
}
-static inline int ima_post_read_file(struct file *file, void *buf, loff_t size,
+static inline int ima_post_read_file(struct file *file, char *buf, loff_t size,
enum kernel_read_file_id id)
{
return 0;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 4e26fd49ae7..b7835e287d9 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -797,7 +797,7 @@ const int read_idmap[READING_MAX_ID] = {
* On success return 0. On integrity appraisal error, assuming the file
* is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
*/
-int ima_post_read_file(struct file *file, void *buf, loff_t size,
+int ima_post_read_file(struct file *file, char *buf, loff_t size,
enum kernel_read_file_id read_id)
{
enum ima_hooks func;
--
2.25.1
From: Roberto Sassu <[email protected]>
Add the description for missing parameters of evm_inode_setattr() to
avoid the warning arising with W=n compile option.
Fixes: 817b54aa45db ("evm: add evm_inode_setattr to prevent updating an invalid security.evm")
Fixes: c1632a0f1120 ("fs: port ->setattr() to pass mnt_idmap")
Signed-off-by: Roberto Sassu <[email protected]>
---
security/integrity/evm/evm_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 1155a58ae87..8b5c472f78b 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -798,7 +798,9 @@ static int evm_attr_change(struct mnt_idmap *idmap,
/**
* evm_inode_setattr - prevent updating an invalid EVM extended attribute
+ * @idmap: idmap of the mount
* @dentry: pointer to the affected dentry
+ * @attr: iattr structure containing the new file attributes
*
* Permit update of file attributes when files have a valid EVM signature,
* except in the case of them having an immutable portable signature.
--
2.25.1
From: Roberto Sassu <[email protected]>
In preparation for moving IMA and EVM to the LSM infrastructure, introduce
the inode_post_remove_acl hook.
Signed-off-by: Roberto Sassu <[email protected]>
---
fs/posix_acl.c | 1 +
include/linux/lsm_hook_defs.h | 2 ++
include/linux/security.h | 8 ++++++++
security/security.c | 17 +++++++++++++++++
4 files changed, 28 insertions(+)
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index acddf2dff4c..5b8c92fce0c 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -1213,6 +1213,7 @@ int vfs_remove_acl(struct mnt_idmap *idmap, struct dentry *dentry,
error = -EOPNOTSUPP;
if (!error) {
fsnotify_xattr(dentry);
+ security_inode_post_remove_acl(idmap, dentry, acl_name);
evm_inode_post_remove_acl(idmap, dentry, acl_name);
}
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 9a3e14db0af..6c324fe5099 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -162,6 +162,8 @@ LSM_HOOK(int, 0, inode_get_acl, struct mnt_idmap *idmap,
struct dentry *dentry, const char *acl_name)
LSM_HOOK(int, 0, inode_remove_acl, struct mnt_idmap *idmap,
struct dentry *dentry, const char *acl_name)
+LSM_HOOK(void, LSM_RET_VOID, inode_post_remove_acl, struct mnt_idmap *idmap,
+ struct dentry *dentry, const char *acl_name)
LSM_HOOK(int, 0, inode_need_killpriv, struct dentry *dentry)
LSM_HOOK(int, 0, inode_killpriv, struct mnt_idmap *idmap,
struct dentry *dentry)
diff --git a/include/linux/security.h b/include/linux/security.h
index b0691bf7237..f8df5b69667 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -372,6 +372,9 @@ int security_inode_get_acl(struct mnt_idmap *idmap,
struct dentry *dentry, const char *acl_name);
int security_inode_remove_acl(struct mnt_idmap *idmap,
struct dentry *dentry, const char *acl_name);
+void security_inode_post_remove_acl(struct mnt_idmap *idmap,
+ struct dentry *dentry,
+ const char *acl_name);
void security_inode_post_setxattr(struct dentry *dentry, const char *name,
const void *value, size_t size, int flags);
int security_inode_getxattr(struct dentry *dentry, const char *name);
@@ -914,6 +917,11 @@ static inline int security_inode_remove_acl(struct mnt_idmap *idmap,
return 0;
}
+static inline void security_inode_post_remove_acl(struct mnt_idmap *idmap,
+ struct dentry *dentry,
+ const char *acl_name)
+{ }
+
static inline void security_inode_post_setxattr(struct dentry *dentry,
const char *name, const void *value, size_t size, int flags)
{ }
diff --git a/security/security.c b/security/security.c
index fc11d70bb02..b3a9c317f75 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2373,6 +2373,23 @@ int security_inode_remove_acl(struct mnt_idmap *idmap,
return evm_inode_remove_acl(idmap, dentry, acl_name);
}
+/**
+ * security_inode_post_remove_acl() - Update inode sec after remove_acl op
+ * @idmap: idmap of the mount
+ * @dentry: file
+ * @acl_name: acl name
+ *
+ * Update inode security field after successful remove_acl operation on @dentry
+ * in @idmap. The posix acls are identified by @acl_name.
+ */
+void security_inode_post_remove_acl(struct mnt_idmap *idmap,
+ struct dentry *dentry, const char *acl_name)
+{
+ if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
+ return;
+ call_void_hook(inode_post_remove_acl, idmap, dentry, acl_name);
+}
+
/**
* security_inode_post_setxattr() - Update the inode after a setxattr operation
* @dentry: file
--
2.25.1
From: Roberto Sassu <[email protected]>
In preparation for moving IMA and EVM to the LSM infrastructure, introduce
the key_post_create_or_update hook.
Signed-off-by: Roberto Sassu <[email protected]>
---
include/linux/lsm_hook_defs.h | 3 +++
include/linux/security.h | 11 +++++++++++
security/keys/key.c | 7 ++++++-
security/security.c | 19 +++++++++++++++++++
4 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 6c324fe5099..cf171e65420 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -397,6 +397,9 @@ LSM_HOOK(void, LSM_RET_VOID, key_free, struct key *key)
LSM_HOOK(int, 0, key_permission, key_ref_t key_ref, const struct cred *cred,
enum key_need_perm need_perm)
LSM_HOOK(int, 0, key_getsecurity, struct key *key, char **_buffer)
+LSM_HOOK(void, LSM_RET_VOID, key_post_create_or_update, struct key *keyring,
+ struct key *key, const void *payload, size_t payload_len,
+ unsigned long flags, bool create)
#endif /* CONFIG_KEYS */
#ifdef CONFIG_AUDIT
diff --git a/include/linux/security.h b/include/linux/security.h
index f8df5b69667..be23a303bba 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1952,6 +1952,9 @@ void security_key_free(struct key *key);
int security_key_permission(key_ref_t key_ref, const struct cred *cred,
enum key_need_perm need_perm);
int security_key_getsecurity(struct key *key, char **_buffer);
+void security_key_post_create_or_update(struct key *keyring, struct key *key,
+ const void *payload, size_t payload_len,
+ unsigned long flags, bool create);
#else
@@ -1979,6 +1982,14 @@ static inline int security_key_getsecurity(struct key *key, char **_buffer)
return 0;
}
+static inline void security_key_post_create_or_update(struct key *keyring,
+ struct key *key,
+ const void *payload,
+ size_t payload_len,
+ unsigned long flags,
+ bool create)
+{ }
+
#endif
#endif /* CONFIG_KEYS */
diff --git a/security/keys/key.c b/security/keys/key.c
index 5c0c7df833f..0f9c6faf349 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -934,6 +934,8 @@ static key_ref_t __key_create_or_update(key_ref_t keyring_ref,
goto error_link_end;
}
+ security_key_post_create_or_update(keyring, key, payload, plen, flags,
+ true);
ima_post_key_create_or_update(keyring, key, payload, plen,
flags, true);
@@ -967,10 +969,13 @@ static key_ref_t __key_create_or_update(key_ref_t keyring_ref,
key_ref = __key_update(key_ref, &prep);
- if (!IS_ERR(key_ref))
+ if (!IS_ERR(key_ref)) {
+ security_key_post_create_or_update(keyring, key, payload, plen,
+ flags, false);
ima_post_key_create_or_update(keyring, key,
payload, plen,
flags, false);
+ }
goto error_free_prep;
}
diff --git a/security/security.c b/security/security.c
index b3a9c317f75..322090a50cd 100644
--- a/security/security.c
+++ b/security/security.c
@@ -5195,6 +5195,25 @@ int security_key_getsecurity(struct key *key, char **_buffer)
*_buffer = NULL;
return call_int_hook(key_getsecurity, 0, key, _buffer);
}
+
+/**
+ * security_key_post_create_or_update() - Tell caller of key creation or update
+ * @keyring: keyring to which the key is linked to
+ * @key: created or updated key
+ * @payload: data used to instantiate or update the key
+ * @payload_len: length of payload
+ * @flags: key flags
+ * @create: flag indicating whether the key was created or updated
+ *
+ * Notify the caller of a key creation or update.
+ */
+void security_key_post_create_or_update(struct key *keyring, struct key *key,
+ const void *payload, size_t payload_len,
+ unsigned long flags, bool create)
+{
+ call_void_hook(key_post_create_or_update, keyring, key, payload,
+ payload_len, flags, create);
+}
#endif /* CONFIG_KEYS */
#ifdef CONFIG_AUDIT
--
2.25.1
From: Roberto Sassu <[email protected]>
Remove hardcoded calls to integrity functions from the LSM infrastructure.
Also move the global declaration of integrity_inode_get() to
security/integrity/integrity.h, so that the function can be still called by
IMA.
Register integrity functions as hook implementations in
integrity_lsm_init().
Signed-off-by: Roberto Sassu <[email protected]>
---
include/linux/integrity.h | 26 --------------------------
security/integrity/iint.c | 11 ++++++++++-
security/integrity/integrity.h | 7 +++++++
security/security.c | 9 +--------
4 files changed, 18 insertions(+), 35 deletions(-)
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index 2ea0f2f65ab..afaae7ad26f 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -21,38 +21,12 @@ enum integrity_status {
/* List of EVM protected security xattrs */
#ifdef CONFIG_INTEGRITY
-extern struct integrity_iint_cache *integrity_inode_get(struct inode *inode);
-extern void integrity_inode_free(struct inode *inode);
extern void __init integrity_load_keys(void);
#else
-static inline struct integrity_iint_cache *
- integrity_inode_get(struct inode *inode)
-{
- return NULL;
-}
-
-static inline void integrity_inode_free(struct inode *inode)
-{
- return;
-}
-
static inline void integrity_load_keys(void)
{
}
#endif /* CONFIG_INTEGRITY */
-#ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
-
-extern int integrity_kernel_module_request(char *kmod_name);
-
-#else
-
-static inline int integrity_kernel_module_request(char *kmod_name)
-{
- return 0;
-}
-
-#endif /* CONFIG_INTEGRITY_ASYMMETRIC_KEYS */
-
#endif /* _LINUX_INTEGRITY_H */
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 952d5ea4e18..b12215d8b13 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -143,7 +143,7 @@ struct integrity_iint_cache *integrity_inode_get(struct inode *inode)
*
* Free the integrity information(iint) associated with an inode.
*/
-void integrity_inode_free(struct inode *inode)
+static void integrity_inode_free(struct inode *inode)
{
struct integrity_iint_cache *iint;
@@ -172,12 +172,21 @@ static void init_once(void *foo)
mutex_init(&iint->mutex);
}
+static struct security_hook_list integrity_hooks[] __lsm_ro_after_init = {
+ LSM_HOOK_INIT(inode_free_security, integrity_inode_free),
+#ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
+ LSM_HOOK_INIT(kernel_module_request, integrity_kernel_module_request),
+#endif
+};
+
static int __init integrity_lsm_init(void)
{
iint_cache =
kmem_cache_create("iint_cache", sizeof(struct integrity_iint_cache),
0, SLAB_PANIC, init_once);
+ security_add_hooks(integrity_hooks, ARRAY_SIZE(integrity_hooks),
+ "integrity");
init_ima_lsm();
init_evm_lsm();
return 0;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 76e7eda6651..a3cbc65f9c6 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -177,6 +177,7 @@ struct integrity_iint_cache {
* integrity data associated with an inode.
*/
struct integrity_iint_cache *integrity_iint_find(struct inode *inode);
+struct integrity_iint_cache *integrity_inode_get(struct inode *inode);
int integrity_kernel_read(struct file *file, loff_t offset,
void *addr, unsigned long count);
@@ -250,12 +251,18 @@ static inline int __init integrity_load_cert(const unsigned int id,
#ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
int asymmetric_verify(struct key *keyring, const char *sig,
int siglen, const char *data, int datalen);
+extern int integrity_kernel_module_request(char *kmod_name);
#else
static inline int asymmetric_verify(struct key *keyring, const char *sig,
int siglen, const char *data, int datalen)
{
return -EOPNOTSUPP;
}
+
+static inline int integrity_kernel_module_request(char *kmod_name)
+{
+ return 0;
+}
#endif
#ifdef CONFIG_IMA_APPRAISE_MODSIG
diff --git a/security/security.c b/security/security.c
index 74abf04feef..985e4bf3309 100644
--- a/security/security.c
+++ b/security/security.c
@@ -19,7 +19,6 @@
#include <linux/kernel.h>
#include <linux/kernel_read_file.h>
#include <linux/lsm_hooks.h>
-#include <linux/integrity.h>
#include <linux/fsnotify.h>
#include <linux/mman.h>
#include <linux/mount.h>
@@ -1496,7 +1495,6 @@ static void inode_free_by_rcu(struct rcu_head *head)
*/
void security_inode_free(struct inode *inode)
{
- integrity_inode_free(inode);
call_void_hook(inode_free_security, inode);
/*
* The inode may still be referenced in a path walk and
@@ -3147,12 +3145,7 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode)
*/
int security_kernel_module_request(char *kmod_name)
{
- int ret;
-
- ret = call_int_hook(kernel_module_request, 0, kmod_name);
- if (ret)
- return ret;
- return integrity_kernel_module_request(kmod_name);
+ return call_int_hook(kernel_module_request, 0, kmod_name);
}
/**
--
2.25.1
On 3/3/23 13:18, Roberto Sassu wrote:
> From: Roberto Sassu <[email protected]>
>
> In preparation for moving IMA and EVM to the LSM infrastructure, introduce
> the inode_post_remove_acl hook.
>
> Signed-off-by: Roberto Sassu <[email protected]>
> ---
>
> +/**
> + * security_inode_post_remove_acl() - Update inode sec after remove_acl op
> + * @idmap: idmap of the mount
> + * @dentry: file
> + * @acl_name: acl name
> + *
> + * Update inode security field after successful remove_acl operation on @dentry
> + * in @idmap. The posix acls are identified by @acl_name.
> + */
> +void security_inode_post_remove_acl(struct mnt_idmap *idmap,
> + struct dentry *dentry, const char *acl_name)
> +{
> + if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> + return;
Was that a mistake before that EVM and IMA functions did not filtered out private inodes?
Stefan
On Mon, 2023-03-06 at 10:22 -0500, Stefan Berger wrote:
>
> On 3/3/23 13:18, Roberto Sassu wrote:
> > From: Roberto Sassu <[email protected]>
> >
> > In preparation for moving IMA and EVM to the LSM infrastructure, introduce
> > the inode_post_remove_acl hook.
> >
> > Signed-off-by: Roberto Sassu <[email protected]>
> > ---
> >
> > +/**
> > + * security_inode_post_remove_acl() - Update inode sec after remove_acl op
> > + * @idmap: idmap of the mount
> > + * @dentry: file
> > + * @acl_name: acl name
> > + *
> > + * Update inode security field after successful remove_acl operation on @dentry
> > + * in @idmap. The posix acls are identified by @acl_name.
> > + */
> > +void security_inode_post_remove_acl(struct mnt_idmap *idmap,
> > + struct dentry *dentry, const char *acl_name)
> > +{
> > + if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> > + return;
>
> Was that a mistake before that EVM and IMA functions did not filtered out private inodes?
Looks like that. At least for hooks that are not called from
security.c.
Thanks
Roberto
On 3/3/23 13:18, Roberto Sassu wrote:
> From: Roberto Sassu <[email protected]>
>
> Change ima_inode_post_setattr() definition, so that it can be registered as
> implementation of the inode_post_setattr hook.
>
> Signed-off-by: Roberto Sassu <[email protected]>
Reviewed-by: Stefan Berger <[email protected]>
On 3/3/23 13:18, Roberto Sassu wrote:
> From: Roberto Sassu <[email protected]>
>
> Change evm_inode_setxattr() definition, so that it can be registered as
> implementation of the inode_setxattr hook.
Reviewed-by: Stefan Berger <[email protected]>
On 3/3/23 13:18, Roberto Sassu wrote:
> From: Roberto Sassu <[email protected]>
>
> Add the description for missing parameters of evm_inode_setattr() to
> avoid the warning arising with W=n compile option.
>
> Fixes: 817b54aa45db ("evm: add evm_inode_setattr to prevent updating an invalid security.evm")
> Fixes: c1632a0f1120 ("fs: port ->setattr() to pass mnt_idmap")
> Signed-off-by: Roberto Sassu <[email protected]>
Among the previous patches I think there were 2 fixes like this one you could possibly also split off.
Reviewed-by: Stefan Berger <[email protected]>
> ---
> security/integrity/evm/evm_main.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 1155a58ae87..8b5c472f78b 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -798,7 +798,9 @@ static int evm_attr_change(struct mnt_idmap *idmap,
>
> /**
> * evm_inode_setattr - prevent updating an invalid EVM extended attribute
> + * @idmap: idmap of the mount
> * @dentry: pointer to the affected dentry
> + * @attr: iattr structure containing the new file attributes
> *
> * Permit update of file attributes when files have a valid EVM signature,
> * except in the case of them having an immutable portable signature.
On Mon, 2023-03-06 at 11:16 -0500, Stefan Berger wrote:
>
> On 3/6/23 10:34, Roberto Sassu wrote:
> > On Mon, 2023-03-06 at 10:22 -0500, Stefan Berger wrote:
> > > On 3/3/23 13:18, Roberto Sassu wrote:
> > > > From: Roberto Sassu <[email protected]>
> > > >
> > > > In preparation for moving IMA and EVM to the LSM infrastructure, introduce
> > > > the inode_post_remove_acl hook.
> > > >
> > > > Signed-off-by: Roberto Sassu <[email protected]>
> > > > ---
> > > >
> > > > +/**
> > > > + * security_inode_post_remove_acl() - Update inode sec after remove_acl op
> > > > + * @idmap: idmap of the mount
> > > > + * @dentry: file
> > > > + * @acl_name: acl name
> > > > + *
> > > > + * Update inode security field after successful remove_acl operation on @dentry
> > > > + * in @idmap. The posix acls are identified by @acl_name.
> > > > + */
> > > > +void security_inode_post_remove_acl(struct mnt_idmap *idmap,
> > > > + struct dentry *dentry, const char *acl_name)
> > > > +{
> > > > + if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> > > > + return;
> > >
> > > Was that a mistake before that EVM and IMA functions did not filtered out private inodes?
> >
> > Looks like that. At least for hooks that are not called from
> > security.c.
>
> It seems like that all security_* functions are filtering on private inodes. Anonymous inodes have them and some filesystem set the S_PRIVATE flag. So it may not make a difference fro IMA and EVM then.
Currently, what it would happen is that the HMAC would be updated
without check, since the check function is usually in security.c
(skipped) and the post elsewhere.
With this patch set, also the post function would not be executed for
private inodes. Maybe, it is worth mentioning it in the next version.
Thanks
Roberto
On 3/3/23 13:18, Roberto Sassu wrote:
> From: Roberto Sassu <[email protected]>
>
> Change ima_post_create_tmpfile() definition, so that it can be registered
> as implementation of the post_create_tmpfile hook.
>
> Signed-off-by: Roberto Sassu <[email protected]>
Reviewed-by: Stefan Berger <[email protected]>
On 3/3/23 13:18, Roberto Sassu wrote:
> From: Roberto Sassu <[email protected]>
>
> Change ima_post_read_file() definition, so that it can be registered as
> implementation of the post_read_file hook.
>
> Signed-off-by: Roberto Sassu <[email protected]>
Reviewed-by: Stefan Berger <[email protected]>
On 3/6/23 10:34, Roberto Sassu wrote:
> On Mon, 2023-03-06 at 10:22 -0500, Stefan Berger wrote:
>>
>> On 3/3/23 13:18, Roberto Sassu wrote:
>>> From: Roberto Sassu <[email protected]>
>>>
>>> In preparation for moving IMA and EVM to the LSM infrastructure, introduce
>>> the inode_post_remove_acl hook.
>>>
>>> Signed-off-by: Roberto Sassu <[email protected]>
>>> ---
>>>
>>> +/**
>>> + * security_inode_post_remove_acl() - Update inode sec after remove_acl op
>>> + * @idmap: idmap of the mount
>>> + * @dentry: file
>>> + * @acl_name: acl name
>>> + *
>>> + * Update inode security field after successful remove_acl operation on @dentry
>>> + * in @idmap. The posix acls are identified by @acl_name.
>>> + */
>>> +void security_inode_post_remove_acl(struct mnt_idmap *idmap,
>>> + struct dentry *dentry, const char *acl_name)
>>> +{
>>> + if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
>>> + return;
>>
>> Was that a mistake before that EVM and IMA functions did not filtered out private inodes?
>
> Looks like that. At least for hooks that are not called from
> security.c.
It seems like that all security_* functions are filtering on private inodes. Anonymous inodes have them and some filesystem set the S_PRIVATE flag. So it may not make a difference fro IMA and EVM then.
Stefan
>
> Thanks
>
> Roberto
>
On Mon, 2023-03-06 at 12:04 -0500, Stefan Berger wrote:
>
> On 3/3/23 13:18, Roberto Sassu wrote:
> > From: Roberto Sassu <[email protected]>
> >
> > Add the description for missing parameters of evm_inode_setattr() to
> > avoid the warning arising with W=n compile option.
> >
> > Fixes: 817b54aa45db ("evm: add evm_inode_setattr to prevent updating an invalid security.evm")
> > Fixes: c1632a0f1120 ("fs: port ->setattr() to pass mnt_idmap")
> > Signed-off-by: Roberto Sassu <[email protected]>
>
> Among the previous patches I think there were 2 fixes like this one you could possibly also split off.
Didn't find it.
Thanks
Roberto
> Reviewed-by: Stefan Berger <[email protected]>
> > ---
> > security/integrity/evm/evm_main.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> > index 1155a58ae87..8b5c472f78b 100644
> > --- a/security/integrity/evm/evm_main.c
> > +++ b/security/integrity/evm/evm_main.c
> > @@ -798,7 +798,9 @@ static int evm_attr_change(struct mnt_idmap *idmap,
> >
> > /**
> > * evm_inode_setattr - prevent updating an invalid EVM extended attribute
> > + * @idmap: idmap of the mount
> > * @dentry: pointer to the affected dentry
> > + * @attr: iattr structure containing the new file attributes
> > *
> > * Permit update of file attributes when files have a valid EVM signature,
> > * except in the case of them having an immutable portable signature.
On 3/3/23 13:18, Roberto Sassu wrote:
> From: Roberto Sassu <[email protected]>
>
> In preparation for moving IMA and EVM to the LSM infrastructure, introduce
> the key_post_create_or_update hook.
>
> Signed-off-by: Roberto Sassu <[email protected]>
Reviewed-by: Stefan Berger <[email protected]>
Hi Roberto,
On Fri, 2023-03-03 at 19:18 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <[email protected]>
>
> This patch set depends on:
> - https://lore.kernel.org/linux-integrity/[email protected]/ (there will be a v8 shortly)
> - https://lore.kernel.org/linux-security-module/[email protected]/
>
> IMA and EVM are not effectively LSMs, especially due the fact that in the
> past they could not provide a security blob while there is another LSM
> active.
>
> That changed in the recent years, the LSM stacking feature now makes it
> possible to stack together multiple LSMs, and allows them to provide a
> security blob for most kernel objects. While the LSM stacking feature has
> some limitations being worked out, it is already suitable to make IMA and
> EVM as LSMs.
>
> In short, while this patch set is big, it does not make any functional
> change to IMA and EVM. IMA and EVM functions are called by the LSM
> infrastructure in the same places as before (except ima_post_path_mknod()),
> rather being hardcoded calls, and the inode metadata pointer is directly
> stored in the inode security blob rather than in a separate rbtree.
>
> More specifically, patches 1-13 make IMA and EVM functions suitable to
> be registered to the LSM infrastructure, by aligning function parameters.
>
> Patches 14-22 add new LSM hooks in the same places where IMA and EVM
> functions are called, if there is no LSM hook already.
>
> Patch 23 adds the 'last' ordering strategy for LSMs, so that IMA and EVM
> functions are called in the same order as of today. Also, like with the
> 'first' strategy, LSMs using it are always enabled, so IMA and EVM
> functions will be always called (if IMA and EVM are compiled built-in).
>
> Patches 24-27 do the bulk of the work, remove hardcoded calls to IMA and
> EVM functions, register those functions in the LSM infrastructure, and let
> the latter call them. In addition, they also reserve one slot for EVM to
> supply an xattr to the inode_init_security hook.
>
> Finally, patch 28 removes the rbtree used to bind metadata to the inodes,
> and instead reserve a space in the inode security blob to store the pointer
> to metadata. This also brings performance improvements due to retrieving
> metadata in constant time, as opposed to logarithmic.
Prior to IMA being upstreamed, it went through a number of iterations,
first on the security hooks, then as a separate parallel set of
integrity hooks, and, finally, co-located with the security hooks,
where they exist. With this patch set we've come full circle.
With the LSM stacking support, multiple LSMs can now use the
'i_security' field removing the need for the rbtree indirection for
accessing integrity state info.
Roberto, thank you for making this change. Mostly it looks good.
Reviewing the patch set will be easier once the prereq's and this patch
set can be properly applied.
--
thanks,
Mimi
Hi Roberto,
On Fri, 2023-03-03 at 19:18 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <[email protected]>
>
> Change ima_post_create_tmpfile() definition, so that it can be registered
> as implementation of the post_create_tmpfile hook.
Since neither security_create_tmpfile() nor
security_post_create_tmpfile() already exist, why not pass a pointer to
the file to conform to the other file related security hooks?
Mimi
>
> Signed-off-by: Roberto Sassu <[email protected]>
> ---
> fs/namei.c | 2 +-
> include/linux/ima.h | 7 +++++--
> security/integrity/ima/ima_main.c | 8 ++++++--
> 3 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index b5a1ec29193..57727a1ae38 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3622,7 +3622,7 @@ static int vfs_tmpfile(struct mnt_idmap *idmap,
> inode->i_state |= I_LINKABLE;
> spin_unlock(&inode->i_lock);
> }
> - ima_post_create_tmpfile(idmap, inode);
> + ima_post_create_tmpfile(idmap, dir, file_dentry(file), mode);
> return 0;
> }
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 179ce52013b..7535686a403 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -19,7 +19,8 @@ extern enum hash_algo ima_get_current_hash_algo(void);
> extern int ima_bprm_check(struct linux_binprm *bprm);
> extern int ima_file_check(struct file *file, int mask);
> extern void ima_post_create_tmpfile(struct mnt_idmap *idmap,
> - struct inode *inode);
> + struct inode *dir, struct dentry *dentry,
> + umode_t mode);
> extern void ima_file_free(struct file *file);
> extern int ima_file_mmap(struct file *file, unsigned long reqprot,
> unsigned long prot, unsigned long flags);
> @@ -69,7 +70,9 @@ static inline int ima_file_check(struct file *file, int mask)
> }
>
> static inline void ima_post_create_tmpfile(struct mnt_idmap *idmap,
> - struct inode *inode)
> + struct inode *dir,
> + struct dentry *dentry,
> + umode_t mode)
> {
> }
>
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 8941305376b..4a3d0c8bcba 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -659,16 +659,20 @@ EXPORT_SYMBOL_GPL(ima_inode_hash);
> /**
> * ima_post_create_tmpfile - mark newly created tmpfile as new
> * @idmap: idmap of the mount the inode was found from
> - * @inode: inode of the newly created tmpfile
> + * @dir: inode structure of the parent of the new file
> + * @dentry: dentry structure of the new file
> + * @mode: mode of the new file
> *
> * No measuring, appraising or auditing of newly created tmpfiles is needed.
> * Skip calling process_measurement(), but indicate which newly, created
> * tmpfiles are in policy.
> */
> void ima_post_create_tmpfile(struct mnt_idmap *idmap,
> - struct inode *inode)
> + struct inode *dir, struct dentry *dentry,
> + umode_t mode)
> {
> struct integrity_iint_cache *iint;
> + struct inode *inode = dentry->d_inode;
> int must_appraise;
>
> if (!ima_policy_flag || !S_ISREG(inode->i_mode))
On Fri, 2023-03-03 at 19:18 +0100, Roberto Sassu wrote:
> diff --git a/security/security.c b/security/security.c
> index b3a9c317f75..322090a50cd 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -5195,6 +5195,25 @@ int security_key_getsecurity(struct key *key, char **_buffer)
> *_buffer = NULL;
> return call_int_hook(key_getsecurity, 0, key, _buffer);
> }
> +
> +/**
> + * security_key_post_create_or_update() - Tell caller of key creation or update
^Notification of key create or update
--
thanks,
Mimi
On Wed, 2023-03-08 at 10:14 -0500, Mimi Zohar wrote:
> Hi Roberto,
>
> On Fri, 2023-03-03 at 19:18 +0100, Roberto Sassu wrote:
> > From: Roberto Sassu <[email protected]>
> >
> > This patch set depends on:
> > - https://lore.kernel.org/linux-integrity/[email protected]/ (there will be a v8 shortly)
> > - https://lore.kernel.org/linux-security-module/[email protected]/
> >
> > IMA and EVM are not effectively LSMs, especially due the fact that in the
> > past they could not provide a security blob while there is another LSM
> > active.
> >
> > That changed in the recent years, the LSM stacking feature now makes it
> > possible to stack together multiple LSMs, and allows them to provide a
> > security blob for most kernel objects. While the LSM stacking feature has
> > some limitations being worked out, it is already suitable to make IMA and
> > EVM as LSMs.
> >
> > In short, while this patch set is big, it does not make any functional
> > change to IMA and EVM. IMA and EVM functions are called by the LSM
> > infrastructure in the same places as before (except ima_post_path_mknod()),
> > rather being hardcoded calls, and the inode metadata pointer is directly
> > stored in the inode security blob rather than in a separate rbtree.
> >
> > More specifically, patches 1-13 make IMA and EVM functions suitable to
> > be registered to the LSM infrastructure, by aligning function parameters.
> >
> > Patches 14-22 add new LSM hooks in the same places where IMA and EVM
> > functions are called, if there is no LSM hook already.
> >
> > Patch 23 adds the 'last' ordering strategy for LSMs, so that IMA and EVM
> > functions are called in the same order as of today. Also, like with the
> > 'first' strategy, LSMs using it are always enabled, so IMA and EVM
> > functions will be always called (if IMA and EVM are compiled built-in).
> >
> > Patches 24-27 do the bulk of the work, remove hardcoded calls to IMA and
> > EVM functions, register those functions in the LSM infrastructure, and let
> > the latter call them. In addition, they also reserve one slot for EVM to
> > supply an xattr to the inode_init_security hook.
> >
> > Finally, patch 28 removes the rbtree used to bind metadata to the inodes,
> > and instead reserve a space in the inode security blob to store the pointer
> > to metadata. This also brings performance improvements due to retrieving
> > metadata in constant time, as opposed to logarithmic.
>
> Prior to IMA being upstreamed, it went through a number of iterations,
> first on the security hooks, then as a separate parallel set of
> integrity hooks, and, finally, co-located with the security hooks,
> where they exist. With this patch set we've come full circle.
>
> With the LSM stacking support, multiple LSMs can now use the
> 'i_security' field removing the need for the rbtree indirection for
> accessing integrity state info.
>
> Roberto, thank you for making this change. Mostly it looks good.
> Reviewing the patch set will be easier once the prereq's and this patch
> set can be properly applied.
Welcome. Yes, once Paul reviews the other patch set, we can
progressively apply the patches.
Thanks
Roberto
On Wed, 2023-03-08 at 10:15 -0500, Mimi Zohar wrote:
> Hi Roberto,
>
> On Fri, 2023-03-03 at 19:18 +0100, Roberto Sassu wrote:
> > From: Roberto Sassu <[email protected]>
> >
> > Change ima_post_create_tmpfile() definition, so that it can be registered
> > as implementation of the post_create_tmpfile hook.
>
> Since neither security_create_tmpfile() nor
> security_post_create_tmpfile() already exist, why not pass a pointer to
> the file to conform to the other file related security hooks?
Ok, will change the parameter.
Thanks
Roberto
> Mimi
>
> > Signed-off-by: Roberto Sassu <[email protected]>
> > ---
> > fs/namei.c | 2 +-
> > include/linux/ima.h | 7 +++++--
> > security/integrity/ima/ima_main.c | 8 ++++++--
> > 3 files changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/namei.c b/fs/namei.c
> > index b5a1ec29193..57727a1ae38 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -3622,7 +3622,7 @@ static int vfs_tmpfile(struct mnt_idmap *idmap,
> > inode->i_state |= I_LINKABLE;
> > spin_unlock(&inode->i_lock);
> > }
> > - ima_post_create_tmpfile(idmap, inode);
> > + ima_post_create_tmpfile(idmap, dir, file_dentry(file), mode);
> > return 0;
> > }
> >
> > diff --git a/include/linux/ima.h b/include/linux/ima.h
> > index 179ce52013b..7535686a403 100644
> > --- a/include/linux/ima.h
> > +++ b/include/linux/ima.h
> > @@ -19,7 +19,8 @@ extern enum hash_algo ima_get_current_hash_algo(void);
> > extern int ima_bprm_check(struct linux_binprm *bprm);
> > extern int ima_file_check(struct file *file, int mask);
> > extern void ima_post_create_tmpfile(struct mnt_idmap *idmap,
> > - struct inode *inode);
> > + struct inode *dir, struct dentry *dentry,
> > + umode_t mode);
> > extern void ima_file_free(struct file *file);
> > extern int ima_file_mmap(struct file *file, unsigned long reqprot,
> > unsigned long prot, unsigned long flags);
> > @@ -69,7 +70,9 @@ static inline int ima_file_check(struct file *file, int mask)
> > }
> >
> > static inline void ima_post_create_tmpfile(struct mnt_idmap *idmap,
> > - struct inode *inode)
> > + struct inode *dir,
> > + struct dentry *dentry,
> > + umode_t mode)
> > {
> > }
> >
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 8941305376b..4a3d0c8bcba 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -659,16 +659,20 @@ EXPORT_SYMBOL_GPL(ima_inode_hash);
> > /**
> > * ima_post_create_tmpfile - mark newly created tmpfile as new
> > * @idmap: idmap of the mount the inode was found from
> > - * @inode: inode of the newly created tmpfile
> > + * @dir: inode structure of the parent of the new file
> > + * @dentry: dentry structure of the new file
> > + * @mode: mode of the new file
> > *
> > * No measuring, appraising or auditing of newly created tmpfiles is needed.
> > * Skip calling process_measurement(), but indicate which newly, created
> > * tmpfiles are in policy.
> > */
> > void ima_post_create_tmpfile(struct mnt_idmap *idmap,
> > - struct inode *inode)
> > + struct inode *dir, struct dentry *dentry,
> > + umode_t mode)
> > {
> > struct integrity_iint_cache *iint;
> > + struct inode *inode = dentry->d_inode;
> > int must_appraise;
> >
> > if (!ima_policy_flag || !S_ISREG(inode->i_mode))