* [PATCH 1/3] Add a new LSM-supporting anonymous inode interface
2020-02-14 3:26 ` [PATCH 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
@ 2020-02-14 3:26 ` Daniel Colascione
2020-02-14 3:26 ` [PATCH 2/3] Teach SELinux about anonymous inodes Daniel Colascione
` (6 subsequent siblings)
7 siblings, 0 replies; 60+ messages in thread
From: Daniel Colascione @ 2020-02-14 3:26 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra
Cc: Daniel Colascione
This change adds two new functions, anon_inode_getfile_secure and
anon_inode_getfd_secure, that create anonymous-node files with
individual non-S_PRIVATE inodes to which security modules can apply
policy. Existing callers continue using the original singleton-inode
kind of anonymous-inode file. We can transition anonymous inode users
to the new kind of anonymous inode in individual patches for the sake
of bisection and review.
The new functions accept an optional context_inode parameter that
callers can use to provide additional contextual information to
security modules, e.g., indicating that one anonymous struct file is a
logical child of another, allowing a security model to propagate
security information from one to the other.
Signed-off-by: Daniel Colascione <dancol@google.com>
---
fs/anon_inodes.c | 196 ++++++++++++++++++++++++++++--------
fs/userfaultfd.c | 4 +-
include/linux/anon_inodes.h | 13 +++
include/linux/lsm_hooks.h | 9 ++
include/linux/security.h | 4 +
security/security.c | 10 ++
6 files changed, 191 insertions(+), 45 deletions(-)
diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 89714308c25b..114a04fc1db4 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -55,75 +55,135 @@ static struct file_system_type anon_inode_fs_type = {
.kill_sb = kill_anon_super,
};
-/**
- * anon_inode_getfile - creates a new file instance by hooking it up to an
- * anonymous inode, and a dentry that describe the "class"
- * of the file
- *
- * @name: [in] name of the "class" of the new file
- * @fops: [in] file operations for the new file
- * @priv: [in] private data for the new file (will be file's private_data)
- * @flags: [in] flags
- *
- * Creates a new file by hooking it on a single inode. This is useful for files
- * that do not need to have a full-fledged inode in order to operate correctly.
- * All the files created with anon_inode_getfile() will share a single inode,
- * hence saving memory and avoiding code duplication for the file/inode/dentry
- * setup. Returns the newly created file* or an error pointer.
- */
-struct file *anon_inode_getfile(const char *name,
- const struct file_operations *fops,
- void *priv, int flags)
+static struct inode *anon_inode_make_secure_inode(
+ const char *name,
+ const struct inode *context_inode,
+ const struct file_operations *fops)
+{
+ struct inode *inode;
+ const struct qstr qname = QSTR_INIT(name, strlen(name));
+ int error;
+
+ inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
+ if (IS_ERR(inode))
+ return ERR_PTR(PTR_ERR(inode));
+ inode->i_flags &= ~S_PRIVATE;
+ error = security_inode_init_security_anon(
+ inode, &qname, fops, context_inode);
+ if (error) {
+ iput(inode);
+ return ERR_PTR(error);
+ }
+ return inode;
+}
+
+struct file *_anon_inode_getfile(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode,
+ bool secure)
{
+ struct inode *inode;
struct file *file;
- if (IS_ERR(anon_inode_inode))
- return ERR_PTR(-ENODEV);
+ if (secure) {
+ inode = anon_inode_make_secure_inode(
+ name, context_inode, fops);
+ if (IS_ERR(inode))
+ return ERR_PTR(PTR_ERR(inode));
+ } else {
+ inode = anon_inode_inode;
+ if (IS_ERR(inode))
+ return ERR_PTR(-ENODEV);
+ /*
+ * We know the anon_inode inode count is always
+ * greater than zero, so ihold() is safe.
+ */
+ ihold(inode);
+ }
- if (fops->owner && !try_module_get(fops->owner))
- return ERR_PTR(-ENOENT);
+ if (fops->owner && !try_module_get(fops->owner)) {
+ file = ERR_PTR(-ENOENT);
+ goto err;
+ }
- /*
- * We know the anon_inode inode count is always greater than zero,
- * so ihold() is safe.
- */
- ihold(anon_inode_inode);
- file = alloc_file_pseudo(anon_inode_inode, anon_inode_mnt, name,
+ file = alloc_file_pseudo(inode, anon_inode_mnt, name,
flags & (O_ACCMODE | O_NONBLOCK), fops);
if (IS_ERR(file))
goto err;
- file->f_mapping = anon_inode_inode->i_mapping;
+ file->f_mapping = inode->i_mapping;
file->private_data = priv;
return file;
err:
- iput(anon_inode_inode);
+ iput(inode);
module_put(fops->owner);
return file;
}
-EXPORT_SYMBOL_GPL(anon_inode_getfile);
/**
- * anon_inode_getfd - creates a new file instance by hooking it up to an
- * anonymous inode, and a dentry that describe the "class"
- * of the file
+ * anon_inode_getfile_secure - creates a new file instance by hooking
+ * it up to a new anonymous inode and a
+ * dentry that describe the "class" of the
+ * file. Make it possible to use security
+ * modules to control access to the
+ * new file.
*
* @name: [in] name of the "class" of the new file
* @fops: [in] file operations for the new file
* @priv: [in] private data for the new file (will be file's private_data)
- * @flags: [in] flags
+ * @flags: [in] flags for the file
+ * @anon_inode_flags: [in] flags for anon_inode*
+ *
+ * Creates a new file by hooking it on an unspecified inode. This is
+ * useful for files that do not need to have a full-fledged inode in
+ * order to operate correctly. All the files created with
+ * anon_inode_getfile_secure() will have distinct inodes, avoiding
+ * code duplication for the file/inode/dentry setup. Returns the
+ * newly created file* or an error pointer.
+ */
+struct file *anon_inode_getfile_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode)
+{
+ return _anon_inode_getfile(
+ name, fops, priv, flags, context_inode, true);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfile_secure);
+
+/**
+ * anon_inode_getfile - creates a new file instance by hooking it up
+ * to an anonymous inode and a dentry that
+ * describe the "class" of the file.
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags for the file
*
- * Creates a new file by hooking it on a single inode. This is useful for files
+ * Creates a new file by hooking it on an unspecified inode. This is useful for files
* that do not need to have a full-fledged inode in order to operate correctly.
- * All the files created with anon_inode_getfd() will share a single inode,
+ * All the files created with anon_inode_getfile() will share a single inode,
* hence saving memory and avoiding code duplication for the file/inode/dentry
- * setup. Returns new descriptor or an error code.
+ * setup. Returns the newly created file* or an error pointer.
*/
-int anon_inode_getfd(const char *name, const struct file_operations *fops,
- void *priv, int flags)
+struct file *anon_inode_getfile(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags)
+{
+ return _anon_inode_getfile(name, fops, priv, flags, NULL, false);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfile);
+
+static int _anon_inode_getfd(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode,
+ bool secure)
{
int error, fd;
struct file *file;
@@ -133,7 +193,8 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops,
return error;
fd = error;
- file = anon_inode_getfile(name, fops, priv, flags);
+ file = _anon_inode_getfile(name, fops, priv, flags, context_inode,
+ secure);
if (IS_ERR(file)) {
error = PTR_ERR(file);
goto err_put_unused_fd;
@@ -146,6 +207,57 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops,
put_unused_fd(fd);
return error;
}
+
+/**
+ * anon_inode_getfd_secure - creates a new file instance by hooking it
+ * up to a new anonymous inode and a dentry
+ * that describe the "class" of the file.
+ * Make it possible to use security modules
+ * to control access to the new file.
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags
+ *
+ * Creates a new file by hooking it on an unspecified inode. This is
+ * useful for files that do not need to have a full-fledged inode in
+ * order to operate correctly. All the files created with
+ * anon_inode_getfile_secure() will have distinct inodes, avoiding
+ * code duplication for the file/inode/dentry setup. Returns a newly
+ * created file descriptor or an error code.
+ */
+int anon_inode_getfd_secure(const char *name, const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode)
+{
+ return _anon_inode_getfd(name, fops, priv, flags,
+ context_inode, true);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfd_secure);
+
+/**
+ * anon_inode_getfd - creates a new file instance by hooking it up to
+ * an anonymous inode and a dentry that describe
+ * the "class" of the file
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags
+ *
+ * Creates a new file by hooking it on an unspecified inode. This is
+ * useful for files that do not need to have a full-fledged inode in
+ * order to operate correctly. All the files created with
+ * anon_inode_getfile() will use the same singleton inode, reducing
+ * memory use and avoiding code duplication for the file/inode/dentry
+ * setup. Returns a newly created file descriptor or an error code.
+ */
+int anon_inode_getfd(const char *name, const struct file_operations *fops,
+ void *priv, int flags)
+{
+ return _anon_inode_getfd(name, fops, priv, flags, NULL, false);
+}
EXPORT_SYMBOL_GPL(anon_inode_getfd);
static int __init anon_inode_init(void)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 37df7c9eedb1..07b0f6e03849 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1014,8 +1014,6 @@ static __poll_t userfaultfd_poll(struct file *file, poll_table *wait)
}
}
-static const struct file_operations userfaultfd_fops;
-
static int resolve_userfault_fork(struct userfaultfd_ctx *ctx,
struct userfaultfd_ctx *new,
struct uffd_msg *msg)
@@ -1920,7 +1918,7 @@ static void userfaultfd_show_fdinfo(struct seq_file *m, struct file *f)
}
#endif
-static const struct file_operations userfaultfd_fops = {
+const struct file_operations userfaultfd_fops = {
#ifdef CONFIG_PROC_FS
.show_fdinfo = userfaultfd_show_fdinfo,
#endif
diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
index d0d7d96261ad..67bd85d92dca 100644
--- a/include/linux/anon_inodes.h
+++ b/include/linux/anon_inodes.h
@@ -10,12 +10,25 @@
#define _LINUX_ANON_INODES_H
struct file_operations;
+struct inode;
+
+struct file *anon_inode_getfile_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode);
struct file *anon_inode_getfile(const char *name,
const struct file_operations *fops,
void *priv, int flags);
+
+int anon_inode_getfd_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode);
+
int anon_inode_getfd(const char *name, const struct file_operations *fops,
void *priv, int flags);
+
#endif /* _LINUX_ANON_INODES_H */
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 20d8cf194fb7..de5d37e388df 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -215,6 +215,10 @@
* Returns 0 if @name and @value have been successfully set,
* -EOPNOTSUPP if no security attribute is needed, or
* -ENOMEM on memory allocation failure.
+ * @inode_init_security_anon:
+ * Set up a secure anonymous inode.
+ * Returns 0 on success. Returns -EPERM if the security module denies
+ * the creation of this inode.
* @inode_create:
* Check permission to create a regular file.
* @dir contains inode structure of the parent of the new file.
@@ -1552,6 +1556,10 @@ union security_list_options {
const struct qstr *qstr,
const char **name, void **value,
size_t *len);
+ int (*inode_init_security_anon)(struct inode *inode,
+ const struct qstr *name,
+ const struct file_operations *fops,
+ const struct inode *context_inode);
int (*inode_create)(struct inode *dir, struct dentry *dentry,
umode_t mode);
int (*inode_link)(struct dentry *old_dentry, struct inode *dir,
@@ -1884,6 +1892,7 @@ struct security_hook_heads {
struct hlist_head inode_alloc_security;
struct hlist_head inode_free_security;
struct hlist_head inode_init_security;
+ struct hlist_head inode_init_security_anon;
struct hlist_head inode_create;
struct hlist_head inode_link;
struct hlist_head inode_unlink;
diff --git a/include/linux/security.h b/include/linux/security.h
index 64b19f050343..8ea76af0be7a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -320,6 +320,10 @@ void security_inode_free(struct inode *inode);
int security_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr,
initxattrs initxattrs, void *fs_data);
+int security_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct file_operations *fops,
+ const struct inode *context_inode);
int security_old_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr, const char **name,
void **value, size_t *len);
diff --git a/security/security.c b/security/security.c
index 565bc9b67276..d06f3969c030 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1033,6 +1033,16 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
}
EXPORT_SYMBOL(security_inode_init_security);
+int
+security_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct file_operations *fops,
+ const struct inode *context_inode)
+{
+ return call_int_hook(inode_init_security_anon, 0, inode, name,
+ fops, context_inode);
+}
+
int security_old_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr, const char **name,
void **value, size_t *len)
--
2.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH 2/3] Teach SELinux about anonymous inodes
2020-02-14 3:26 ` [PATCH 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
2020-02-14 3:26 ` [PATCH 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
@ 2020-02-14 3:26 ` Daniel Colascione
2020-02-14 16:39 ` Stephen Smalley
2020-02-14 3:26 ` [PATCH 3/3] Wire UFFD up to SELinux Daniel Colascione
` (5 subsequent siblings)
7 siblings, 1 reply; 60+ messages in thread
From: Daniel Colascione @ 2020-02-14 3:26 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra
Cc: Daniel Colascione
This change uses the anon_inodes and LSM infrastructure introduced in
the previous patch to give SELinux the ability to control
anonymous-inode files that are created using the new _secure()
anon_inodes functions.
A SELinux policy author detects and controls these anonymous inodes by
adding a name-based type_transition rule that assigns a new security
type to anonymous-inode files created in some domain. The name used
for the name-based transition is the name associated with the
anonymous inode for file listings --- e.g., "[userfaultfd]" or
"[perf_event]".
Example:
type uffd_t;
type_transition sysadm_t sysadm_t : file uffd_t "[userfaultfd]";
allow sysadm_t uffd_t:file { create };
(The next patch in this series is necessary for making userfaultfd
support this new interface. The example above is just
for exposition.)
Signed-off-by: Daniel Colascione <dancol@google.com>
---
security/selinux/hooks.c | 57 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 1659b59fb5d7..6de0892620b3 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2915,6 +2915,62 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
return 0;
}
+static int selinux_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct file_operations *fops,
+ const struct inode *context_inode)
+{
+ const struct task_security_struct *tsec = selinux_cred(current_cred());
+ struct common_audit_data ad;
+ struct inode_security_struct *isec;
+ int rc;
+
+ if (unlikely(IS_PRIVATE(inode)))
+ return 0;
+
+ if (unlikely(!selinux_state.initialized))
+ return 0;
+
+ isec = selinux_inode(inode);
+
+ /*
+ * We only get here once per ephemeral inode. The inode has
+ * been initialized via inode_alloc_security but is otherwise
+ * untouched.
+ */
+
+ if (context_inode) {
+ struct inode_security_struct *context_isec =
+ selinux_inode(context_inode);
+ if (IS_ERR(context_isec))
+ return PTR_ERR(context_isec);
+ isec->sid = context_isec->sid;
+ } else {
+ rc = security_transition_sid(
+ &selinux_state, tsec->sid, tsec->sid,
+ SECCLASS_FILE, name, &isec->sid);
+ if (rc)
+ return rc;
+ }
+
+ isec->initialized = LABEL_INITIALIZED;
+
+ /*
+ * Now that we've initialized security, check whether we're
+ * allowed to actually create this type of anonymous inode.
+ */
+
+ ad.type = LSM_AUDIT_DATA_INODE;
+ ad.u.inode = inode;
+
+ return avc_has_perm(&selinux_state,
+ tsec->sid,
+ isec->sid,
+ isec->sclass,
+ FILE__CREATE,
+ &ad);
+}
+
static int selinux_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode)
{
return may_create(dir, dentry, SECCLASS_FILE);
@@ -6923,6 +6979,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security),
LSM_HOOK_INIT(inode_init_security, selinux_inode_init_security),
+ LSM_HOOK_INIT(inode_init_security_anon, selinux_inode_init_security_anon),
LSM_HOOK_INIT(inode_create, selinux_inode_create),
LSM_HOOK_INIT(inode_link, selinux_inode_link),
LSM_HOOK_INIT(inode_unlink, selinux_inode_unlink),
--
2.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH 2/3] Teach SELinux about anonymous inodes
2020-02-14 3:26 ` [PATCH 2/3] Teach SELinux about anonymous inodes Daniel Colascione
@ 2020-02-14 16:39 ` Stephen Smalley
2020-02-14 17:21 ` Daniel Colascione
0 siblings, 1 reply; 60+ messages in thread
From: Stephen Smalley @ 2020-02-14 16:39 UTC (permalink / raw)
To: Daniel Colascione, timmurray, selinux, linux-security-module,
linux-fsdevel, linux-kernel, kvm, viro, paul, nnk, lokeshgidra
On 2/13/20 10:26 PM, Daniel Colascione wrote:
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 1659b59fb5d7..6de0892620b3 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2915,6 +2915,62 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> return 0;
> }
>
> +static int selinux_inode_init_security_anon(struct inode *inode,
> + const struct qstr *name,
> + const struct file_operations *fops,
> + const struct inode *context_inode)
> +{
> + const struct task_security_struct *tsec = selinux_cred(current_cred());
> + struct common_audit_data ad;
> + struct inode_security_struct *isec;
> + int rc;
> +
> + if (unlikely(IS_PRIVATE(inode)))
> + return 0;
This is not possible since the caller clears S_PRIVATE before calling
and it would be a bug to call the hook on an inode that was intended to
be private, so we shouldn't check it here.
> +
> + if (unlikely(!selinux_state.initialized))
> + return 0;
Are we doing this to defer initialization until selinux_complete_init()
- that's normally why we bail in the !initialized case? Not entirely
sure what will happen in such a situation since we won't have the
context_inode or the allocating task information at that time, so we
certainly won't get the same result - probably they would all be labeled
with whatever anon_inodefs is assigned via genfscon or
SECINITSID_UNLABELED by default. If we instead just drop this test and
proceed, we'll inherit the context inode SID if specified or we'll call
security_transition_sid(), which in the !initialized case will just
return the tsid i.e. tsec->sid, so it will be labeled with the creating
task SID (SECINITSID_KERNEL prior to initialization). Then the
avc_has_perm() call will pass because everything gets allowed until
initialization. So you could drop this check and userfaultfds created
before policy load would get the kernel SID, or you can keep it and they
will get the unlabeled SID. Preference?
> +
> + isec = selinux_inode(inode);
> +
> + /*
> + * We only get here once per ephemeral inode. The inode has
> + * been initialized via inode_alloc_security but is otherwise
> + * untouched.
> + */
> +
> + if (context_inode) {
> + struct inode_security_struct *context_isec =
> + selinux_inode(context_inode);
> + if (IS_ERR(context_isec))
> + return PTR_ERR(context_isec);
This isn't possible AFAICT so you don't need to test for it or handle
it. In fact, even the test for NULL in selinux_inode() is bogus and
should get dropped AFAICT; we always allocate an inode security blob
even before policy load so it would be a bug if we ever had a NULL there.
> + isec->sid = context_isec->sid;
> + } else {
> + rc = security_transition_sid(
> + &selinux_state, tsec->sid, tsec->sid,
> + SECCLASS_FILE, name, &isec->sid);
> + if (rc)
> + return rc;
> + }
Since you switched to using security_transition_sid(), you are not using
the fops parameter anymore nor comparing with userfaultfd_fops, so you
could drop the parameter from the hook and leave the latter static in
the first patch.
That's assuming you are ok with having to define these type_transition
rules for the userfaultfd case instead of having your own separate
security class. Wondering how many different anon inode names/classes
there are in the kernel today and how much they change over time; for a
small, relatively stable set, separate classes might be ok; for a large,
dynamic set, type transitions should scale better. We might still need
to create a mapping table in SELinux from the names to some stable
identifier for the policy lookup if we can't rely on the names being stable.
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 2/3] Teach SELinux about anonymous inodes
2020-02-14 16:39 ` Stephen Smalley
@ 2020-02-14 17:21 ` Daniel Colascione
2020-02-14 18:02 ` Stephen Smalley
0 siblings, 1 reply; 60+ messages in thread
From: Daniel Colascione @ 2020-02-14 17:21 UTC (permalink / raw)
To: Stephen Smalley
Cc: Tim Murray, SElinux list, LSM List, Linux FS Devel, linux-kernel,
kvm, Al Viro, paul, Nick Kralevich, Lokesh Gidra
On Fri, Feb 14, 2020 at 8:38 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
> On 2/13/20 10:26 PM, Daniel Colascione wrote:
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 1659b59fb5d7..6de0892620b3 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -2915,6 +2915,62 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> > return 0;
> > }
> >
> > +static int selinux_inode_init_security_anon(struct inode *inode,
> > + const struct qstr *name,
> > + const struct file_operations *fops,
> > + const struct inode *context_inode)
> > +{
> > + const struct task_security_struct *tsec = selinux_cred(current_cred());
> > + struct common_audit_data ad;
> > + struct inode_security_struct *isec;
> > + int rc;
> > +
> > + if (unlikely(IS_PRIVATE(inode)))
> > + return 0;
>
> This is not possible since the caller clears S_PRIVATE before calling
> and it would be a bug to call the hook on an inode that was intended to
> be private, so we shouldn't check it here.
>
> > +
> > + if (unlikely(!selinux_state.initialized))
> > + return 0;
>
> Are we doing this to defer initialization until selinux_complete_init()
> - that's normally why we bail in the !initialized case? Not entirely
> sure what will happen in such a situation since we won't have the
> context_inode or the allocating task information at that time, so we
> certainly won't get the same result - probably they would all be labeled
> with whatever anon_inodefs is assigned via genfscon or
> SECINITSID_UNLABELED by default.
> If we instead just drop this test and
> proceed, we'll inherit the context inode SID if specified or we'll call
> security_transition_sid(), which in the !initialized case will just
> return the tsid i.e. tsec->sid, so it will be labeled with the creating
> task SID (SECINITSID_KERNEL prior to initialization). Then the
> avc_has_perm() call will pass because everything gets allowed until
> initialization. So you could drop this check and userfaultfds created
> before policy load would get the kernel SID, or you can keep it and they
> will get the unlabeled SID. Preference?
The kernel SID seems safer. Thanks for the explanation!
> > +
> > + isec = selinux_inode(inode);
> > +
> > + /*
> > + * We only get here once per ephemeral inode. The inode has
> > + * been initialized via inode_alloc_security but is otherwise
> > + * untouched.
> > + */
> > +
> > + if (context_inode) {
> > + struct inode_security_struct *context_isec =
> > + selinux_inode(context_inode);
> > + if (IS_ERR(context_isec))
> > + return PTR_ERR(context_isec);
>
> This isn't possible AFAICT so you don't need to test for it or handle
> it. In fact, even the test for NULL in selinux_inode() is bogus and
> should get dropped AFAICT; we always allocate an inode security blob
> even before policy load so it would be a bug if we ever had a NULL there.
Thanks. Will fix.
> > + isec->sid = context_isec->sid;
> > + } else {
> > + rc = security_transition_sid(
> > + &selinux_state, tsec->sid, tsec->sid,
> > + SECCLASS_FILE, name, &isec->sid);
> > + if (rc)
> > + return rc;
> > + }
>
> Since you switched to using security_transition_sid(), you are not using
> the fops parameter anymore nor comparing with userfaultfd_fops, so you
> could drop the parameter from the hook and leave the latter static in
> the first patch.
That's true, but I figured different LSMs might want different rules
that depend on the fops. I'm also okay with removing this parameter
for now, since we're not using it.
> That's assuming you are ok with having to define these type_transition
> rules for the userfaultfd case instead of having your own separate
> security class. Wondering how many different anon inode names/classes
> there are in the kernel today and how much they change over time; for a
> small, relatively stable set, separate classes might be ok; for a large,
> dynamic set, type transitions should scale better.
I think we can get away without a class per anonymous-inode-type. I do
wonder whether we need a class for all anonymous inodes, though: if we
just give them the file class and use the anon inode type name for the
type_transition rule, isn't it possible that the type_transition rule
might also fire for plain files with the same names in the last path
component and the same originating sid? (Maybe I'm not understanding
type_transition rules properly.) Using a class to encompass all
anonymous inodes would address this problem (assuming the problem
exists in the first place).
> We might still need
> to create a mapping table in SELinux from the names to some stable
> identifier for the policy lookup if we can't rely on the names being stable.
Sure. The anonymous inode type names have historically been stable,
though, so maybe we can just use the names from anon_inodes directly
for now and then add some kind of remapping if we want to change those
names in the core, remaping to the old name for SELinux
type_transition purposes.
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 2/3] Teach SELinux about anonymous inodes
2020-02-14 17:21 ` Daniel Colascione
@ 2020-02-14 18:02 ` Stephen Smalley
2020-02-14 18:08 ` Stephen Smalley
0 siblings, 1 reply; 60+ messages in thread
From: Stephen Smalley @ 2020-02-14 18:02 UTC (permalink / raw)
To: Daniel Colascione
Cc: Tim Murray, SElinux list, LSM List, Linux FS Devel, linux-kernel,
kvm, Al Viro, paul, Nick Kralevich, Lokesh Gidra
On 2/14/20 12:21 PM, Daniel Colascione wrote:
> On Fri, Feb 14, 2020 at 8:38 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>> That's assuming you are ok with having to define these type_transition
>> rules for the userfaultfd case instead of having your own separate
>> security class. Wondering how many different anon inode names/classes
>> there are in the kernel today and how much they change over time; for a
>> small, relatively stable set, separate classes might be ok; for a large,
>> dynamic set, type transitions should scale better.
>
> I think we can get away without a class per anonymous-inode-type. I do
> wonder whether we need a class for all anonymous inodes, though: if we
> just give them the file class and use the anon inode type name for the
> type_transition rule, isn't it possible that the type_transition rule
> might also fire for plain files with the same names in the last path
> component and the same originating sid? (Maybe I'm not understanding
> type_transition rules properly.) Using a class to encompass all
> anonymous inodes would address this problem (assuming the problem
> exists in the first place).
It shouldn't fire for non-anon inodes because on a (non-anon) file
creation, security_transition_sid() is passed the parent directory SID
as the second argument and we only assign task SIDs to /proc/pid
directories, which don't support (userspace) file creation anyway.
However, in the absence of a matching type_transition rule, we'll end up
defaulting to the task SID on the anon inode, and without a separate
class we won't be able to distinguish it from a /proc/pid inode. So
that might justify a separate anoninode or similar class.
This however reminded me that for the context_inode case, we not only
want to inherit the SID but also the sclass from the context_inode.
That is so that anon inodes created via device node ioctls inherit the
same SID/class pair as the device node and a single allowx rule can
govern all ioctl commands on that device.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 2/3] Teach SELinux about anonymous inodes
2020-02-14 18:02 ` Stephen Smalley
@ 2020-02-14 18:08 ` Stephen Smalley
2020-02-14 20:24 ` Stephen Smalley
0 siblings, 1 reply; 60+ messages in thread
From: Stephen Smalley @ 2020-02-14 18:08 UTC (permalink / raw)
To: Daniel Colascione
Cc: Tim Murray, SElinux list, LSM List, Linux FS Devel, linux-kernel,
kvm, Al Viro, paul, Nick Kralevich, Lokesh Gidra,
Jeffrey Vander Stoep
On 2/14/20 1:02 PM, Stephen Smalley wrote:
> It shouldn't fire for non-anon inodes because on a (non-anon) file
> creation, security_transition_sid() is passed the parent directory SID
> as the second argument and we only assign task SIDs to /proc/pid
> directories, which don't support (userspace) file creation anyway.
>
> However, in the absence of a matching type_transition rule, we'll end up
> defaulting to the task SID on the anon inode, and without a separate
> class we won't be able to distinguish it from a /proc/pid inode. So
> that might justify a separate anoninode or similar class.
>
> This however reminded me that for the context_inode case, we not only
> want to inherit the SID but also the sclass from the context_inode. That
> is so that anon inodes created via device node ioctls inherit the same
> SID/class pair as the device node and a single allowx rule can govern
> all ioctl commands on that device.
At least that's the way our patch worked with the /dev/kvm example.
However, if we are introducing a separate anoninode class for the
type_transition case, maybe we should apply that to all anon inodes
regardless of how they are labeled (based on context_inode or
transition) and then we'd need to write two allowx rules, one for ioctls
on the original device node and one for those on anon inodes created
from it. Not sure how Android wants to handle that as the original
developer and primary user of SELinux ioctl whitelisting.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 2/3] Teach SELinux about anonymous inodes
2020-02-14 18:08 ` Stephen Smalley
@ 2020-02-14 20:24 ` Stephen Smalley
0 siblings, 0 replies; 60+ messages in thread
From: Stephen Smalley @ 2020-02-14 20:24 UTC (permalink / raw)
To: Daniel Colascione
Cc: Tim Murray, SElinux list, LSM List, Linux FS Devel, linux-kernel,
kvm, Al Viro, paul, Nick Kralevich, Lokesh Gidra,
Jeffrey Vander Stoep
On 2/14/20 1:08 PM, Stephen Smalley wrote:
> On 2/14/20 1:02 PM, Stephen Smalley wrote:
>> It shouldn't fire for non-anon inodes because on a (non-anon) file
>> creation, security_transition_sid() is passed the parent directory SID
>> as the second argument and we only assign task SIDs to /proc/pid
>> directories, which don't support (userspace) file creation anyway.
>>
>> However, in the absence of a matching type_transition rule, we'll end
>> up defaulting to the task SID on the anon inode, and without a
>> separate class we won't be able to distinguish it from a /proc/pid
>> inode. So that might justify a separate anoninode or similar class.
>>
>> This however reminded me that for the context_inode case, we not only
>> want to inherit the SID but also the sclass from the context_inode.
>> That is so that anon inodes created via device node ioctls inherit the
>> same SID/class pair as the device node and a single allowx rule can
>> govern all ioctl commands on that device.
>
> At least that's the way our patch worked with the /dev/kvm example.
> However, if we are introducing a separate anoninode class for the
> type_transition case, maybe we should apply that to all anon inodes
> regardless of how they are labeled (based on context_inode or
> transition) and then we'd need to write two allowx rules, one for ioctls
> on the original device node and one for those on anon inodes created
> from it. Not sure how Android wants to handle that as the original
> developer and primary user of SELinux ioctl whitelisting.
I would tentatively argue for inheriting both sclass and SID from the
context_inode for the sake of sane policy writing. In the userfaultfd
case, that will still end up using the new SECCLASS_ANONINODE or
whatever since the sclass will be initially set to that value for the
transition SID case and then inherited on fork. But for /dev/kvm, it
would be the class from the /dev/kvm inode, i.e. SECCLASS_CHR_FILE.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 3/3] Wire UFFD up to SELinux
2020-02-14 3:26 ` [PATCH 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
2020-02-14 3:26 ` [PATCH 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
2020-02-14 3:26 ` [PATCH 2/3] Teach SELinux about anonymous inodes Daniel Colascione
@ 2020-02-14 3:26 ` Daniel Colascione
2020-03-25 23:02 ` [PATCH v2 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
` (4 subsequent siblings)
7 siblings, 0 replies; 60+ messages in thread
From: Daniel Colascione @ 2020-02-14 3:26 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra
Cc: Daniel Colascione
This change gives userfaultfd file descriptors a real security
context, allowing policy to act on them.
Signed-off-by: Daniel Colascione <dancol@google.com>
---
fs/userfaultfd.c | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 07b0f6e03849..06e92697aba4 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -76,6 +76,8 @@ struct userfaultfd_ctx {
bool mmap_changing;
/* mm with one ore more vmas attached to this userfaultfd_ctx */
struct mm_struct *mm;
+ /* The inode that owns this context --- not a strong reference. */
+ const struct inode *owner;
};
struct userfaultfd_fork_ctx {
@@ -1020,8 +1022,10 @@ static int resolve_userfault_fork(struct userfaultfd_ctx *ctx,
{
int fd;
- fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, new,
- O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS));
+ fd = anon_inode_getfd_secure(
+ "[userfaultfd]", &userfaultfd_fops, new,
+ O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS),
+ ctx->owner);
if (fd < 0)
return fd;
@@ -1943,6 +1947,7 @@ static void init_once_userfaultfd_ctx(void *mem)
SYSCALL_DEFINE1(userfaultfd, int, flags)
{
+ struct file *file;
struct userfaultfd_ctx *ctx;
int fd;
@@ -1972,8 +1977,25 @@ SYSCALL_DEFINE1(userfaultfd, int, flags)
/* prevent the mm struct to be freed */
mmgrab(ctx->mm);
- fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, ctx,
- O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS));
+ file = anon_inode_getfile_secure(
+ "[userfaultfd]", &userfaultfd_fops, ctx,
+ O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS),
+ NULL);
+ if (IS_ERR(file)) {
+ fd = PTR_ERR(file);
+ goto out;
+ }
+
+ fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
+ if (fd < 0) {
+ fput(file);
+ goto out;
+ }
+
+ ctx->owner = file_inode(file);
+ fd_install(fd, file);
+
+out:
if (fd < 0) {
mmdrop(ctx->mm);
kmem_cache_free(userfaultfd_ctx_cachep, ctx);
--
2.25.0.265.gbab2e86ba0-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v2 0/3] SELinux support for anonymous inodes and UFFD
2020-02-14 3:26 ` [PATCH 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
` (2 preceding siblings ...)
2020-02-14 3:26 ` [PATCH 3/3] Wire UFFD up to SELinux Daniel Colascione
@ 2020-03-25 23:02 ` Daniel Colascione
2020-03-25 23:02 ` [PATCH v2 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
` (3 subsequent siblings)
7 siblings, 0 replies; 60+ messages in thread
From: Daniel Colascione @ 2020-03-25 23:02 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra
Cc: Daniel Colascione
Userfaultfd in unprivileged contexts could be potentially very
useful. We'd like to harden userfaultfd to make such unprivileged use
less risky. This patch series allows SELinux to manage userfaultfd
file descriptors and in the future, other kinds of
anonymous-inode-based file descriptor. SELinux policy authors can
apply policy types to anonymous inodes by providing name-based
transition rules keyed off the anonymous inode internal name (
"[userfaultfd]" in the case of userfaultfd(2) file descriptors) and
applying policy to the new SIDs thus produced.
Inside the kernel, a pair of new anon_inodes interface,
anon_inode_getfile_secure and anon_inode_getfd_secure, allow callers
to opt into this SELinux management. In this new "secure" mode,
anon_inodes creates new ephemeral inodes for anonymous file objects
instead of reusing the normal anon_inodes singleton dummy inode. A new
LSM hook gives security modules an opportunity to configure and veto
these ephemeral inodes.
This patch series is one of two fork of [1] and is an
alternative to [2].
The primary difference between the two patch series is that this
partch series creates a unique inode for each "secure" anonymous
inode, while the other patch series ([2]) continues using the
singleton dummy anonymous inode and adds a way to attach SELinux
security information directly to file objects.
I prefer the approach in this patch series because 1) it's a smaller
patch than [2], and 2) it produces a more regular security
architecture: in this patch series, secure anonymous inodes aren't
S_PRIVATE and they maintain the SELinux property that the label for a
file is in its inode. We do need an additional inode per anonymous
file, but per-struct-file inode creation doesn't seem to be a problem
for pipes and sockets.
The previous version of this feature ([1]) created a new SELinux
security class for userfaultfd file descriptors. This version adopts
the generic transition-based approach of [2].
This patch series also differs from [2] in that it doesn't affect all
anonymous inodes right away --- instead requiring anon_inodes callers
to opt in --- but this difference isn't one of basic approach. The
important question to resolve is whether we should be creating new
inodes or enhancing per-file data.
Changes from the first version of the patch:
- Removed some error checks
- Defined a new anon_inode SELinux class to resolve the
ambiguity in [3]
- Inherit sclass as well as descriptor from context inode
[1] https://lore.kernel.org/lkml/20200211225547.235083-1-dancol@google.com/
[2] https://lore.kernel.org/linux-fsdevel/20200213194157.5877-1-sds@tycho.nsa.gov/
[3] https://lore.kernel.org/lkml/23f725ca-5b5a-5938-fcc8-5bbbfc9ba9bc@tycho.nsa.gov/
Daniel Colascione (3):
Add a new LSM-supporting anonymous inode interface
Teach SELinux about anonymous inodes
Wire UFFD up to SELinux
fs/anon_inodes.c | 196 ++++++++++++++++++++++------
fs/userfaultfd.c | 30 ++++-
include/linux/anon_inodes.h | 13 ++
include/linux/lsm_hooks.h | 9 ++
include/linux/security.h | 4 +
security/security.c | 10 ++
security/selinux/hooks.c | 54 ++++++++
security/selinux/include/classmap.h | 2 +
8 files changed, 272 insertions(+), 46 deletions(-)
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH v2 1/3] Add a new LSM-supporting anonymous inode interface
2020-02-14 3:26 ` [PATCH 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
` (3 preceding siblings ...)
2020-03-25 23:02 ` [PATCH v2 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
@ 2020-03-25 23:02 ` Daniel Colascione
2020-03-26 13:53 ` Stephen Smalley
2020-03-25 23:02 ` [PATCH v2 2/3] Teach SELinux about anonymous inodes Daniel Colascione
` (2 subsequent siblings)
7 siblings, 1 reply; 60+ messages in thread
From: Daniel Colascione @ 2020-03-25 23:02 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra
Cc: Daniel Colascione
This change adds two new functions, anon_inode_getfile_secure and
anon_inode_getfd_secure, that create anonymous-node files with
individual non-S_PRIVATE inodes to which security modules can apply
policy. Existing callers continue using the original singleton-inode
kind of anonymous-inode file. We can transition anonymous inode users
to the new kind of anonymous inode in individual patches for the sake
of bisection and review.
The new functions accept an optional context_inode parameter that
callers can use to provide additional contextual information to
security modules, e.g., indicating that one anonymous struct file is a
logical child of another, allowing a security model to propagate
security information from one to the other.
Signed-off-by: Daniel Colascione <dancol@google.com>
---
fs/anon_inodes.c | 196 ++++++++++++++++++++++++++++--------
fs/userfaultfd.c | 4 +-
include/linux/anon_inodes.h | 13 +++
include/linux/lsm_hooks.h | 9 ++
include/linux/security.h | 4 +
security/security.c | 10 ++
6 files changed, 191 insertions(+), 45 deletions(-)
diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 89714308c25b..114a04fc1db4 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -55,75 +55,135 @@ static struct file_system_type anon_inode_fs_type = {
.kill_sb = kill_anon_super,
};
-/**
- * anon_inode_getfile - creates a new file instance by hooking it up to an
- * anonymous inode, and a dentry that describe the "class"
- * of the file
- *
- * @name: [in] name of the "class" of the new file
- * @fops: [in] file operations for the new file
- * @priv: [in] private data for the new file (will be file's private_data)
- * @flags: [in] flags
- *
- * Creates a new file by hooking it on a single inode. This is useful for files
- * that do not need to have a full-fledged inode in order to operate correctly.
- * All the files created with anon_inode_getfile() will share a single inode,
- * hence saving memory and avoiding code duplication for the file/inode/dentry
- * setup. Returns the newly created file* or an error pointer.
- */
-struct file *anon_inode_getfile(const char *name,
- const struct file_operations *fops,
- void *priv, int flags)
+static struct inode *anon_inode_make_secure_inode(
+ const char *name,
+ const struct inode *context_inode,
+ const struct file_operations *fops)
+{
+ struct inode *inode;
+ const struct qstr qname = QSTR_INIT(name, strlen(name));
+ int error;
+
+ inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
+ if (IS_ERR(inode))
+ return ERR_PTR(PTR_ERR(inode));
+ inode->i_flags &= ~S_PRIVATE;
+ error = security_inode_init_security_anon(
+ inode, &qname, fops, context_inode);
+ if (error) {
+ iput(inode);
+ return ERR_PTR(error);
+ }
+ return inode;
+}
+
+struct file *_anon_inode_getfile(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode,
+ bool secure)
{
+ struct inode *inode;
struct file *file;
- if (IS_ERR(anon_inode_inode))
- return ERR_PTR(-ENODEV);
+ if (secure) {
+ inode = anon_inode_make_secure_inode(
+ name, context_inode, fops);
+ if (IS_ERR(inode))
+ return ERR_PTR(PTR_ERR(inode));
+ } else {
+ inode = anon_inode_inode;
+ if (IS_ERR(inode))
+ return ERR_PTR(-ENODEV);
+ /*
+ * We know the anon_inode inode count is always
+ * greater than zero, so ihold() is safe.
+ */
+ ihold(inode);
+ }
- if (fops->owner && !try_module_get(fops->owner))
- return ERR_PTR(-ENOENT);
+ if (fops->owner && !try_module_get(fops->owner)) {
+ file = ERR_PTR(-ENOENT);
+ goto err;
+ }
- /*
- * We know the anon_inode inode count is always greater than zero,
- * so ihold() is safe.
- */
- ihold(anon_inode_inode);
- file = alloc_file_pseudo(anon_inode_inode, anon_inode_mnt, name,
+ file = alloc_file_pseudo(inode, anon_inode_mnt, name,
flags & (O_ACCMODE | O_NONBLOCK), fops);
if (IS_ERR(file))
goto err;
- file->f_mapping = anon_inode_inode->i_mapping;
+ file->f_mapping = inode->i_mapping;
file->private_data = priv;
return file;
err:
- iput(anon_inode_inode);
+ iput(inode);
module_put(fops->owner);
return file;
}
-EXPORT_SYMBOL_GPL(anon_inode_getfile);
/**
- * anon_inode_getfd - creates a new file instance by hooking it up to an
- * anonymous inode, and a dentry that describe the "class"
- * of the file
+ * anon_inode_getfile_secure - creates a new file instance by hooking
+ * it up to a new anonymous inode and a
+ * dentry that describe the "class" of the
+ * file. Make it possible to use security
+ * modules to control access to the
+ * new file.
*
* @name: [in] name of the "class" of the new file
* @fops: [in] file operations for the new file
* @priv: [in] private data for the new file (will be file's private_data)
- * @flags: [in] flags
+ * @flags: [in] flags for the file
+ * @anon_inode_flags: [in] flags for anon_inode*
+ *
+ * Creates a new file by hooking it on an unspecified inode. This is
+ * useful for files that do not need to have a full-fledged inode in
+ * order to operate correctly. All the files created with
+ * anon_inode_getfile_secure() will have distinct inodes, avoiding
+ * code duplication for the file/inode/dentry setup. Returns the
+ * newly created file* or an error pointer.
+ */
+struct file *anon_inode_getfile_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode)
+{
+ return _anon_inode_getfile(
+ name, fops, priv, flags, context_inode, true);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfile_secure);
+
+/**
+ * anon_inode_getfile - creates a new file instance by hooking it up
+ * to an anonymous inode and a dentry that
+ * describe the "class" of the file.
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags for the file
*
- * Creates a new file by hooking it on a single inode. This is useful for files
+ * Creates a new file by hooking it on an unspecified inode. This is useful for files
* that do not need to have a full-fledged inode in order to operate correctly.
- * All the files created with anon_inode_getfd() will share a single inode,
+ * All the files created with anon_inode_getfile() will share a single inode,
* hence saving memory and avoiding code duplication for the file/inode/dentry
- * setup. Returns new descriptor or an error code.
+ * setup. Returns the newly created file* or an error pointer.
*/
-int anon_inode_getfd(const char *name, const struct file_operations *fops,
- void *priv, int flags)
+struct file *anon_inode_getfile(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags)
+{
+ return _anon_inode_getfile(name, fops, priv, flags, NULL, false);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfile);
+
+static int _anon_inode_getfd(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode,
+ bool secure)
{
int error, fd;
struct file *file;
@@ -133,7 +193,8 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops,
return error;
fd = error;
- file = anon_inode_getfile(name, fops, priv, flags);
+ file = _anon_inode_getfile(name, fops, priv, flags, context_inode,
+ secure);
if (IS_ERR(file)) {
error = PTR_ERR(file);
goto err_put_unused_fd;
@@ -146,6 +207,57 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops,
put_unused_fd(fd);
return error;
}
+
+/**
+ * anon_inode_getfd_secure - creates a new file instance by hooking it
+ * up to a new anonymous inode and a dentry
+ * that describe the "class" of the file.
+ * Make it possible to use security modules
+ * to control access to the new file.
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags
+ *
+ * Creates a new file by hooking it on an unspecified inode. This is
+ * useful for files that do not need to have a full-fledged inode in
+ * order to operate correctly. All the files created with
+ * anon_inode_getfile_secure() will have distinct inodes, avoiding
+ * code duplication for the file/inode/dentry setup. Returns a newly
+ * created file descriptor or an error code.
+ */
+int anon_inode_getfd_secure(const char *name, const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode)
+{
+ return _anon_inode_getfd(name, fops, priv, flags,
+ context_inode, true);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfd_secure);
+
+/**
+ * anon_inode_getfd - creates a new file instance by hooking it up to
+ * an anonymous inode and a dentry that describe
+ * the "class" of the file
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags
+ *
+ * Creates a new file by hooking it on an unspecified inode. This is
+ * useful for files that do not need to have a full-fledged inode in
+ * order to operate correctly. All the files created with
+ * anon_inode_getfile() will use the same singleton inode, reducing
+ * memory use and avoiding code duplication for the file/inode/dentry
+ * setup. Returns a newly created file descriptor or an error code.
+ */
+int anon_inode_getfd(const char *name, const struct file_operations *fops,
+ void *priv, int flags)
+{
+ return _anon_inode_getfd(name, fops, priv, flags, NULL, false);
+}
EXPORT_SYMBOL_GPL(anon_inode_getfd);
static int __init anon_inode_init(void)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 37df7c9eedb1..07b0f6e03849 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1014,8 +1014,6 @@ static __poll_t userfaultfd_poll(struct file *file, poll_table *wait)
}
}
-static const struct file_operations userfaultfd_fops;
-
static int resolve_userfault_fork(struct userfaultfd_ctx *ctx,
struct userfaultfd_ctx *new,
struct uffd_msg *msg)
@@ -1920,7 +1918,7 @@ static void userfaultfd_show_fdinfo(struct seq_file *m, struct file *f)
}
#endif
-static const struct file_operations userfaultfd_fops = {
+const struct file_operations userfaultfd_fops = {
#ifdef CONFIG_PROC_FS
.show_fdinfo = userfaultfd_show_fdinfo,
#endif
diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
index d0d7d96261ad..67bd85d92dca 100644
--- a/include/linux/anon_inodes.h
+++ b/include/linux/anon_inodes.h
@@ -10,12 +10,25 @@
#define _LINUX_ANON_INODES_H
struct file_operations;
+struct inode;
+
+struct file *anon_inode_getfile_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode);
struct file *anon_inode_getfile(const char *name,
const struct file_operations *fops,
void *priv, int flags);
+
+int anon_inode_getfd_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode);
+
int anon_inode_getfd(const char *name, const struct file_operations *fops,
void *priv, int flags);
+
#endif /* _LINUX_ANON_INODES_H */
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 20d8cf194fb7..de5d37e388df 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -215,6 +215,10 @@
* Returns 0 if @name and @value have been successfully set,
* -EOPNOTSUPP if no security attribute is needed, or
* -ENOMEM on memory allocation failure.
+ * @inode_init_security_anon:
+ * Set up a secure anonymous inode.
+ * Returns 0 on success. Returns -EPERM if the security module denies
+ * the creation of this inode.
* @inode_create:
* Check permission to create a regular file.
* @dir contains inode structure of the parent of the new file.
@@ -1552,6 +1556,10 @@ union security_list_options {
const struct qstr *qstr,
const char **name, void **value,
size_t *len);
+ int (*inode_init_security_anon)(struct inode *inode,
+ const struct qstr *name,
+ const struct file_operations *fops,
+ const struct inode *context_inode);
int (*inode_create)(struct inode *dir, struct dentry *dentry,
umode_t mode);
int (*inode_link)(struct dentry *old_dentry, struct inode *dir,
@@ -1884,6 +1892,7 @@ struct security_hook_heads {
struct hlist_head inode_alloc_security;
struct hlist_head inode_free_security;
struct hlist_head inode_init_security;
+ struct hlist_head inode_init_security_anon;
struct hlist_head inode_create;
struct hlist_head inode_link;
struct hlist_head inode_unlink;
diff --git a/include/linux/security.h b/include/linux/security.h
index 64b19f050343..8ea76af0be7a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -320,6 +320,10 @@ void security_inode_free(struct inode *inode);
int security_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr,
initxattrs initxattrs, void *fs_data);
+int security_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct file_operations *fops,
+ const struct inode *context_inode);
int security_old_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr, const char **name,
void **value, size_t *len);
diff --git a/security/security.c b/security/security.c
index 565bc9b67276..d06f3969c030 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1033,6 +1033,16 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
}
EXPORT_SYMBOL(security_inode_init_security);
+int
+security_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct file_operations *fops,
+ const struct inode *context_inode)
+{
+ return call_int_hook(inode_init_security_anon, 0, inode, name,
+ fops, context_inode);
+}
+
int security_old_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr, const char **name,
void **value, size_t *len)
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v2 1/3] Add a new LSM-supporting anonymous inode interface
2020-03-25 23:02 ` [PATCH v2 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
@ 2020-03-26 13:53 ` Stephen Smalley
0 siblings, 0 replies; 60+ messages in thread
From: Stephen Smalley @ 2020-03-26 13:53 UTC (permalink / raw)
To: Daniel Colascione, timmurray, selinux, linux-security-module,
linux-fsdevel, linux-kernel, kvm, viro, paul, nnk, lokeshgidra,
James Morris
On 3/25/20 7:02 PM, Daniel Colascione wrote:
> This change adds two new functions, anon_inode_getfile_secure and
> anon_inode_getfd_secure, that create anonymous-node files with
> individual non-S_PRIVATE inodes to which security modules can apply
> policy. Existing callers continue using the original singleton-inode
> kind of anonymous-inode file. We can transition anonymous inode users
> to the new kind of anonymous inode in individual patches for the sake
> of bisection and review.
>
> The new functions accept an optional context_inode parameter that
> callers can use to provide additional contextual information to
> security modules, e.g., indicating that one anonymous struct file is a
> logical child of another, allowing a security model to propagate
> security information from one to the other.
>
> Signed-off-by: Daniel Colascione <dancol@google.com>
> ---
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index 89714308c25b..114a04fc1db4 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -55,75 +55,135 @@ static struct file_system_type anon_inode_fs_type = {
> .kill_sb = kill_anon_super,
> };
>
> -/**
> - * anon_inode_getfile - creates a new file instance by hooking it up to an
> - * anonymous inode, and a dentry that describe the "class"
> - * of the file
> - *
> - * @name: [in] name of the "class" of the new file
> - * @fops: [in] file operations for the new file
> - * @priv: [in] private data for the new file (will be file's private_data)
> - * @flags: [in] flags
> - *
> - * Creates a new file by hooking it on a single inode. This is useful for files
> - * that do not need to have a full-fledged inode in order to operate correctly.
> - * All the files created with anon_inode_getfile() will share a single inode,
> - * hence saving memory and avoiding code duplication for the file/inode/dentry
> - * setup. Returns the newly created file* or an error pointer.
> - */
> -struct file *anon_inode_getfile(const char *name,
> - const struct file_operations *fops,
> - void *priv, int flags)
> +static struct inode *anon_inode_make_secure_inode(
> + const char *name,
> + const struct inode *context_inode,
> + const struct file_operations *fops)
> +{
> + struct inode *inode;
> + const struct qstr qname = QSTR_INIT(name, strlen(name));
> + int error;
> +
> + inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
> + if (IS_ERR(inode))
> + return ERR_PTR(PTR_ERR(inode));
Just return inode here?
> + inode->i_flags &= ~S_PRIVATE;
> + error = security_inode_init_security_anon(
> + inode, &qname, fops, context_inode);
Would drop fops argument to the security hook until we have an actual
user of it; one can always add it later.
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index 37df7c9eedb1..07b0f6e03849 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -1014,8 +1014,6 @@ static __poll_t userfaultfd_poll(struct file *file, poll_table *wait)
> }
> }
>
> -static const struct file_operations userfaultfd_fops;
> -
> static int resolve_userfault_fork(struct userfaultfd_ctx *ctx,
> struct userfaultfd_ctx *new,
> struct uffd_msg *msg)
> @@ -1920,7 +1918,7 @@ static void userfaultfd_show_fdinfo(struct seq_file *m, struct file *f)
> }
> #endif
>
> -static const struct file_operations userfaultfd_fops = {
> +const struct file_operations userfaultfd_fops = {
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = userfaultfd_show_fdinfo,
> #endif
These changes can be dropped entirely AFAICT.
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 20d8cf194fb7..de5d37e388df 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -215,6 +215,10 @@
> * Returns 0 if @name and @value have been successfully set,
> * -EOPNOTSUPP if no security attribute is needed, or
> * -ENOMEM on memory allocation failure.
> + * @inode_init_security_anon:
> + * Set up a secure anonymous inode.
> + * Returns 0 on success. Returns -EPERM if the security module denies
> + * the creation of this inode.
I'd favor describing the arguments (@name, @context_inode) too.
> * @inode_create:
> * Check permission to create a regular file.
> * @dir contains inode structure of the parent of the new file.
> @@ -1552,6 +1556,10 @@ union security_list_options {
> const struct qstr *qstr,
> const char **name, void **value,
> size_t *len);
> + int (*inode_init_security_anon)(struct inode *inode,
> + const struct qstr *name,
> + const struct file_operations *fops,
> + const struct inode *context_inode);
> int (*inode_create)(struct inode *dir, struct dentry *dentry,
Can drop the fops argument.
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 64b19f050343..8ea76af0be7a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -320,6 +320,10 @@ void security_inode_free(struct inode *inode);
> int security_inode_init_security(struct inode *inode, struct inode *dir,
> const struct qstr *qstr,
> initxattrs initxattrs, void *fs_data);
> +int security_inode_init_security_anon(struct inode *inode,
> + const struct qstr *name,
> + const struct file_operations *fops,
> + const struct inode *context_inode);
Ditto
> diff --git a/security/security.c b/security/security.c
> index 565bc9b67276..d06f3969c030 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1033,6 +1033,16 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> }
> EXPORT_SYMBOL(security_inode_init_security);
>
> +int
> +security_inode_init_security_anon(struct inode *inode,
> + const struct qstr *name,
> + const struct file_operations *fops,
> + const struct inode *context_inode)
> +{
> + return call_int_hook(inode_init_security_anon, 0, inode, name,
> + fops, context_inode);
> +}
> +
And again.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 2/3] Teach SELinux about anonymous inodes
2020-02-14 3:26 ` [PATCH 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
` (4 preceding siblings ...)
2020-03-25 23:02 ` [PATCH v2 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
@ 2020-03-25 23:02 ` Daniel Colascione
2020-03-26 13:58 ` Stephen Smalley
2020-03-26 17:37 ` Stephen Smalley
2020-03-25 23:02 ` [PATCH v2 3/3] Wire UFFD up to SELinux Daniel Colascione
2020-03-26 18:14 ` [PATCH v3 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
7 siblings, 2 replies; 60+ messages in thread
From: Daniel Colascione @ 2020-03-25 23:02 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra
Cc: Daniel Colascione
This change uses the anon_inodes and LSM infrastructure introduced in
the previous patch to give SELinux the ability to control
anonymous-inode files that are created using the new _secure()
anon_inodes functions.
A SELinux policy author detects and controls these anonymous inodes by
adding a name-based type_transition rule that assigns a new security
type to anonymous-inode files created in some domain. The name used
for the name-based transition is the name associated with the
anonymous inode for file listings --- e.g., "[userfaultfd]" or
"[perf_event]".
Example:
type uffd_t;
type_transition sysadm_t sysadm_t : file uffd_t "[userfaultfd]";
allow sysadm_t uffd_t:file { create };
(The next patch in this series is necessary for making userfaultfd
support this new interface. The example above is just
for exposition.)
Signed-off-by: Daniel Colascione <dancol@google.com>
---
security/selinux/hooks.c | 54 +++++++++++++++++++++++++++++
security/selinux/include/classmap.h | 2 ++
2 files changed, 56 insertions(+)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 1659b59fb5d7..b9eb45c2e4e5 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2915,6 +2915,59 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
return 0;
}
+static int selinux_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct file_operations *fops,
+ const struct inode *context_inode)
+{
+ const struct task_security_struct *tsec = selinux_cred(current_cred());
+ struct common_audit_data ad;
+ struct inode_security_struct *isec;
+ int rc;
+
+ if (unlikely(!selinux_state.initialized))
+ return 0;
+
+ isec = selinux_inode(inode);
+
+ /*
+ * We only get here once per ephemeral inode. The inode has
+ * been initialized via inode_alloc_security but is otherwise
+ * untouched.
+ */
+
+ if (context_inode) {
+ struct inode_security_struct *context_isec =
+ selinux_inode(context_inode);
+ isec->sclass = context_isec->sclass;
+ isec->sid = context_isec->sid;
+ } else {
+ isec->sclass = SECCLASS_ANON_INODE;
+ rc = security_transition_sid(
+ &selinux_state, tsec->sid, tsec->sid,
+ SECCLASS_FILE, name, &isec->sid);
+ if (rc)
+ return rc;
+ }
+
+ isec->initialized = LABEL_INITIALIZED;
+
+ /*
+ * Now that we've initialized security, check whether we're
+ * allowed to actually create this type of anonymous inode.
+ */
+
+ ad.type = LSM_AUDIT_DATA_INODE;
+ ad.u.inode = inode;
+
+ return avc_has_perm(&selinux_state,
+ tsec->sid,
+ isec->sid,
+ isec->sclass,
+ FILE__CREATE,
+ &ad);
+}
+
static int selinux_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode)
{
return may_create(dir, dentry, SECCLASS_FILE);
@@ -6923,6 +6976,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security),
LSM_HOOK_INIT(inode_init_security, selinux_inode_init_security),
+ LSM_HOOK_INIT(inode_init_security_anon, selinux_inode_init_security_anon),
LSM_HOOK_INIT(inode_create, selinux_inode_create),
LSM_HOOK_INIT(inode_link, selinux_inode_link),
LSM_HOOK_INIT(inode_unlink, selinux_inode_unlink),
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 986f3ac14282..263750b6aaac 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -248,6 +248,8 @@ struct security_class_mapping secclass_map[] = {
{"open", "cpu", "kernel", "tracepoint", "read", "write"} },
{ "lockdown",
{ "integrity", "confidentiality", NULL } },
+ { "anon_inode",
+ { COMMON_FILE_PERMS, NULL } },
{ NULL }
};
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v2 2/3] Teach SELinux about anonymous inodes
2020-03-25 23:02 ` [PATCH v2 2/3] Teach SELinux about anonymous inodes Daniel Colascione
@ 2020-03-26 13:58 ` Stephen Smalley
2020-03-26 17:59 ` Daniel Colascione
2020-03-26 17:37 ` Stephen Smalley
1 sibling, 1 reply; 60+ messages in thread
From: Stephen Smalley @ 2020-03-26 13:58 UTC (permalink / raw)
To: Daniel Colascione, timmurray, selinux, linux-security-module,
linux-fsdevel, linux-kernel, kvm, viro, paul, nnk, lokeshgidra
On 3/25/20 7:02 PM, Daniel Colascione wrote:
> This change uses the anon_inodes and LSM infrastructure introduced in
> the previous patch to give SELinux the ability to control
> anonymous-inode files that are created using the new _secure()
> anon_inodes functions.
>
> A SELinux policy author detects and controls these anonymous inodes by
> adding a name-based type_transition rule that assigns a new security
> type to anonymous-inode files created in some domain. The name used
> for the name-based transition is the name associated with the
> anonymous inode for file listings --- e.g., "[userfaultfd]" or
> "[perf_event]".
>
> Example:
>
> type uffd_t;
> type_transition sysadm_t sysadm_t : file uffd_t "[userfaultfd]";
> allow sysadm_t uffd_t:file { create };
>
> (The next patch in this series is necessary for making userfaultfd
> support this new interface. The example above is just
> for exposition.)
>
> Signed-off-by: Daniel Colascione <dancol@google.com>
> ---
> security/selinux/hooks.c | 54 +++++++++++++++++++++++++++++
> security/selinux/include/classmap.h | 2 ++
> 2 files changed, 56 insertions(+)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 1659b59fb5d7..b9eb45c2e4e5 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2915,6 +2915,59 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> return 0;
> }
>
> +static int selinux_inode_init_security_anon(struct inode *inode,
> + const struct qstr *name,
> + const struct file_operations *fops,
> + const struct inode *context_inode)
> +{
> + const struct task_security_struct *tsec = selinux_cred(current_cred());
> + struct common_audit_data ad;
> + struct inode_security_struct *isec;
> + int rc;
> +
> + if (unlikely(!selinux_state.initialized))
> + return 0;
This leaves secure anon inodes created before first policy load with the
unlabeled SID rather than defaulting to the SID of the creating task
(kernel SID in that situation). Is that what you want? Alternatively
you can just remove this test and let it proceed; nothing should be
break and the anon inodes will get the kernel SID.
> +
> + isec = selinux_inode(inode);
> +
> + /*
> + * We only get here once per ephemeral inode. The inode has
> + * been initialized via inode_alloc_security but is otherwise
> + * untouched.
> + */
> +
> + if (context_inode) {
> + struct inode_security_struct *context_isec =
> + selinux_inode(context_inode);
> + isec->sclass = context_isec->sclass;
> + isec->sid = context_isec->sid;
> + } else {
> + isec->sclass = SECCLASS_ANON_INODE;
> + rc = security_transition_sid(
> + &selinux_state, tsec->sid, tsec->sid,
> + SECCLASS_FILE, name, &isec->sid);
> + if (rc)
> + return rc;
> + }
> +
> + isec->initialized = LABEL_INITIALIZED;
> +
> + /*
> + * Now that we've initialized security, check whether we're
> + * allowed to actually create this type of anonymous inode.
> + */
> +
> + ad.type = LSM_AUDIT_DATA_INODE;
> + ad.u.inode = inode;
> +
> + return avc_has_perm(&selinux_state,
> + tsec->sid,
> + isec->sid,
> + isec->sclass,
> + FILE__CREATE,
> + &ad);
> +}
> +
> static int selinux_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode)
> {
> return may_create(dir, dentry, SECCLASS_FILE);
> @@ -6923,6 +6976,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>
> LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security),
> LSM_HOOK_INIT(inode_init_security, selinux_inode_init_security),
> + LSM_HOOK_INIT(inode_init_security_anon, selinux_inode_init_security_anon),
> LSM_HOOK_INIT(inode_create, selinux_inode_create),
> LSM_HOOK_INIT(inode_link, selinux_inode_link),
> LSM_HOOK_INIT(inode_unlink, selinux_inode_unlink),
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 986f3ac14282..263750b6aaac 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -248,6 +248,8 @@ struct security_class_mapping secclass_map[] = {
> {"open", "cpu", "kernel", "tracepoint", "read", "write"} },
> { "lockdown",
> { "integrity", "confidentiality", NULL } },
> + { "anon_inode",
> + { COMMON_FILE_PERMS, NULL } },
> { NULL }
> };
>
>
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH v2 2/3] Teach SELinux about anonymous inodes
2020-03-26 13:58 ` Stephen Smalley
@ 2020-03-26 17:59 ` Daniel Colascione
0 siblings, 0 replies; 60+ messages in thread
From: Daniel Colascione @ 2020-03-26 17:59 UTC (permalink / raw)
To: Stephen Smalley
Cc: Tim Murray, SElinux list, LSM List, Linux FS Devel, linux-kernel,
kvm, Al Viro, Paul Moore, Nick Kralevich, Lokesh Gidra
Thanks for taking a look!
On Thu, Mar 26, 2020 at 6:57 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
> On 3/25/20 7:02 PM, Daniel Colascione wrote:
> > This change uses the anon_inodes and LSM infrastructure introduced in
> > the previous patch to give SELinux the ability to control
> > anonymous-inode files that are created using the new _secure()
> > anon_inodes functions.
> >
> > A SELinux policy author detects and controls these anonymous inodes by
> > adding a name-based type_transition rule that assigns a new security
> > type to anonymous-inode files created in some domain. The name used
> > for the name-based transition is the name associated with the
> > anonymous inode for file listings --- e.g., "[userfaultfd]" or
> > "[perf_event]".
> >
> > Example:
> >
> > type uffd_t;
> > type_transition sysadm_t sysadm_t : file uffd_t "[userfaultfd]";
> > allow sysadm_t uffd_t:file { create };
Oops. Will fix.
> > (The next patch in this series is necessary for making userfaultfd
> > support this new interface. The example above is just
> > for exposition.)
> >
> > Signed-off-by: Daniel Colascione <dancol@google.com>
> > ---
> > security/selinux/hooks.c | 54 +++++++++++++++++++++++++++++
> > security/selinux/include/classmap.h | 2 ++
> > 2 files changed, 56 insertions(+)
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 1659b59fb5d7..b9eb45c2e4e5 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -2915,6 +2915,59 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> > return 0;
> > }
> >
> > +static int selinux_inode_init_security_anon(struct inode *inode,
> > + const struct qstr *name,
> > + const struct file_operations *fops,
> > + const struct inode *context_inode)
> > +{
> > + const struct task_security_struct *tsec = selinux_cred(current_cred());
> > + struct common_audit_data ad;
> > + struct inode_security_struct *isec;
> > + int rc;
> > +
> > + if (unlikely(!selinux_state.initialized))
> > + return 0;
>
> This leaves secure anon inodes created before first policy load with the
> unlabeled SID rather than defaulting to the SID of the creating task
> (kernel SID in that situation). Is that what you want? Alternatively
> you can just remove this test and let it proceed; nothing should be
> break and the anon inodes will get the kernel SID.
We talked about this decision on the last thread [1], and I think you
mentioned that either the unlabeled or the kernel SID approach would
be defensible. Using the unlabeled SID seems more "honest" to me than
using the kernel SID: the unlabeled SID says "we don't know", while
using kernel SID would be making an affirmative claim that the
anonymous inode belongs to the kernel, and claim wouldn't be true.
That's why I'm leaning toward the unlabeled approach right now.
[1] https://lore.kernel.org/lkml/9ca03838-8686-0007-0971-ee63bf5031da@tycho.nsa.gov/
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 2/3] Teach SELinux about anonymous inodes
2020-03-25 23:02 ` [PATCH v2 2/3] Teach SELinux about anonymous inodes Daniel Colascione
2020-03-26 13:58 ` Stephen Smalley
@ 2020-03-26 17:37 ` Stephen Smalley
1 sibling, 0 replies; 60+ messages in thread
From: Stephen Smalley @ 2020-03-26 17:37 UTC (permalink / raw)
To: Daniel Colascione, timmurray, selinux, linux-security-module,
linux-fsdevel, linux-kernel, kvm, viro, paul, nnk, lokeshgidra
On 3/25/20 7:02 PM, Daniel Colascione wrote:
> This change uses the anon_inodes and LSM infrastructure introduced in
> the previous patch to give SELinux the ability to control
> anonymous-inode files that are created using the new _secure()
> anon_inodes functions.
>
> A SELinux policy author detects and controls these anonymous inodes by
> adding a name-based type_transition rule that assigns a new security
> type to anonymous-inode files created in some domain. The name used
> for the name-based transition is the name associated with the
> anonymous inode for file listings --- e.g., "[userfaultfd]" or
> "[perf_event]".
>
> Example:
>
> type uffd_t;
> type_transition sysadm_t sysadm_t : file uffd_t "[userfaultfd]";
> allow sysadm_t uffd_t:file { create };
These should use :anon_inode rather than :file since the class is no
longer file.
>
> (The next patch in this series is necessary for making userfaultfd
> support this new interface. The example above is just
> for exposition.)
>
> Signed-off-by: Daniel Colascione <dancol@google.com>
> ---
> security/selinux/hooks.c | 54 +++++++++++++++++++++++++++++
> security/selinux/include/classmap.h | 2 ++
> 2 files changed, 56 insertions(+)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 1659b59fb5d7..b9eb45c2e4e5 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2915,6 +2915,59 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> return 0;
> }
>
> +static int selinux_inode_init_security_anon(struct inode *inode,
> + const struct qstr *name,
> + const struct file_operations *fops,
> + const struct inode *context_inode)
> +{
> + const struct task_security_struct *tsec = selinux_cred(current_cred());
> + struct common_audit_data ad;
> + struct inode_security_struct *isec;
> + int rc;
> +
> + if (unlikely(!selinux_state.initialized))
> + return 0;
> +
> + isec = selinux_inode(inode);
> +
> + /*
> + * We only get here once per ephemeral inode. The inode has
> + * been initialized via inode_alloc_security but is otherwise
> + * untouched.
> + */
> +
> + if (context_inode) {
> + struct inode_security_struct *context_isec =
> + selinux_inode(context_inode);
> + isec->sclass = context_isec->sclass;
> + isec->sid = context_isec->sid;
> + } else {
> + isec->sclass = SECCLASS_ANON_INODE;
> + rc = security_transition_sid(
> + &selinux_state, tsec->sid, tsec->sid,
> + SECCLASS_FILE, name, &isec->sid);
You should use isec->sclass == SECCLASS_ANON_INODE instead of
SECCLASS_FILE here.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 3/3] Wire UFFD up to SELinux
2020-02-14 3:26 ` [PATCH 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
` (5 preceding siblings ...)
2020-03-25 23:02 ` [PATCH v2 2/3] Teach SELinux about anonymous inodes Daniel Colascione
@ 2020-03-25 23:02 ` Daniel Colascione
2020-03-25 23:49 ` Casey Schaufler
2020-03-26 18:14 ` [PATCH v3 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
7 siblings, 1 reply; 60+ messages in thread
From: Daniel Colascione @ 2020-03-25 23:02 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra
Cc: Daniel Colascione
This change gives userfaultfd file descriptors a real security
context, allowing policy to act on them.
Signed-off-by: Daniel Colascione <dancol@google.com>
---
fs/userfaultfd.c | 34 +++++++++++++++++++++++++++++-----
1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 07b0f6e03849..78ff5d898733 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -76,6 +76,8 @@ struct userfaultfd_ctx {
bool mmap_changing;
/* mm with one ore more vmas attached to this userfaultfd_ctx */
struct mm_struct *mm;
+ /* The inode that owns this context --- not a strong reference. */
+ const struct inode *owner;
};
struct userfaultfd_fork_ctx {
@@ -1014,14 +1016,18 @@ static __poll_t userfaultfd_poll(struct file *file, poll_table *wait)
}
}
+static const struct file_operations userfaultfd_fops;
+
static int resolve_userfault_fork(struct userfaultfd_ctx *ctx,
struct userfaultfd_ctx *new,
struct uffd_msg *msg)
{
int fd;
- fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, new,
- O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS));
+ fd = anon_inode_getfd_secure(
+ "[userfaultfd]", &userfaultfd_fops, new,
+ O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS),
+ ctx->owner);
if (fd < 0)
return fd;
@@ -1918,7 +1924,7 @@ static void userfaultfd_show_fdinfo(struct seq_file *m, struct file *f)
}
#endif
-const struct file_operations userfaultfd_fops = {
+static const struct file_operations userfaultfd_fops = {
#ifdef CONFIG_PROC_FS
.show_fdinfo = userfaultfd_show_fdinfo,
#endif
@@ -1943,6 +1949,7 @@ static void init_once_userfaultfd_ctx(void *mem)
SYSCALL_DEFINE1(userfaultfd, int, flags)
{
+ struct file *file;
struct userfaultfd_ctx *ctx;
int fd;
@@ -1972,8 +1979,25 @@ SYSCALL_DEFINE1(userfaultfd, int, flags)
/* prevent the mm struct to be freed */
mmgrab(ctx->mm);
- fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, ctx,
- O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS));
+ file = anon_inode_getfile_secure(
+ "[userfaultfd]", &userfaultfd_fops, ctx,
+ O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS),
+ NULL);
+ if (IS_ERR(file)) {
+ fd = PTR_ERR(file);
+ goto out;
+ }
+
+ fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
+ if (fd < 0) {
+ fput(file);
+ goto out;
+ }
+
+ ctx->owner = file_inode(file);
+ fd_install(fd, file);
+
+out:
if (fd < 0) {
mmdrop(ctx->mm);
kmem_cache_free(userfaultfd_ctx_cachep, ctx);
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v2 3/3] Wire UFFD up to SELinux
2020-03-25 23:02 ` [PATCH v2 3/3] Wire UFFD up to SELinux Daniel Colascione
@ 2020-03-25 23:49 ` Casey Schaufler
0 siblings, 0 replies; 60+ messages in thread
From: Casey Schaufler @ 2020-03-25 23:49 UTC (permalink / raw)
To: Daniel Colascione, timmurray, selinux, linux-security-module,
linux-fsdevel, linux-kernel, kvm, viro, paul, nnk, sds,
lokeshgidra, Casey Schaufler
On 3/25/2020 4:02 PM, Daniel Colascione wrote:
> This change gives userfaultfd file descriptors a real security
> context, allowing policy to act on them.
You should change the title to "Wire UFFD up to secure anon inodes".
This code should support any LSM that wants to control anon inodes.
If it doesn't, it isn't correct.
All references to SELinux behavior (i.e. assigning a "security context")
should be restricted to the SELinux specific bits of the patch set. You
should be describing how any LSM can use this, not just the LSM you've
targeted.
>
> Signed-off-by: Daniel Colascione <dancol@google.com>
> ---
> fs/userfaultfd.c | 34 +++++++++++++++++++++++++++++-----
> 1 file changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index 07b0f6e03849..78ff5d898733 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -76,6 +76,8 @@ struct userfaultfd_ctx {
> bool mmap_changing;
> /* mm with one ore more vmas attached to this userfaultfd_ctx */
> struct mm_struct *mm;
> + /* The inode that owns this context --- not a strong reference. */
> + const struct inode *owner;
> };
>
> struct userfaultfd_fork_ctx {
> @@ -1014,14 +1016,18 @@ static __poll_t userfaultfd_poll(struct file *file, poll_table *wait)
> }
> }
>
> +static const struct file_operations userfaultfd_fops;
> +
> static int resolve_userfault_fork(struct userfaultfd_ctx *ctx,
> struct userfaultfd_ctx *new,
> struct uffd_msg *msg)
> {
> int fd;
>
> - fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, new,
> - O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS));
> + fd = anon_inode_getfd_secure(
> + "[userfaultfd]", &userfaultfd_fops, new,
> + O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS),
> + ctx->owner);
> if (fd < 0)
> return fd;
>
> @@ -1918,7 +1924,7 @@ static void userfaultfd_show_fdinfo(struct seq_file *m, struct file *f)
> }
> #endif
>
> -const struct file_operations userfaultfd_fops = {
> +static const struct file_operations userfaultfd_fops = {
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = userfaultfd_show_fdinfo,
> #endif
> @@ -1943,6 +1949,7 @@ static void init_once_userfaultfd_ctx(void *mem)
>
> SYSCALL_DEFINE1(userfaultfd, int, flags)
> {
> + struct file *file;
> struct userfaultfd_ctx *ctx;
> int fd;
>
> @@ -1972,8 +1979,25 @@ SYSCALL_DEFINE1(userfaultfd, int, flags)
> /* prevent the mm struct to be freed */
> mmgrab(ctx->mm);
>
> - fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, ctx,
> - O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS));
> + file = anon_inode_getfile_secure(
> + "[userfaultfd]", &userfaultfd_fops, ctx,
> + O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS),
> + NULL);
> + if (IS_ERR(file)) {
> + fd = PTR_ERR(file);
> + goto out;
> + }
> +
> + fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
> + if (fd < 0) {
> + fput(file);
> + goto out;
> + }
> +
> + ctx->owner = file_inode(file);
> + fd_install(fd, file);
> +
> +out:
> if (fd < 0) {
> mmdrop(ctx->mm);
> kmem_cache_free(userfaultfd_ctx_cachep, ctx);
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v3 0/3] SELinux support for anonymous inodes and UFFD
2020-02-14 3:26 ` [PATCH 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
` (6 preceding siblings ...)
2020-03-25 23:02 ` [PATCH v2 3/3] Wire UFFD up to SELinux Daniel Colascione
@ 2020-03-26 18:14 ` Daniel Colascione
2020-03-26 18:14 ` [PATCH v3 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
` (3 more replies)
7 siblings, 4 replies; 60+ messages in thread
From: Daniel Colascione @ 2020-03-26 18:14 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra, jmorris
Cc: Daniel Colascione
Userfaultfd in unprivileged contexts could be potentially very
useful. We'd like to harden userfaultfd to make such unprivileged use
less risky. This patch series allows SELinux to manage userfaultfd
file descriptors and in the future, other kinds of
anonymous-inode-based file descriptor. SELinux policy authors can
apply policy types to anonymous inodes by providing name-based
transition rules keyed off the anonymous inode internal name (
"[userfaultfd]" in the case of userfaultfd(2) file descriptors) and
applying policy to the new SIDs thus produced.
Inside the kernel, a pair of new anon_inodes interface,
anon_inode_getfile_secure and anon_inode_getfd_secure, allow callers
to opt into this SELinux management. In this new "secure" mode,
anon_inodes creates new ephemeral inodes for anonymous file objects
instead of reusing the normal anon_inodes singleton dummy inode. A new
LSM hook gives security modules an opportunity to configure and veto
these ephemeral inodes.
This patch series is one of two fork of [1] and is an
alternative to [2].
The primary difference between the two patch series is that this
partch series creates a unique inode for each "secure" anonymous
inode, while the other patch series ([2]) continues using the
singleton dummy anonymous inode and adds a way to attach SELinux
security information directly to file objects.
I prefer the approach in this patch series because 1) it's a smaller
patch than [2], and 2) it produces a more regular security
architecture: in this patch series, secure anonymous inodes aren't
S_PRIVATE and they maintain the SELinux property that the label for a
file is in its inode. We do need an additional inode per anonymous
file, but per-struct-file inode creation doesn't seem to be a problem
for pipes and sockets.
The previous version of this feature ([1]) created a new SELinux
security class for userfaultfd file descriptors. This version adopts
the generic transition-based approach of [2].
This patch series also differs from [2] in that it doesn't affect all
anonymous inodes right away --- instead requiring anon_inodes callers
to opt in --- but this difference isn't one of basic approach. The
important question to resolve is whether we should be creating new
inodes or enhancing per-file data.
Changes from the first version of the patch:
- Removed some error checks
- Defined a new anon_inode SELinux class to resolve the
ambiguity in [3]
- Inherit sclass as well as descriptor from context inode
Changes from the second version of the patch:
- Fixed example policy in the commit message to reflect the use of
the new anon_inode class.
[1] https://lore.kernel.org/lkml/20200211225547.235083-1-dancol@google.com/
[2] https://lore.kernel.org/linux-fsdevel/20200213194157.5877-1-sds@tycho.nsa.gov/
[3] https://lore.kernel.org/lkml/23f725ca-5b5a-5938-fcc8-5bbbfc9ba9bc@tycho.nsa.gov/
Daniel Colascione (3):
Add a new LSM-supporting anonymous inode interface
Teach SELinux about anonymous inodes
Wire UFFD up to SELinux
fs/anon_inodes.c | 196 ++++++++++++++++++++++------
fs/userfaultfd.c | 30 ++++-
include/linux/anon_inodes.h | 13 ++
include/linux/lsm_hooks.h | 9 ++
include/linux/security.h | 4 +
security/security.c | 10 ++
security/selinux/hooks.c | 54 ++++++++
security/selinux/include/classmap.h | 2 +
8 files changed, 272 insertions(+), 46 deletions(-)
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH v3 1/3] Add a new LSM-supporting anonymous inode interface
2020-03-26 18:14 ` [PATCH v3 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
@ 2020-03-26 18:14 ` Daniel Colascione
2020-03-26 19:00 ` Stephen Smalley
2020-03-26 18:14 ` [PATCH v3 2/3] Teach SELinux about anonymous inodes Daniel Colascione
` (2 subsequent siblings)
3 siblings, 1 reply; 60+ messages in thread
From: Daniel Colascione @ 2020-03-26 18:14 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra, jmorris
Cc: Daniel Colascione
This change adds two new functions, anon_inode_getfile_secure and
anon_inode_getfd_secure, that create anonymous-node files with
individual non-S_PRIVATE inodes to which security modules can apply
policy. Existing callers continue using the original singleton-inode
kind of anonymous-inode file. We can transition anonymous inode users
to the new kind of anonymous inode in individual patches for the sake
of bisection and review.
The new functions accept an optional context_inode parameter that
callers can use to provide additional contextual information to
security modules, e.g., indicating that one anonymous struct file is a
logical child of another, allowing a security model to propagate
security information from one to the other.
Signed-off-by: Daniel Colascione <dancol@google.com>
---
fs/anon_inodes.c | 196 ++++++++++++++++++++++++++++--------
fs/userfaultfd.c | 4 +-
include/linux/anon_inodes.h | 13 +++
include/linux/lsm_hooks.h | 9 ++
include/linux/security.h | 4 +
security/security.c | 10 ++
6 files changed, 191 insertions(+), 45 deletions(-)
diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 89714308c25b..114a04fc1db4 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -55,75 +55,135 @@ static struct file_system_type anon_inode_fs_type = {
.kill_sb = kill_anon_super,
};
-/**
- * anon_inode_getfile - creates a new file instance by hooking it up to an
- * anonymous inode, and a dentry that describe the "class"
- * of the file
- *
- * @name: [in] name of the "class" of the new file
- * @fops: [in] file operations for the new file
- * @priv: [in] private data for the new file (will be file's private_data)
- * @flags: [in] flags
- *
- * Creates a new file by hooking it on a single inode. This is useful for files
- * that do not need to have a full-fledged inode in order to operate correctly.
- * All the files created with anon_inode_getfile() will share a single inode,
- * hence saving memory and avoiding code duplication for the file/inode/dentry
- * setup. Returns the newly created file* or an error pointer.
- */
-struct file *anon_inode_getfile(const char *name,
- const struct file_operations *fops,
- void *priv, int flags)
+static struct inode *anon_inode_make_secure_inode(
+ const char *name,
+ const struct inode *context_inode,
+ const struct file_operations *fops)
+{
+ struct inode *inode;
+ const struct qstr qname = QSTR_INIT(name, strlen(name));
+ int error;
+
+ inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
+ if (IS_ERR(inode))
+ return ERR_PTR(PTR_ERR(inode));
+ inode->i_flags &= ~S_PRIVATE;
+ error = security_inode_init_security_anon(
+ inode, &qname, fops, context_inode);
+ if (error) {
+ iput(inode);
+ return ERR_PTR(error);
+ }
+ return inode;
+}
+
+struct file *_anon_inode_getfile(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode,
+ bool secure)
{
+ struct inode *inode;
struct file *file;
- if (IS_ERR(anon_inode_inode))
- return ERR_PTR(-ENODEV);
+ if (secure) {
+ inode = anon_inode_make_secure_inode(
+ name, context_inode, fops);
+ if (IS_ERR(inode))
+ return ERR_PTR(PTR_ERR(inode));
+ } else {
+ inode = anon_inode_inode;
+ if (IS_ERR(inode))
+ return ERR_PTR(-ENODEV);
+ /*
+ * We know the anon_inode inode count is always
+ * greater than zero, so ihold() is safe.
+ */
+ ihold(inode);
+ }
- if (fops->owner && !try_module_get(fops->owner))
- return ERR_PTR(-ENOENT);
+ if (fops->owner && !try_module_get(fops->owner)) {
+ file = ERR_PTR(-ENOENT);
+ goto err;
+ }
- /*
- * We know the anon_inode inode count is always greater than zero,
- * so ihold() is safe.
- */
- ihold(anon_inode_inode);
- file = alloc_file_pseudo(anon_inode_inode, anon_inode_mnt, name,
+ file = alloc_file_pseudo(inode, anon_inode_mnt, name,
flags & (O_ACCMODE | O_NONBLOCK), fops);
if (IS_ERR(file))
goto err;
- file->f_mapping = anon_inode_inode->i_mapping;
+ file->f_mapping = inode->i_mapping;
file->private_data = priv;
return file;
err:
- iput(anon_inode_inode);
+ iput(inode);
module_put(fops->owner);
return file;
}
-EXPORT_SYMBOL_GPL(anon_inode_getfile);
/**
- * anon_inode_getfd - creates a new file instance by hooking it up to an
- * anonymous inode, and a dentry that describe the "class"
- * of the file
+ * anon_inode_getfile_secure - creates a new file instance by hooking
+ * it up to a new anonymous inode and a
+ * dentry that describe the "class" of the
+ * file. Make it possible to use security
+ * modules to control access to the
+ * new file.
*
* @name: [in] name of the "class" of the new file
* @fops: [in] file operations for the new file
* @priv: [in] private data for the new file (will be file's private_data)
- * @flags: [in] flags
+ * @flags: [in] flags for the file
+ * @anon_inode_flags: [in] flags for anon_inode*
+ *
+ * Creates a new file by hooking it on an unspecified inode. This is
+ * useful for files that do not need to have a full-fledged inode in
+ * order to operate correctly. All the files created with
+ * anon_inode_getfile_secure() will have distinct inodes, avoiding
+ * code duplication for the file/inode/dentry setup. Returns the
+ * newly created file* or an error pointer.
+ */
+struct file *anon_inode_getfile_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode)
+{
+ return _anon_inode_getfile(
+ name, fops, priv, flags, context_inode, true);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfile_secure);
+
+/**
+ * anon_inode_getfile - creates a new file instance by hooking it up
+ * to an anonymous inode and a dentry that
+ * describe the "class" of the file.
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags for the file
*
- * Creates a new file by hooking it on a single inode. This is useful for files
+ * Creates a new file by hooking it on an unspecified inode. This is useful for files
* that do not need to have a full-fledged inode in order to operate correctly.
- * All the files created with anon_inode_getfd() will share a single inode,
+ * All the files created with anon_inode_getfile() will share a single inode,
* hence saving memory and avoiding code duplication for the file/inode/dentry
- * setup. Returns new descriptor or an error code.
+ * setup. Returns the newly created file* or an error pointer.
*/
-int anon_inode_getfd(const char *name, const struct file_operations *fops,
- void *priv, int flags)
+struct file *anon_inode_getfile(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags)
+{
+ return _anon_inode_getfile(name, fops, priv, flags, NULL, false);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfile);
+
+static int _anon_inode_getfd(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode,
+ bool secure)
{
int error, fd;
struct file *file;
@@ -133,7 +193,8 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops,
return error;
fd = error;
- file = anon_inode_getfile(name, fops, priv, flags);
+ file = _anon_inode_getfile(name, fops, priv, flags, context_inode,
+ secure);
if (IS_ERR(file)) {
error = PTR_ERR(file);
goto err_put_unused_fd;
@@ -146,6 +207,57 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops,
put_unused_fd(fd);
return error;
}
+
+/**
+ * anon_inode_getfd_secure - creates a new file instance by hooking it
+ * up to a new anonymous inode and a dentry
+ * that describe the "class" of the file.
+ * Make it possible to use security modules
+ * to control access to the new file.
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags
+ *
+ * Creates a new file by hooking it on an unspecified inode. This is
+ * useful for files that do not need to have a full-fledged inode in
+ * order to operate correctly. All the files created with
+ * anon_inode_getfile_secure() will have distinct inodes, avoiding
+ * code duplication for the file/inode/dentry setup. Returns a newly
+ * created file descriptor or an error code.
+ */
+int anon_inode_getfd_secure(const char *name, const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode)
+{
+ return _anon_inode_getfd(name, fops, priv, flags,
+ context_inode, true);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfd_secure);
+
+/**
+ * anon_inode_getfd - creates a new file instance by hooking it up to
+ * an anonymous inode and a dentry that describe
+ * the "class" of the file
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags
+ *
+ * Creates a new file by hooking it on an unspecified inode. This is
+ * useful for files that do not need to have a full-fledged inode in
+ * order to operate correctly. All the files created with
+ * anon_inode_getfile() will use the same singleton inode, reducing
+ * memory use and avoiding code duplication for the file/inode/dentry
+ * setup. Returns a newly created file descriptor or an error code.
+ */
+int anon_inode_getfd(const char *name, const struct file_operations *fops,
+ void *priv, int flags)
+{
+ return _anon_inode_getfd(name, fops, priv, flags, NULL, false);
+}
EXPORT_SYMBOL_GPL(anon_inode_getfd);
static int __init anon_inode_init(void)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 37df7c9eedb1..07b0f6e03849 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -1014,8 +1014,6 @@ static __poll_t userfaultfd_poll(struct file *file, poll_table *wait)
}
}
-static const struct file_operations userfaultfd_fops;
-
static int resolve_userfault_fork(struct userfaultfd_ctx *ctx,
struct userfaultfd_ctx *new,
struct uffd_msg *msg)
@@ -1920,7 +1918,7 @@ static void userfaultfd_show_fdinfo(struct seq_file *m, struct file *f)
}
#endif
-static const struct file_operations userfaultfd_fops = {
+const struct file_operations userfaultfd_fops = {
#ifdef CONFIG_PROC_FS
.show_fdinfo = userfaultfd_show_fdinfo,
#endif
diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
index d0d7d96261ad..67bd85d92dca 100644
--- a/include/linux/anon_inodes.h
+++ b/include/linux/anon_inodes.h
@@ -10,12 +10,25 @@
#define _LINUX_ANON_INODES_H
struct file_operations;
+struct inode;
+
+struct file *anon_inode_getfile_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode);
struct file *anon_inode_getfile(const char *name,
const struct file_operations *fops,
void *priv, int flags);
+
+int anon_inode_getfd_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode);
+
int anon_inode_getfd(const char *name, const struct file_operations *fops,
void *priv, int flags);
+
#endif /* _LINUX_ANON_INODES_H */
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 20d8cf194fb7..de5d37e388df 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -215,6 +215,10 @@
* Returns 0 if @name and @value have been successfully set,
* -EOPNOTSUPP if no security attribute is needed, or
* -ENOMEM on memory allocation failure.
+ * @inode_init_security_anon:
+ * Set up a secure anonymous inode.
+ * Returns 0 on success. Returns -EPERM if the security module denies
+ * the creation of this inode.
* @inode_create:
* Check permission to create a regular file.
* @dir contains inode structure of the parent of the new file.
@@ -1552,6 +1556,10 @@ union security_list_options {
const struct qstr *qstr,
const char **name, void **value,
size_t *len);
+ int (*inode_init_security_anon)(struct inode *inode,
+ const struct qstr *name,
+ const struct file_operations *fops,
+ const struct inode *context_inode);
int (*inode_create)(struct inode *dir, struct dentry *dentry,
umode_t mode);
int (*inode_link)(struct dentry *old_dentry, struct inode *dir,
@@ -1884,6 +1892,7 @@ struct security_hook_heads {
struct hlist_head inode_alloc_security;
struct hlist_head inode_free_security;
struct hlist_head inode_init_security;
+ struct hlist_head inode_init_security_anon;
struct hlist_head inode_create;
struct hlist_head inode_link;
struct hlist_head inode_unlink;
diff --git a/include/linux/security.h b/include/linux/security.h
index 64b19f050343..8ea76af0be7a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -320,6 +320,10 @@ void security_inode_free(struct inode *inode);
int security_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr,
initxattrs initxattrs, void *fs_data);
+int security_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct file_operations *fops,
+ const struct inode *context_inode);
int security_old_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr, const char **name,
void **value, size_t *len);
diff --git a/security/security.c b/security/security.c
index 565bc9b67276..d06f3969c030 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1033,6 +1033,16 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
}
EXPORT_SYMBOL(security_inode_init_security);
+int
+security_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct file_operations *fops,
+ const struct inode *context_inode)
+{
+ return call_int_hook(inode_init_security_anon, 0, inode, name,
+ fops, context_inode);
+}
+
int security_old_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr, const char **name,
void **value, size_t *len)
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v3 1/3] Add a new LSM-supporting anonymous inode interface
2020-03-26 18:14 ` [PATCH v3 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
@ 2020-03-26 19:00 ` Stephen Smalley
0 siblings, 0 replies; 60+ messages in thread
From: Stephen Smalley @ 2020-03-26 19:00 UTC (permalink / raw)
To: Daniel Colascione, timmurray, selinux, linux-security-module,
linux-fsdevel, linux-kernel, kvm, viro, paul, nnk, lokeshgidra,
jmorris
On 3/26/20 2:14 PM, Daniel Colascione wrote:
> This change adds two new functions, anon_inode_getfile_secure and
> anon_inode_getfd_secure, that create anonymous-node files with
> individual non-S_PRIVATE inodes to which security modules can apply
> policy. Existing callers continue using the original singleton-inode
> kind of anonymous-inode file. We can transition anonymous inode users
> to the new kind of anonymous inode in individual patches for the sake
> of bisection and review.
>
> The new functions accept an optional context_inode parameter that
> callers can use to provide additional contextual information to
> security modules, e.g., indicating that one anonymous struct file is a
> logical child of another, allowing a security model to propagate
> security information from one to the other.
>
> Signed-off-by: Daniel Colascione <dancol@google.com>
> ---
Repeating verbatim what I said on v2 of patch 1/3.
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index 89714308c25b..114a04fc1db4 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -55,75 +55,135 @@ static struct file_system_type anon_inode_fs_type = {
> .kill_sb = kill_anon_super,
> };
>
> -/**
> - * anon_inode_getfile - creates a new file instance by hooking it up to an
> - * anonymous inode, and a dentry that describe the "class"
> - * of the file
> - *
> - * @name: [in] name of the "class" of the new file
> - * @fops: [in] file operations for the new file
> - * @priv: [in] private data for the new file (will be file's private_data)
> - * @flags: [in] flags
> - *
> - * Creates a new file by hooking it on a single inode. This is useful for files
> - * that do not need to have a full-fledged inode in order to operate correctly.
> - * All the files created with anon_inode_getfile() will share a single inode,
> - * hence saving memory and avoiding code duplication for the file/inode/dentry
> - * setup. Returns the newly created file* or an error pointer.
> - */
> -struct file *anon_inode_getfile(const char *name,
> - const struct file_operations *fops,
> - void *priv, int flags)
> +static struct inode *anon_inode_make_secure_inode(
> + const char *name,
> + const struct inode *context_inode,
> + const struct file_operations *fops)
> +{
> + struct inode *inode;
> + const struct qstr qname = QSTR_INIT(name, strlen(name));
> + int error;
> +
> + inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
> + if (IS_ERR(inode))
> + return ERR_PTR(PTR_ERR(inode));
Just return inode here ?
> + inode->i_flags &= ~S_PRIVATE;
> + error = security_inode_init_security_anon(
> + inode, &qname, fops, context_inode);
Would drop fops argument until we have a real user for it; we can always
add it later.
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index 37df7c9eedb1..07b0f6e03849 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -1014,8 +1014,6 @@ static __poll_t userfaultfd_poll(struct file *file, poll_table *wait)
> }
> }
>
> -static const struct file_operations userfaultfd_fops;
> -
> static int resolve_userfault_fork(struct userfaultfd_ctx *ctx,
> struct userfaultfd_ctx *new,
> struct uffd_msg *msg)
> @@ -1920,7 +1918,7 @@ static void userfaultfd_show_fdinfo(struct seq_file *m, struct file *f)
> }
> #endif
>
> -static const struct file_operations userfaultfd_fops = {
> +const struct file_operations userfaultfd_fops = {
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = userfaultfd_show_fdinfo,
> #endif
These changes are unnecessary for this patch and reverted by patch 3, so
drop them here.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v3 2/3] Teach SELinux about anonymous inodes
2020-03-26 18:14 ` [PATCH v3 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
2020-03-26 18:14 ` [PATCH v3 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
@ 2020-03-26 18:14 ` Daniel Colascione
2020-03-26 19:02 ` Stephen Smalley
2020-03-26 18:14 ` [PATCH v3 3/3] Wire UFFD up to SELinux Daniel Colascione
2020-03-26 20:06 ` [PATCH v4 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
3 siblings, 1 reply; 60+ messages in thread
From: Daniel Colascione @ 2020-03-26 18:14 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra, jmorris
Cc: Daniel Colascione
This change uses the anon_inodes and LSM infrastructure introduced in
the previous patch to give SELinux the ability to control
anonymous-inode files that are created using the new _secure()
anon_inodes functions.
A SELinux policy author detects and controls these anonymous inodes by
adding a name-based type_transition rule that assigns a new security
type to anonymous-inode files created in some domain. The name used
for the name-based transition is the name associated with the
anonymous inode for file listings --- e.g., "[userfaultfd]" or
"[perf_event]".
Example:
type uffd_t;
type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]";
allow sysadm_t uffd_t:anon_inode { create };
(The next patch in this series is necessary for making userfaultfd
support this new interface. The example above is just
for exposition.)
Signed-off-by: Daniel Colascione <dancol@google.com>
---
security/selinux/hooks.c | 54 +++++++++++++++++++++++++++++
security/selinux/include/classmap.h | 2 ++
2 files changed, 56 insertions(+)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 1659b59fb5d7..b9eb45c2e4e5 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2915,6 +2915,59 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
return 0;
}
+static int selinux_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct file_operations *fops,
+ const struct inode *context_inode)
+{
+ const struct task_security_struct *tsec = selinux_cred(current_cred());
+ struct common_audit_data ad;
+ struct inode_security_struct *isec;
+ int rc;
+
+ if (unlikely(!selinux_state.initialized))
+ return 0;
+
+ isec = selinux_inode(inode);
+
+ /*
+ * We only get here once per ephemeral inode. The inode has
+ * been initialized via inode_alloc_security but is otherwise
+ * untouched.
+ */
+
+ if (context_inode) {
+ struct inode_security_struct *context_isec =
+ selinux_inode(context_inode);
+ isec->sclass = context_isec->sclass;
+ isec->sid = context_isec->sid;
+ } else {
+ isec->sclass = SECCLASS_ANON_INODE;
+ rc = security_transition_sid(
+ &selinux_state, tsec->sid, tsec->sid,
+ SECCLASS_FILE, name, &isec->sid);
+ if (rc)
+ return rc;
+ }
+
+ isec->initialized = LABEL_INITIALIZED;
+
+ /*
+ * Now that we've initialized security, check whether we're
+ * allowed to actually create this type of anonymous inode.
+ */
+
+ ad.type = LSM_AUDIT_DATA_INODE;
+ ad.u.inode = inode;
+
+ return avc_has_perm(&selinux_state,
+ tsec->sid,
+ isec->sid,
+ isec->sclass,
+ FILE__CREATE,
+ &ad);
+}
+
static int selinux_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode)
{
return may_create(dir, dentry, SECCLASS_FILE);
@@ -6923,6 +6976,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security),
LSM_HOOK_INIT(inode_init_security, selinux_inode_init_security),
+ LSM_HOOK_INIT(inode_init_security_anon, selinux_inode_init_security_anon),
LSM_HOOK_INIT(inode_create, selinux_inode_create),
LSM_HOOK_INIT(inode_link, selinux_inode_link),
LSM_HOOK_INIT(inode_unlink, selinux_inode_unlink),
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 986f3ac14282..263750b6aaac 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -248,6 +248,8 @@ struct security_class_mapping secclass_map[] = {
{"open", "cpu", "kernel", "tracepoint", "read", "write"} },
{ "lockdown",
{ "integrity", "confidentiality", NULL } },
+ { "anon_inode",
+ { COMMON_FILE_PERMS, NULL } },
{ NULL }
};
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v3 2/3] Teach SELinux about anonymous inodes
2020-03-26 18:14 ` [PATCH v3 2/3] Teach SELinux about anonymous inodes Daniel Colascione
@ 2020-03-26 19:02 ` Stephen Smalley
0 siblings, 0 replies; 60+ messages in thread
From: Stephen Smalley @ 2020-03-26 19:02 UTC (permalink / raw)
To: Daniel Colascione, timmurray, selinux, linux-security-module,
linux-fsdevel, linux-kernel, kvm, viro, paul, nnk, lokeshgidra,
jmorris
On 3/26/20 2:14 PM, Daniel Colascione wrote:
> This change uses the anon_inodes and LSM infrastructure introduced in
> the previous patch to give SELinux the ability to control
> anonymous-inode files that are created using the new _secure()
> anon_inodes functions.
>
> A SELinux policy author detects and controls these anonymous inodes by
> adding a name-based type_transition rule that assigns a new security
> type to anonymous-inode files created in some domain. The name used
> for the name-based transition is the name associated with the
> anonymous inode for file listings --- e.g., "[userfaultfd]" or
> "[perf_event]".
>
> Example:
>
> type uffd_t;
> type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]";
> allow sysadm_t uffd_t:anon_inode { create };
>
> (The next patch in this series is necessary for making userfaultfd
> support this new interface. The example above is just
> for exposition.)
>
> Signed-off-by: Daniel Colascione <dancol@google.com>
> ---
> security/selinux/hooks.c | 54 +++++++++++++++++++++++++++++
> security/selinux/include/classmap.h | 2 ++
> 2 files changed, 56 insertions(+)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 1659b59fb5d7..b9eb45c2e4e5 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2915,6 +2915,59 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> return 0;
> }
>
> +static int selinux_inode_init_security_anon(struct inode *inode,
> + const struct qstr *name,
> + const struct file_operations *fops,
> + const struct inode *context_inode)
> +{
> + const struct task_security_struct *tsec = selinux_cred(current_cred());
> + struct common_audit_data ad;
> + struct inode_security_struct *isec;
> + int rc;
> +
> + if (unlikely(!selinux_state.initialized))
> + return 0;
> +
> + isec = selinux_inode(inode);
> +
> + /*
> + * We only get here once per ephemeral inode. The inode has
> + * been initialized via inode_alloc_security but is otherwise
> + * untouched.
> + */
> +
> + if (context_inode) {
> + struct inode_security_struct *context_isec =
> + selinux_inode(context_inode);
> + isec->sclass = context_isec->sclass;
> + isec->sid = context_isec->sid;
> + } else {
> + isec->sclass = SECCLASS_ANON_INODE;
> + rc = security_transition_sid(
> + &selinux_state, tsec->sid, tsec->sid,
> + SECCLASS_FILE, name, &isec->sid);
I guess you missed the 2nd comment in my reply to v2 of this patch:
You should use isec->sclass aka SECCLASS_ANON_INODE instead of
SECCLASS_FILE here.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v3 3/3] Wire UFFD up to SELinux
2020-03-26 18:14 ` [PATCH v3 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
2020-03-26 18:14 ` [PATCH v3 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
2020-03-26 18:14 ` [PATCH v3 2/3] Teach SELinux about anonymous inodes Daniel Colascione
@ 2020-03-26 18:14 ` Daniel Colascione
2020-03-26 20:06 ` [PATCH v4 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
3 siblings, 0 replies; 60+ messages in thread
From: Daniel Colascione @ 2020-03-26 18:14 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra, jmorris
Cc: Daniel Colascione
This change gives userfaultfd file descriptors a real security
context, allowing policy to act on them.
Signed-off-by: Daniel Colascione <dancol@google.com>
---
fs/userfaultfd.c | 34 +++++++++++++++++++++++++++++-----
1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 07b0f6e03849..78ff5d898733 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -76,6 +76,8 @@ struct userfaultfd_ctx {
bool mmap_changing;
/* mm with one ore more vmas attached to this userfaultfd_ctx */
struct mm_struct *mm;
+ /* The inode that owns this context --- not a strong reference. */
+ const struct inode *owner;
};
struct userfaultfd_fork_ctx {
@@ -1014,14 +1016,18 @@ static __poll_t userfaultfd_poll(struct file *file, poll_table *wait)
}
}
+static const struct file_operations userfaultfd_fops;
+
static int resolve_userfault_fork(struct userfaultfd_ctx *ctx,
struct userfaultfd_ctx *new,
struct uffd_msg *msg)
{
int fd;
- fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, new,
- O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS));
+ fd = anon_inode_getfd_secure(
+ "[userfaultfd]", &userfaultfd_fops, new,
+ O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS),
+ ctx->owner);
if (fd < 0)
return fd;
@@ -1918,7 +1924,7 @@ static void userfaultfd_show_fdinfo(struct seq_file *m, struct file *f)
}
#endif
-const struct file_operations userfaultfd_fops = {
+static const struct file_operations userfaultfd_fops = {
#ifdef CONFIG_PROC_FS
.show_fdinfo = userfaultfd_show_fdinfo,
#endif
@@ -1943,6 +1949,7 @@ static void init_once_userfaultfd_ctx(void *mem)
SYSCALL_DEFINE1(userfaultfd, int, flags)
{
+ struct file *file;
struct userfaultfd_ctx *ctx;
int fd;
@@ -1972,8 +1979,25 @@ SYSCALL_DEFINE1(userfaultfd, int, flags)
/* prevent the mm struct to be freed */
mmgrab(ctx->mm);
- fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, ctx,
- O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS));
+ file = anon_inode_getfile_secure(
+ "[userfaultfd]", &userfaultfd_fops, ctx,
+ O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS),
+ NULL);
+ if (IS_ERR(file)) {
+ fd = PTR_ERR(file);
+ goto out;
+ }
+
+ fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
+ if (fd < 0) {
+ fput(file);
+ goto out;
+ }
+
+ ctx->owner = file_inode(file);
+ fd_install(fd, file);
+
+out:
if (fd < 0) {
mmdrop(ctx->mm);
kmem_cache_free(userfaultfd_ctx_cachep, ctx);
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v4 0/3] SELinux support for anonymous inodes and UFFD
2020-03-26 18:14 ` [PATCH v3 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
` (2 preceding siblings ...)
2020-03-26 18:14 ` [PATCH v3 3/3] Wire UFFD up to SELinux Daniel Colascione
@ 2020-03-26 20:06 ` Daniel Colascione
2020-03-26 20:06 ` [PATCH v4 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
` (3 more replies)
3 siblings, 4 replies; 60+ messages in thread
From: Daniel Colascione @ 2020-03-26 20:06 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra, jmorris
Cc: Daniel Colascione
Userfaultfd in unprivileged contexts could be potentially very
useful. We'd like to harden userfaultfd to make such unprivileged use
less risky. This patch series allows SELinux to manage userfaultfd
file descriptors and in the future, other kinds of
anonymous-inode-based file descriptor. SELinux policy authors can
apply policy types to anonymous inodes by providing name-based
transition rules keyed off the anonymous inode internal name (
"[userfaultfd]" in the case of userfaultfd(2) file descriptors) and
applying policy to the new SIDs thus produced.
Inside the kernel, a pair of new anon_inodes interface,
anon_inode_getfile_secure and anon_inode_getfd_secure, allow callers
to opt into this SELinux management. In this new "secure" mode,
anon_inodes creates new ephemeral inodes for anonymous file objects
instead of reusing the normal anon_inodes singleton dummy inode. A new
LSM hook gives security modules an opportunity to configure and veto
these ephemeral inodes.
This patch series is one of two fork of [1] and is an
alternative to [2].
The primary difference between the two patch series is that this
partch series creates a unique inode for each "secure" anonymous
inode, while the other patch series ([2]) continues using the
singleton dummy anonymous inode and adds a way to attach SELinux
security information directly to file objects.
I prefer the approach in this patch series because 1) it's a smaller
patch than [2], and 2) it produces a more regular security
architecture: in this patch series, secure anonymous inodes aren't
S_PRIVATE and they maintain the SELinux property that the label for a
file is in its inode. We do need an additional inode per anonymous
file, but per-struct-file inode creation doesn't seem to be a problem
for pipes and sockets.
The previous version of this feature ([1]) created a new SELinux
security class for userfaultfd file descriptors. This version adopts
the generic transition-based approach of [2].
This patch series also differs from [2] in that it doesn't affect all
anonymous inodes right away --- instead requiring anon_inodes callers
to opt in --- but this difference isn't one of basic approach. The
important question to resolve is whether we should be creating new
inodes or enhancing per-file data.
Changes from the first version of the patch:
- Removed some error checks
- Defined a new anon_inode SELinux class to resolve the
ambiguity in [3]
- Inherit sclass as well as descriptor from context inode
Changes from the second version of the patch:
- Fixed example policy in the commit message to reflect the use of
the new anon_inode class.
Changes from the third version of the patch:
- Dropped the fops parameter to the LSM hook
- Documented hook parameters
- Fixed incorrect class used for SELinux transition
- Removed stray UFFD changed early in the series
- Removed a redundant ERR_PTR(PTR_ERR())
[1] https://lore.kernel.org/lkml/20200211225547.235083-1-dancol@google.com/
[2] https://lore.kernel.org/linux-fsdevel/20200213194157.5877-1-sds@tycho.nsa.gov/
[3] https://lore.kernel.org/lkml/23f725ca-5b5a-5938-fcc8-5bbbfc9ba9bc@tycho.nsa.gov/
Daniel Colascione (3):
Add a new LSM-supporting anonymous inode interface
Teach SELinux about anonymous inodes
Wire UFFD up to SELinux
fs/anon_inodes.c | 196 ++++++++++++++++++++++------
fs/userfaultfd.c | 30 ++++-
include/linux/anon_inodes.h | 13 ++
include/linux/lsm_hooks.h | 11 ++
include/linux/security.h | 3 +
security/security.c | 9 ++
security/selinux/hooks.c | 53 ++++++++
security/selinux/include/classmap.h | 2 +
8 files changed, 271 insertions(+), 46 deletions(-)
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH v4 1/3] Add a new LSM-supporting anonymous inode interface
2020-03-26 20:06 ` [PATCH v4 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
@ 2020-03-26 20:06 ` Daniel Colascione
2020-03-27 13:40 ` Stephen Smalley
2020-03-26 20:06 ` [PATCH v4 2/3] Teach SELinux about anonymous inodes Daniel Colascione
` (2 subsequent siblings)
3 siblings, 1 reply; 60+ messages in thread
From: Daniel Colascione @ 2020-03-26 20:06 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra, jmorris
Cc: Daniel Colascione
This change adds two new functions, anon_inode_getfile_secure and
anon_inode_getfd_secure, that create anonymous-node files with
individual non-S_PRIVATE inodes to which security modules can apply
policy. Existing callers continue using the original singleton-inode
kind of anonymous-inode file. We can transition anonymous inode users
to the new kind of anonymous inode in individual patches for the sake
of bisection and review.
The new functions accept an optional context_inode parameter that
callers can use to provide additional contextual information to
security modules, e.g., indicating that one anonymous struct file is a
logical child of another, allowing a security model to propagate
security information from one to the other.
Signed-off-by: Daniel Colascione <dancol@google.com>
---
fs/anon_inodes.c | 196 ++++++++++++++++++++++++++++--------
include/linux/anon_inodes.h | 13 +++
include/linux/lsm_hooks.h | 11 ++
include/linux/security.h | 3 +
security/security.c | 9 ++
5 files changed, 190 insertions(+), 42 deletions(-)
diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 89714308c25b..024059e333dc 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -55,75 +55,135 @@ static struct file_system_type anon_inode_fs_type = {
.kill_sb = kill_anon_super,
};
-/**
- * anon_inode_getfile - creates a new file instance by hooking it up to an
- * anonymous inode, and a dentry that describe the "class"
- * of the file
- *
- * @name: [in] name of the "class" of the new file
- * @fops: [in] file operations for the new file
- * @priv: [in] private data for the new file (will be file's private_data)
- * @flags: [in] flags
- *
- * Creates a new file by hooking it on a single inode. This is useful for files
- * that do not need to have a full-fledged inode in order to operate correctly.
- * All the files created with anon_inode_getfile() will share a single inode,
- * hence saving memory and avoiding code duplication for the file/inode/dentry
- * setup. Returns the newly created file* or an error pointer.
- */
-struct file *anon_inode_getfile(const char *name,
- const struct file_operations *fops,
- void *priv, int flags)
+static struct inode *anon_inode_make_secure_inode(
+ const char *name,
+ const struct inode *context_inode,
+ const struct file_operations *fops)
+{
+ struct inode *inode;
+ const struct qstr qname = QSTR_INIT(name, strlen(name));
+ int error;
+
+ inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
+ if (IS_ERR(inode))
+ return inode;
+ inode->i_flags &= ~S_PRIVATE;
+ error = security_inode_init_security_anon(
+ inode, &qname, context_inode);
+ if (error) {
+ iput(inode);
+ return ERR_PTR(error);
+ }
+ return inode;
+}
+
+struct file *_anon_inode_getfile(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode,
+ bool secure)
{
+ struct inode *inode;
struct file *file;
- if (IS_ERR(anon_inode_inode))
- return ERR_PTR(-ENODEV);
+ if (secure) {
+ inode = anon_inode_make_secure_inode(
+ name, context_inode, fops);
+ if (IS_ERR(inode))
+ return ERR_PTR(PTR_ERR(inode));
+ } else {
+ inode = anon_inode_inode;
+ if (IS_ERR(inode))
+ return ERR_PTR(-ENODEV);
+ /*
+ * We know the anon_inode inode count is always
+ * greater than zero, so ihold() is safe.
+ */
+ ihold(inode);
+ }
- if (fops->owner && !try_module_get(fops->owner))
- return ERR_PTR(-ENOENT);
+ if (fops->owner && !try_module_get(fops->owner)) {
+ file = ERR_PTR(-ENOENT);
+ goto err;
+ }
- /*
- * We know the anon_inode inode count is always greater than zero,
- * so ihold() is safe.
- */
- ihold(anon_inode_inode);
- file = alloc_file_pseudo(anon_inode_inode, anon_inode_mnt, name,
+ file = alloc_file_pseudo(inode, anon_inode_mnt, name,
flags & (O_ACCMODE | O_NONBLOCK), fops);
if (IS_ERR(file))
goto err;
- file->f_mapping = anon_inode_inode->i_mapping;
+ file->f_mapping = inode->i_mapping;
file->private_data = priv;
return file;
err:
- iput(anon_inode_inode);
+ iput(inode);
module_put(fops->owner);
return file;
}
-EXPORT_SYMBOL_GPL(anon_inode_getfile);
/**
- * anon_inode_getfd - creates a new file instance by hooking it up to an
- * anonymous inode, and a dentry that describe the "class"
- * of the file
+ * anon_inode_getfile_secure - creates a new file instance by hooking
+ * it up to a new anonymous inode and a
+ * dentry that describe the "class" of the
+ * file. Make it possible to use security
+ * modules to control access to the
+ * new file.
*
* @name: [in] name of the "class" of the new file
* @fops: [in] file operations for the new file
* @priv: [in] private data for the new file (will be file's private_data)
- * @flags: [in] flags
+ * @flags: [in] flags for the file
+ * @anon_inode_flags: [in] flags for anon_inode*
+ *
+ * Creates a new file by hooking it on an unspecified inode. This is
+ * useful for files that do not need to have a full-fledged inode in
+ * order to operate correctly. All the files created with
+ * anon_inode_getfile_secure() will have distinct inodes, avoiding
+ * code duplication for the file/inode/dentry setup. Returns the
+ * newly created file* or an error pointer.
+ */
+struct file *anon_inode_getfile_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode)
+{
+ return _anon_inode_getfile(
+ name, fops, priv, flags, context_inode, true);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfile_secure);
+
+/**
+ * anon_inode_getfile - creates a new file instance by hooking it up
+ * to an anonymous inode and a dentry that
+ * describe the "class" of the file.
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags for the file
*
- * Creates a new file by hooking it on a single inode. This is useful for files
+ * Creates a new file by hooking it on an unspecified inode. This is useful for files
* that do not need to have a full-fledged inode in order to operate correctly.
- * All the files created with anon_inode_getfd() will share a single inode,
+ * All the files created with anon_inode_getfile() will share a single inode,
* hence saving memory and avoiding code duplication for the file/inode/dentry
- * setup. Returns new descriptor or an error code.
+ * setup. Returns the newly created file* or an error pointer.
*/
-int anon_inode_getfd(const char *name, const struct file_operations *fops,
- void *priv, int flags)
+struct file *anon_inode_getfile(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags)
+{
+ return _anon_inode_getfile(name, fops, priv, flags, NULL, false);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfile);
+
+static int _anon_inode_getfd(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode,
+ bool secure)
{
int error, fd;
struct file *file;
@@ -133,7 +193,8 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops,
return error;
fd = error;
- file = anon_inode_getfile(name, fops, priv, flags);
+ file = _anon_inode_getfile(name, fops, priv, flags, context_inode,
+ secure);
if (IS_ERR(file)) {
error = PTR_ERR(file);
goto err_put_unused_fd;
@@ -146,6 +207,57 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops,
put_unused_fd(fd);
return error;
}
+
+/**
+ * anon_inode_getfd_secure - creates a new file instance by hooking it
+ * up to a new anonymous inode and a dentry
+ * that describe the "class" of the file.
+ * Make it possible to use security modules
+ * to control access to the new file.
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags
+ *
+ * Creates a new file by hooking it on an unspecified inode. This is
+ * useful for files that do not need to have a full-fledged inode in
+ * order to operate correctly. All the files created with
+ * anon_inode_getfile_secure() will have distinct inodes, avoiding
+ * code duplication for the file/inode/dentry setup. Returns a newly
+ * created file descriptor or an error code.
+ */
+int anon_inode_getfd_secure(const char *name, const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode)
+{
+ return _anon_inode_getfd(name, fops, priv, flags,
+ context_inode, true);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfd_secure);
+
+/**
+ * anon_inode_getfd - creates a new file instance by hooking it up to
+ * an anonymous inode and a dentry that describe
+ * the "class" of the file
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags
+ *
+ * Creates a new file by hooking it on an unspecified inode. This is
+ * useful for files that do not need to have a full-fledged inode in
+ * order to operate correctly. All the files created with
+ * anon_inode_getfile() will use the same singleton inode, reducing
+ * memory use and avoiding code duplication for the file/inode/dentry
+ * setup. Returns a newly created file descriptor or an error code.
+ */
+int anon_inode_getfd(const char *name, const struct file_operations *fops,
+ void *priv, int flags)
+{
+ return _anon_inode_getfd(name, fops, priv, flags, NULL, false);
+}
EXPORT_SYMBOL_GPL(anon_inode_getfd);
static int __init anon_inode_init(void)
diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
index d0d7d96261ad..67bd85d92dca 100644
--- a/include/linux/anon_inodes.h
+++ b/include/linux/anon_inodes.h
@@ -10,12 +10,25 @@
#define _LINUX_ANON_INODES_H
struct file_operations;
+struct inode;
+
+struct file *anon_inode_getfile_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode);
struct file *anon_inode_getfile(const char *name,
const struct file_operations *fops,
void *priv, int flags);
+
+int anon_inode_getfd_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode);
+
int anon_inode_getfd(const char *name, const struct file_operations *fops,
void *priv, int flags);
+
#endif /* _LINUX_ANON_INODES_H */
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 20d8cf194fb7..5434c1d285b2 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -215,6 +215,13 @@
* Returns 0 if @name and @value have been successfully set,
* -EOPNOTSUPP if no security attribute is needed, or
* -ENOMEM on memory allocation failure.
+ * @inode_init_security_anon:
+ * Set up a secure anonymous inode.
+ * @inode contains the inode structure
+ * @name name of the anonymous inode class
+ * @context_inode optional related inode
+ * Returns 0 on success. Returns -EPERM if the security module denies
+ * the creation of this inode.
* @inode_create:
* Check permission to create a regular file.
* @dir contains inode structure of the parent of the new file.
@@ -1552,6 +1559,9 @@ union security_list_options {
const struct qstr *qstr,
const char **name, void **value,
size_t *len);
+ int (*inode_init_security_anon)(struct inode *inode,
+ const struct qstr *name,
+ const struct inode *context_inode);
int (*inode_create)(struct inode *dir, struct dentry *dentry,
umode_t mode);
int (*inode_link)(struct dentry *old_dentry, struct inode *dir,
@@ -1884,6 +1894,7 @@ struct security_hook_heads {
struct hlist_head inode_alloc_security;
struct hlist_head inode_free_security;
struct hlist_head inode_init_security;
+ struct hlist_head inode_init_security_anon;
struct hlist_head inode_create;
struct hlist_head inode_link;
struct hlist_head inode_unlink;
diff --git a/include/linux/security.h b/include/linux/security.h
index 64b19f050343..2108c3ce0666 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -320,6 +320,9 @@ void security_inode_free(struct inode *inode);
int security_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr,
initxattrs initxattrs, void *fs_data);
+int security_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct inode *context_inode);
int security_old_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr, const char **name,
void **value, size_t *len);
diff --git a/security/security.c b/security/security.c
index 565bc9b67276..70bfebada024 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1033,6 +1033,15 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
}
EXPORT_SYMBOL(security_inode_init_security);
+int
+security_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct inode *context_inode)
+{
+ return call_int_hook(inode_init_security_anon, 0, inode, name,
+ context_inode);
+}
+
int security_old_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr, const char **name,
void **value, size_t *len)
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v4 1/3] Add a new LSM-supporting anonymous inode interface
2020-03-26 20:06 ` [PATCH v4 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
@ 2020-03-27 13:40 ` Stephen Smalley
0 siblings, 0 replies; 60+ messages in thread
From: Stephen Smalley @ 2020-03-27 13:40 UTC (permalink / raw)
To: Daniel Colascione, timmurray, selinux, linux-security-module,
linux-fsdevel, linux-kernel, kvm, viro, paul, nnk, lokeshgidra,
jmorris
On 3/26/20 4:06 PM, Daniel Colascione wrote:
> This change adds two new functions, anon_inode_getfile_secure and
> anon_inode_getfd_secure, that create anonymous-node files with
> individual non-S_PRIVATE inodes to which security modules can apply
> policy. Existing callers continue using the original singleton-inode
> kind of anonymous-inode file. We can transition anonymous inode users
> to the new kind of anonymous inode in individual patches for the sake
> of bisection and review.
>
> The new functions accept an optional context_inode parameter that
> callers can use to provide additional contextual information to
> security modules, e.g., indicating that one anonymous struct file is a
> logical child of another, allowing a security model to propagate
> security information from one to the other.
>
> Signed-off-by: Daniel Colascione <dancol@google.com>
> ---
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index 89714308c25b..024059e333dc 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -55,75 +55,135 @@ static struct file_system_type anon_inode_fs_type = {
> +static struct inode *anon_inode_make_secure_inode(
> + const char *name,
> + const struct inode *context_inode,
> + const struct file_operations *fops)
fops argument can be removed here too, unused now by this function.
> /**
> - * anon_inode_getfd - creates a new file instance by hooking it up to an
> - * anonymous inode, and a dentry that describe the "class"
> - * of the file
> + * anon_inode_getfile_secure - creates a new file instance by hooking
> + * it up to a new anonymous inode and a
> + * dentry that describe the "class" of the
> + * file. Make it possible to use security
> + * modules to control access to the
> + * new file.
> *
> * @name: [in] name of the "class" of the new file
> * @fops: [in] file operations for the new file
> * @priv: [in] private data for the new file (will be file's private_data)
> - * @flags: [in] flags
> + * @flags: [in] flags for the file
> + * @anon_inode_flags: [in] flags for anon_inode*
anon_inode_flags leftover from prior version of the patch, not an
argument in the current code. Likewise, the "for the file" addendum to
the @flags argument description is a leftover and not needed.
> + * Creates a new file by hooking it on an unspecified inode. This is
> + * useful for files that do not need to have a full-fledged inode in
> + * order to operate correctly. All the files created with
> + * anon_inode_getfile_secure() will have distinct inodes, avoiding
> + * code duplication for the file/inode/dentry setup.
The two preceding sentences directly contradict each other.
> +/**
> + * anon_inode_getfile - creates a new file instance by hooking it up
> + * to an anonymous inode and a dentry that
> + * describe the "class" of the file.
This would be identical to the original except for different word
wrapping. Probably a leftover from prior version of the patch where you
were modifying the existing interface. Recommend reverting such changes
to minimize unnecessary noise in your diff and meke it easier to tell
what changes are real.
> + *
> + * @name: [in] name of the "class" of the new file
> + * @fops: [in] file operations for the new file
> + * @priv: [in] private data for the new file (will be file's private_data)
> + * @flags: [in] flags for the file
> *
> - * Creates a new file by hooking it on a single inode. This is useful for files
> + * Creates a new file by hooking it on an unspecified inode. This is useful for files
Unnecessary difference here; this interface does use a single inode.
> @@ -146,6 +207,57 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops,
> put_unused_fd(fd);
> return error;
> }
> +
> +/**
> + * anon_inode_getfd_secure - creates a new file instance by hooking it
> + * up to a new anonymous inode and a dentry
> + * that describe the "class" of the file.
> + * Make it possible to use security modules
> + * to control access to the new file.
> + *
> + * @name: [in] name of the "class" of the new file
> + * @fops: [in] file operations for the new file
> + * @priv: [in] private data for the new file (will be file's private_data)
> + * @flags: [in] flags
> + *
> + * Creates a new file by hooking it on an unspecified inode. This is
> + * useful for files that do not need to have a full-fledged inode in
> + * order to operate correctly. All the files created with
> + * anon_inode_getfile_secure() will have distinct inodes, avoiding
> + * code duplication for the file/inode/dentry setup.
The two preceding sentences contradict each other.
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v4 2/3] Teach SELinux about anonymous inodes
2020-03-26 20:06 ` [PATCH v4 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
2020-03-26 20:06 ` [PATCH v4 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
@ 2020-03-26 20:06 ` Daniel Colascione
2020-03-27 13:41 ` Stephen Smalley
2020-03-26 20:06 ` [PATCH v4 3/3] Wire UFFD up to SELinux Daniel Colascione
2020-04-01 21:39 ` [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
3 siblings, 1 reply; 60+ messages in thread
From: Daniel Colascione @ 2020-03-26 20:06 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra, jmorris
Cc: Daniel Colascione
This change uses the anon_inodes and LSM infrastructure introduced in
the previous patch to give SELinux the ability to control
anonymous-inode files that are created using the new _secure()
anon_inodes functions.
A SELinux policy author detects and controls these anonymous inodes by
adding a name-based type_transition rule that assigns a new security
type to anonymous-inode files created in some domain. The name used
for the name-based transition is the name associated with the
anonymous inode for file listings --- e.g., "[userfaultfd]" or
"[perf_event]".
Example:
type uffd_t;
type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]";
allow sysadm_t uffd_t:anon_inode { create };
(The next patch in this series is necessary for making userfaultfd
support this new interface. The example above is just
for exposition.)
Signed-off-by: Daniel Colascione <dancol@google.com>
---
security/selinux/hooks.c | 53 +++++++++++++++++++++++++++++
security/selinux/include/classmap.h | 2 ++
2 files changed, 55 insertions(+)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 1659b59fb5d7..6f7222d2e404 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2915,6 +2915,58 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
return 0;
}
+static int selinux_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct inode *context_inode)
+{
+ const struct task_security_struct *tsec = selinux_cred(current_cred());
+ struct common_audit_data ad;
+ struct inode_security_struct *isec;
+ int rc;
+
+ if (unlikely(!selinux_state.initialized))
+ return 0;
+
+ isec = selinux_inode(inode);
+
+ /*
+ * We only get here once per ephemeral inode. The inode has
+ * been initialized via inode_alloc_security but is otherwise
+ * untouched.
+ */
+
+ if (context_inode) {
+ struct inode_security_struct *context_isec =
+ selinux_inode(context_inode);
+ isec->sclass = context_isec->sclass;
+ isec->sid = context_isec->sid;
+ } else {
+ isec->sclass = SECCLASS_ANON_INODE;
+ rc = security_transition_sid(
+ &selinux_state, tsec->sid, tsec->sid,
+ isec->sclass, name, &isec->sid);
+ if (rc)
+ return rc;
+ }
+
+ isec->initialized = LABEL_INITIALIZED;
+
+ /*
+ * Now that we've initialized security, check whether we're
+ * allowed to actually create this type of anonymous inode.
+ */
+
+ ad.type = LSM_AUDIT_DATA_INODE;
+ ad.u.inode = inode;
+
+ return avc_has_perm(&selinux_state,
+ tsec->sid,
+ isec->sid,
+ isec->sclass,
+ FILE__CREATE,
+ &ad);
+}
+
static int selinux_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode)
{
return may_create(dir, dentry, SECCLASS_FILE);
@@ -6923,6 +6975,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security),
LSM_HOOK_INIT(inode_init_security, selinux_inode_init_security),
+ LSM_HOOK_INIT(inode_init_security_anon, selinux_inode_init_security_anon),
LSM_HOOK_INIT(inode_create, selinux_inode_create),
LSM_HOOK_INIT(inode_link, selinux_inode_link),
LSM_HOOK_INIT(inode_unlink, selinux_inode_unlink),
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 986f3ac14282..263750b6aaac 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -248,6 +248,8 @@ struct security_class_mapping secclass_map[] = {
{"open", "cpu", "kernel", "tracepoint", "read", "write"} },
{ "lockdown",
{ "integrity", "confidentiality", NULL } },
+ { "anon_inode",
+ { COMMON_FILE_PERMS, NULL } },
{ NULL }
};
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v4 2/3] Teach SELinux about anonymous inodes
2020-03-26 20:06 ` [PATCH v4 2/3] Teach SELinux about anonymous inodes Daniel Colascione
@ 2020-03-27 13:41 ` Stephen Smalley
0 siblings, 0 replies; 60+ messages in thread
From: Stephen Smalley @ 2020-03-27 13:41 UTC (permalink / raw)
To: Daniel Colascione, timmurray, selinux, linux-security-module,
linux-fsdevel, linux-kernel, kvm, viro, paul, nnk, lokeshgidra,
jmorris
On 3/26/20 4:06 PM, Daniel Colascione wrote:
> This change uses the anon_inodes and LSM infrastructure introduced in
> the previous patch to give SELinux the ability to control
> anonymous-inode files that are created using the new _secure()
> anon_inodes functions.
>
> A SELinux policy author detects and controls these anonymous inodes by
> adding a name-based type_transition rule that assigns a new security
> type to anonymous-inode files created in some domain. The name used
> for the name-based transition is the name associated with the
> anonymous inode for file listings --- e.g., "[userfaultfd]" or
> "[perf_event]".
>
> Example:
>
> type uffd_t;
> type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]";
> allow sysadm_t uffd_t:anon_inode { create };
>
> (The next patch in this series is necessary for making userfaultfd
> support this new interface. The example above is just
> for exposition.)
>
> Signed-off-by: Daniel Colascione <dancol@google.com>
Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v4 3/3] Wire UFFD up to SELinux
2020-03-26 20:06 ` [PATCH v4 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
2020-03-26 20:06 ` [PATCH v4 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
2020-03-26 20:06 ` [PATCH v4 2/3] Teach SELinux about anonymous inodes Daniel Colascione
@ 2020-03-26 20:06 ` Daniel Colascione
2020-04-01 21:39 ` [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
3 siblings, 0 replies; 60+ messages in thread
From: Daniel Colascione @ 2020-03-26 20:06 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra, jmorris
Cc: Daniel Colascione
This change gives userfaultfd file descriptors a real security
context, allowing policy to act on them.
Signed-off-by: Daniel Colascione <dancol@google.com>
---
fs/userfaultfd.c | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 37df7c9eedb1..78ff5d898733 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -76,6 +76,8 @@ struct userfaultfd_ctx {
bool mmap_changing;
/* mm with one ore more vmas attached to this userfaultfd_ctx */
struct mm_struct *mm;
+ /* The inode that owns this context --- not a strong reference. */
+ const struct inode *owner;
};
struct userfaultfd_fork_ctx {
@@ -1022,8 +1024,10 @@ static int resolve_userfault_fork(struct userfaultfd_ctx *ctx,
{
int fd;
- fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, new,
- O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS));
+ fd = anon_inode_getfd_secure(
+ "[userfaultfd]", &userfaultfd_fops, new,
+ O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS),
+ ctx->owner);
if (fd < 0)
return fd;
@@ -1945,6 +1949,7 @@ static void init_once_userfaultfd_ctx(void *mem)
SYSCALL_DEFINE1(userfaultfd, int, flags)
{
+ struct file *file;
struct userfaultfd_ctx *ctx;
int fd;
@@ -1974,8 +1979,25 @@ SYSCALL_DEFINE1(userfaultfd, int, flags)
/* prevent the mm struct to be freed */
mmgrab(ctx->mm);
- fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, ctx,
- O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS));
+ file = anon_inode_getfile_secure(
+ "[userfaultfd]", &userfaultfd_fops, ctx,
+ O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS),
+ NULL);
+ if (IS_ERR(file)) {
+ fd = PTR_ERR(file);
+ goto out;
+ }
+
+ fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
+ if (fd < 0) {
+ fput(file);
+ goto out;
+ }
+
+ ctx->owner = file_inode(file);
+ fd_install(fd, file);
+
+out:
if (fd < 0) {
mmdrop(ctx->mm);
kmem_cache_free(userfaultfd_ctx_cachep, ctx);
--
2.25.1.696.g5e7596f4ac-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD
2020-03-26 20:06 ` [PATCH v4 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
` (2 preceding siblings ...)
2020-03-26 20:06 ` [PATCH v4 3/3] Wire UFFD up to SELinux Daniel Colascione
@ 2020-04-01 21:39 ` Daniel Colascione
2020-04-01 21:39 ` [PATCH v5 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
` (4 more replies)
3 siblings, 5 replies; 60+ messages in thread
From: Daniel Colascione @ 2020-04-01 21:39 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra, jmorris
Cc: Daniel Colascione
Userfaultfd in unprivileged contexts could be potentially very
useful. We'd like to harden userfaultfd to make such unprivileged use
less risky. This patch series allows SELinux to manage userfaultfd
file descriptors and in the future, other kinds of
anonymous-inode-based file descriptor. SELinux policy authors can
apply policy types to anonymous inodes by providing name-based
transition rules keyed off the anonymous inode internal name (
"[userfaultfd]" in the case of userfaultfd(2) file descriptors) and
applying policy to the new SIDs thus produced.
Inside the kernel, a pair of new anon_inodes interface,
anon_inode_getfile_secure and anon_inode_getfd_secure, allow callers
to opt into this SELinux management. In this new "secure" mode,
anon_inodes creates new ephemeral inodes for anonymous file objects
instead of reusing the normal anon_inodes singleton dummy inode. A new
LSM hook gives security modules an opportunity to configure and veto
these ephemeral inodes.
This patch series is one of two fork of [1] and is an
alternative to [2].
The primary difference between the two patch series is that this
partch series creates a unique inode for each "secure" anonymous
inode, while the other patch series ([2]) continues using the
singleton dummy anonymous inode and adds a way to attach SELinux
security information directly to file objects.
I prefer the approach in this patch series because 1) it's a smaller
patch than [2], and 2) it produces a more regular security
architecture: in this patch series, secure anonymous inodes aren't
S_PRIVATE and they maintain the SELinux property that the label for a
file is in its inode. We do need an additional inode per anonymous
file, but per-struct-file inode creation doesn't seem to be a problem
for pipes and sockets.
The previous version of this feature ([1]) created a new SELinux
security class for userfaultfd file descriptors. This version adopts
the generic transition-based approach of [2].
This patch series also differs from [2] in that it doesn't affect all
anonymous inodes right away --- instead requiring anon_inodes callers
to opt in --- but this difference isn't one of basic approach. The
important question to resolve is whether we should be creating new
inodes or enhancing per-file data.
Changes from the first version of the patch:
- Removed some error checks
- Defined a new anon_inode SELinux class to resolve the
ambiguity in [3]
- Inherit sclass as well as descriptor from context inode
Changes from the second version of the patch:
- Fixed example policy in the commit message to reflect the use of
the new anon_inode class.
Changes from the third version of the patch:
- Dropped the fops parameter to the LSM hook
- Documented hook parameters
- Fixed incorrect class used for SELinux transition
- Removed stray UFFD changed early in the series
- Removed a redundant ERR_PTR(PTR_ERR())
Changes from the fourth version of the patch:
- Removed an unused parameter from an internal function
- Fixed function documentation
[1] https://lore.kernel.org/lkml/20200211225547.235083-1-dancol@google.com/
[2] https://lore.kernel.org/linux-fsdevel/20200213194157.5877-1-sds@tycho.nsa.gov/
[3] https://lore.kernel.org/lkml/23f725ca-5b5a-5938-fcc8-5bbbfc9ba9bc@tycho.nsa.gov/
Daniel Colascione (3):
Add a new LSM-supporting anonymous inode interface
Teach SELinux about anonymous inodes
Wire UFFD up to SELinux
fs/anon_inodes.c | 191 ++++++++++++++++++++++------
fs/userfaultfd.c | 30 ++++-
include/linux/anon_inodes.h | 13 ++
include/linux/lsm_hooks.h | 11 ++
include/linux/security.h | 3 +
security/security.c | 9 ++
security/selinux/hooks.c | 53 ++++++++
security/selinux/include/classmap.h | 2 +
8 files changed, 267 insertions(+), 45 deletions(-)
--
2.26.0.rc2.310.g2932bb562d-goog
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH v5 1/3] Add a new LSM-supporting anonymous inode interface
2020-04-01 21:39 ` [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
@ 2020-04-01 21:39 ` Daniel Colascione
2020-05-07 16:02 ` James Morris
2020-08-04 21:22 ` Eric Biggers
2020-04-01 21:39 ` [PATCH v5 2/3] Teach SELinux about anonymous inodes Daniel Colascione
` (3 subsequent siblings)
4 siblings, 2 replies; 60+ messages in thread
From: Daniel Colascione @ 2020-04-01 21:39 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra, jmorris
Cc: Daniel Colascione
This change adds two new functions, anon_inode_getfile_secure and
anon_inode_getfd_secure, that create anonymous-node files with
individual non-S_PRIVATE inodes to which security modules can apply
policy. Existing callers continue using the original singleton-inode
kind of anonymous-inode file. We can transition anonymous inode users
to the new kind of anonymous inode in individual patches for the sake
of bisection and review.
The new functions accept an optional context_inode parameter that
callers can use to provide additional contextual information to
security modules, e.g., indicating that one anonymous struct file is a
logical child of another, allowing a security model to propagate
security information from one to the other.
Signed-off-by: Daniel Colascione <dancol@google.com>
---
fs/anon_inodes.c | 191 ++++++++++++++++++++++++++++--------
include/linux/anon_inodes.h | 13 +++
include/linux/lsm_hooks.h | 11 +++
include/linux/security.h | 3 +
security/security.c | 9 ++
5 files changed, 186 insertions(+), 41 deletions(-)
diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 89714308c25b..f87f221167cf 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -55,61 +55,108 @@ static struct file_system_type anon_inode_fs_type = {
.kill_sb = kill_anon_super,
};
-/**
- * anon_inode_getfile - creates a new file instance by hooking it up to an
- * anonymous inode, and a dentry that describe the "class"
- * of the file
- *
- * @name: [in] name of the "class" of the new file
- * @fops: [in] file operations for the new file
- * @priv: [in] private data for the new file (will be file's private_data)
- * @flags: [in] flags
- *
- * Creates a new file by hooking it on a single inode. This is useful for files
- * that do not need to have a full-fledged inode in order to operate correctly.
- * All the files created with anon_inode_getfile() will share a single inode,
- * hence saving memory and avoiding code duplication for the file/inode/dentry
- * setup. Returns the newly created file* or an error pointer.
- */
-struct file *anon_inode_getfile(const char *name,
- const struct file_operations *fops,
- void *priv, int flags)
+static struct inode *anon_inode_make_secure_inode(
+ const char *name,
+ const struct inode *context_inode)
+{
+ struct inode *inode;
+ const struct qstr qname = QSTR_INIT(name, strlen(name));
+ int error;
+
+ inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
+ if (IS_ERR(inode))
+ return inode;
+ inode->i_flags &= ~S_PRIVATE;
+ error = security_inode_init_security_anon(
+ inode, &qname, context_inode);
+ if (error) {
+ iput(inode);
+ return ERR_PTR(error);
+ }
+ return inode;
+}
+
+struct file *_anon_inode_getfile(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode,
+ bool secure)
{
+ struct inode *inode;
struct file *file;
- if (IS_ERR(anon_inode_inode))
- return ERR_PTR(-ENODEV);
+ if (secure) {
+ inode = anon_inode_make_secure_inode(
+ name, context_inode);
+ if (IS_ERR(inode))
+ return ERR_PTR(PTR_ERR(inode));
+ } else {
+ inode = anon_inode_inode;
+ if (IS_ERR(inode))
+ return ERR_PTR(-ENODEV);
+ /*
+ * We know the anon_inode inode count is always
+ * greater than zero, so ihold() is safe.
+ */
+ ihold(inode);
+ }
- if (fops->owner && !try_module_get(fops->owner))
- return ERR_PTR(-ENOENT);
+ if (fops->owner && !try_module_get(fops->owner)) {
+ file = ERR_PTR(-ENOENT);
+ goto err;
+ }
- /*
- * We know the anon_inode inode count is always greater than zero,
- * so ihold() is safe.
- */
- ihold(anon_inode_inode);
- file = alloc_file_pseudo(anon_inode_inode, anon_inode_mnt, name,
+ file = alloc_file_pseudo(inode, anon_inode_mnt, name,
flags & (O_ACCMODE | O_NONBLOCK), fops);
if (IS_ERR(file))
goto err;
- file->f_mapping = anon_inode_inode->i_mapping;
+ file->f_mapping = inode->i_mapping;
file->private_data = priv;
return file;
err:
- iput(anon_inode_inode);
+ iput(inode);
module_put(fops->owner);
return file;
}
-EXPORT_SYMBOL_GPL(anon_inode_getfile);
/**
- * anon_inode_getfd - creates a new file instance by hooking it up to an
- * anonymous inode, and a dentry that describe the "class"
- * of the file
+ * anon_inode_getfile_secure - creates a new file instance by hooking
+ * it up to a new anonymous inode and a
+ * dentry that describe the "class" of the
+ * file. Make it possible to use security
+ * modules to control access to the
+ * new file.
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags
+ *
+ * Creates a new file by hooking it on an unspecified inode. This is
+ * useful for files that do not need to have a full-fledged filesystem
+ * to operate correctly. All the files created with
+ * anon_inode_getfile_secure() will have distinct inodes, avoiding
+ * code duplication for the file/inode/dentry setup. Returns the
+ * newly created file* or an error pointer.
+ */
+struct file *anon_inode_getfile_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode)
+{
+ return _anon_inode_getfile(
+ name, fops, priv, flags, context_inode, true);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfile_secure);
+
+/**
+ * anon_inode_getfile - creates a new file instance by hooking it up to an
+ * anonymous inode, and a dentry that describe the "class"
+ * of the file
*
* @name: [in] name of the "class" of the new file
* @fops: [in] file operations for the new file
@@ -118,12 +165,23 @@ EXPORT_SYMBOL_GPL(anon_inode_getfile);
*
* Creates a new file by hooking it on a single inode. This is useful for files
* that do not need to have a full-fledged inode in order to operate correctly.
- * All the files created with anon_inode_getfd() will share a single inode,
+ * All the files created with anon_inode_getfile() will share a single inode,
* hence saving memory and avoiding code duplication for the file/inode/dentry
- * setup. Returns new descriptor or an error code.
+ * setup. Returns the newly created file* or an error pointer.
*/
-int anon_inode_getfd(const char *name, const struct file_operations *fops,
- void *priv, int flags)
+struct file *anon_inode_getfile(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags)
+{
+ return _anon_inode_getfile(name, fops, priv, flags, NULL, false);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfile);
+
+static int _anon_inode_getfd(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode,
+ bool secure)
{
int error, fd;
struct file *file;
@@ -133,7 +191,8 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops,
return error;
fd = error;
- file = anon_inode_getfile(name, fops, priv, flags);
+ file = _anon_inode_getfile(name, fops, priv, flags, context_inode,
+ secure);
if (IS_ERR(file)) {
error = PTR_ERR(file);
goto err_put_unused_fd;
@@ -146,6 +205,57 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops,
put_unused_fd(fd);
return error;
}
+
+/**
+ * anon_inode_getfd_secure - creates a new file instance by hooking it
+ * up to a new anonymous inode and a dentry
+ * that describe the "class" of the file.
+ * Make it possible to use security modules
+ * to control access to the new file.
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags
+ *
+ * Creates a new file by hooking it on an unspecified inode. This is
+ * useful for files that do not need to have a full-fledged filesystem
+ * to operate correctly. All the files created with
+ * anon_inode_getfile_secure() will have distinct inodes, avoiding
+ * code duplication for the file/inode/dentry setup. Returns a newly
+ * created file descriptor or an error code.
+ */
+int anon_inode_getfd_secure(const char *name, const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode)
+{
+ return _anon_inode_getfd(name, fops, priv, flags,
+ context_inode, true);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfd_secure);
+
+/**
+ * anon_inode_getfd - creates a new file instance by hooking it up to
+ * an anonymous inode and a dentry that describe
+ * the "class" of the file
+ *
+ * @name: [in] name of the "class" of the new file
+ * @fops: [in] file operations for the new file
+ * @priv: [in] private data for the new file (will be file's private_data)
+ * @flags: [in] flags
+ *
+ * Creates a new file by hooking it on a single inode. This is
+ * useful for files that do not need to have a full-fledged inode in
+ * order to operate correctly. All the files created with
+ * anon_inode_getfile() will use the same singleton inode, reducing
+ * memory use and avoiding code duplication for the file/inode/dentry
+ * setup. Returns a newly created file descriptor or an error code.
+ */
+int anon_inode_getfd(const char *name, const struct file_operations *fops,
+ void *priv, int flags)
+{
+ return _anon_inode_getfd(name, fops, priv, flags, NULL, false);
+}
EXPORT_SYMBOL_GPL(anon_inode_getfd);
static int __init anon_inode_init(void)
@@ -162,4 +272,3 @@ static int __init anon_inode_init(void)
}
fs_initcall(anon_inode_init);
-
diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
index d0d7d96261ad..67bd85d92dca 100644
--- a/include/linux/anon_inodes.h
+++ b/include/linux/anon_inodes.h
@@ -10,12 +10,25 @@
#define _LINUX_ANON_INODES_H
struct file_operations;
+struct inode;
+
+struct file *anon_inode_getfile_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode);
struct file *anon_inode_getfile(const char *name,
const struct file_operations *fops,
void *priv, int flags);
+
+int anon_inode_getfd_secure(const char *name,
+ const struct file_operations *fops,
+ void *priv, int flags,
+ const struct inode *context_inode);
+
int anon_inode_getfd(const char *name, const struct file_operations *fops,
void *priv, int flags);
+
#endif /* _LINUX_ANON_INODES_H */
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 20d8cf194fb7..5434c1d285b2 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -215,6 +215,13 @@
* Returns 0 if @name and @value have been successfully set,
* -EOPNOTSUPP if no security attribute is needed, or
* -ENOMEM on memory allocation failure.
+ * @inode_init_security_anon:
+ * Set up a secure anonymous inode.
+ * @inode contains the inode structure
+ * @name name of the anonymous inode class
+ * @context_inode optional related inode
+ * Returns 0 on success. Returns -EPERM if the security module denies
+ * the creation of this inode.
* @inode_create:
* Check permission to create a regular file.
* @dir contains inode structure of the parent of the new file.
@@ -1552,6 +1559,9 @@ union security_list_options {
const struct qstr *qstr,
const char **name, void **value,
size_t *len);
+ int (*inode_init_security_anon)(struct inode *inode,
+ const struct qstr *name,
+ const struct inode *context_inode);
int (*inode_create)(struct inode *dir, struct dentry *dentry,
umode_t mode);
int (*inode_link)(struct dentry *old_dentry, struct inode *dir,
@@ -1884,6 +1894,7 @@ struct security_hook_heads {
struct hlist_head inode_alloc_security;
struct hlist_head inode_free_security;
struct hlist_head inode_init_security;
+ struct hlist_head inode_init_security_anon;
struct hlist_head inode_create;
struct hlist_head inode_link;
struct hlist_head inode_unlink;
diff --git a/include/linux/security.h b/include/linux/security.h
index 64b19f050343..2108c3ce0666 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -320,6 +320,9 @@ void security_inode_free(struct inode *inode);
int security_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr,
initxattrs initxattrs, void *fs_data);
+int security_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct inode *context_inode);
int security_old_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr, const char **name,
void **value, size_t *len);
diff --git a/security/security.c b/security/security.c
index 565bc9b67276..70bfebada024 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1033,6 +1033,15 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
}
EXPORT_SYMBOL(security_inode_init_security);
+int
+security_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct inode *context_inode)
+{
+ return call_int_hook(inode_init_security_anon, 0, inode, name,
+ context_inode);
+}
+
int security_old_inode_init_security(struct inode *inode, struct inode *dir,
const struct qstr *qstr, const char **name,
void **value, size_t *len)
--
2.26.0.rc2.310.g2932bb562d-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v5 1/3] Add a new LSM-supporting anonymous inode interface
2020-04-01 21:39 ` [PATCH v5 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
@ 2020-05-07 16:02 ` James Morris
2020-08-04 21:22 ` Eric Biggers
1 sibling, 0 replies; 60+ messages in thread
From: James Morris @ 2020-05-07 16:02 UTC (permalink / raw)
To: Daniel Colascione, Al Viro, Andrew Morton
Cc: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, Paul Moore, nnk, Stephen Smalley, lokeshgidra
On Wed, 1 Apr 2020, Daniel Colascione wrote:
> This change adds two new functions, anon_inode_getfile_secure and
> anon_inode_getfd_secure, that create anonymous-node files with
> individual non-S_PRIVATE inodes to which security modules can apply
> policy. Existing callers continue using the original singleton-inode
> kind of anonymous-inode file. We can transition anonymous inode users
> to the new kind of anonymous inode in individual patches for the sake
> of bisection and review.
>
> The new functions accept an optional context_inode parameter that
> callers can use to provide additional contextual information to
> security modules, e.g., indicating that one anonymous struct file is a
> logical child of another, allowing a security model to propagate
> security information from one to the other.
>
> Signed-off-by: Daniel Colascione <dancol@google.com>
Al, Andrew, wondering if you could look at these anon inode changes
before we merge this?
> ---
> fs/anon_inodes.c | 191 ++++++++++++++++++++++++++++--------
> include/linux/anon_inodes.h | 13 +++
> include/linux/lsm_hooks.h | 11 +++
> include/linux/security.h | 3 +
> security/security.c | 9 ++
> 5 files changed, 186 insertions(+), 41 deletions(-)
>
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index 89714308c25b..f87f221167cf 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -55,61 +55,108 @@ static struct file_system_type anon_inode_fs_type = {
> .kill_sb = kill_anon_super,
> };
>
> -/**
> - * anon_inode_getfile - creates a new file instance by hooking it up to an
> - * anonymous inode, and a dentry that describe the "class"
> - * of the file
> - *
> - * @name: [in] name of the "class" of the new file
> - * @fops: [in] file operations for the new file
> - * @priv: [in] private data for the new file (will be file's private_data)
> - * @flags: [in] flags
> - *
> - * Creates a new file by hooking it on a single inode. This is useful for files
> - * that do not need to have a full-fledged inode in order to operate correctly.
> - * All the files created with anon_inode_getfile() will share a single inode,
> - * hence saving memory and avoiding code duplication for the file/inode/dentry
> - * setup. Returns the newly created file* or an error pointer.
> - */
> -struct file *anon_inode_getfile(const char *name,
> - const struct file_operations *fops,
> - void *priv, int flags)
> +static struct inode *anon_inode_make_secure_inode(
> + const char *name,
> + const struct inode *context_inode)
> +{
> + struct inode *inode;
> + const struct qstr qname = QSTR_INIT(name, strlen(name));
> + int error;
> +
> + inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
> + if (IS_ERR(inode))
> + return inode;
> + inode->i_flags &= ~S_PRIVATE;
> + error = security_inode_init_security_anon(
> + inode, &qname, context_inode);
> + if (error) {
> + iput(inode);
> + return ERR_PTR(error);
> + }
> + return inode;
> +}
> +
> +struct file *_anon_inode_getfile(const char *name,
> + const struct file_operations *fops,
> + void *priv, int flags,
> + const struct inode *context_inode,
> + bool secure)
> {
> + struct inode *inode;
> struct file *file;
>
> - if (IS_ERR(anon_inode_inode))
> - return ERR_PTR(-ENODEV);
> + if (secure) {
> + inode = anon_inode_make_secure_inode(
> + name, context_inode);
> + if (IS_ERR(inode))
> + return ERR_PTR(PTR_ERR(inode));
> + } else {
> + inode = anon_inode_inode;
> + if (IS_ERR(inode))
> + return ERR_PTR(-ENODEV);
> + /*
> + * We know the anon_inode inode count is always
> + * greater than zero, so ihold() is safe.
> + */
> + ihold(inode);
> + }
>
> - if (fops->owner && !try_module_get(fops->owner))
> - return ERR_PTR(-ENOENT);
> + if (fops->owner && !try_module_get(fops->owner)) {
> + file = ERR_PTR(-ENOENT);
> + goto err;
> + }
>
> - /*
> - * We know the anon_inode inode count is always greater than zero,
> - * so ihold() is safe.
> - */
> - ihold(anon_inode_inode);
> - file = alloc_file_pseudo(anon_inode_inode, anon_inode_mnt, name,
> + file = alloc_file_pseudo(inode, anon_inode_mnt, name,
> flags & (O_ACCMODE | O_NONBLOCK), fops);
> if (IS_ERR(file))
> goto err;
>
> - file->f_mapping = anon_inode_inode->i_mapping;
> + file->f_mapping = inode->i_mapping;
>
> file->private_data = priv;
>
> return file;
>
> err:
> - iput(anon_inode_inode);
> + iput(inode);
> module_put(fops->owner);
> return file;
> }
> -EXPORT_SYMBOL_GPL(anon_inode_getfile);
>
> /**
> - * anon_inode_getfd - creates a new file instance by hooking it up to an
> - * anonymous inode, and a dentry that describe the "class"
> - * of the file
> + * anon_inode_getfile_secure - creates a new file instance by hooking
> + * it up to a new anonymous inode and a
> + * dentry that describe the "class" of the
> + * file. Make it possible to use security
> + * modules to control access to the
> + * new file.
> + *
> + * @name: [in] name of the "class" of the new file
> + * @fops: [in] file operations for the new file
> + * @priv: [in] private data for the new file (will be file's private_data)
> + * @flags: [in] flags
> + *
> + * Creates a new file by hooking it on an unspecified inode. This is
> + * useful for files that do not need to have a full-fledged filesystem
> + * to operate correctly. All the files created with
> + * anon_inode_getfile_secure() will have distinct inodes, avoiding
> + * code duplication for the file/inode/dentry setup. Returns the
> + * newly created file* or an error pointer.
> + */
> +struct file *anon_inode_getfile_secure(const char *name,
> + const struct file_operations *fops,
> + void *priv, int flags,
> + const struct inode *context_inode)
> +{
> + return _anon_inode_getfile(
> + name, fops, priv, flags, context_inode, true);
> +}
> +EXPORT_SYMBOL_GPL(anon_inode_getfile_secure);
> +
> +/**
> + * anon_inode_getfile - creates a new file instance by hooking it up to an
> + * anonymous inode, and a dentry that describe the "class"
> + * of the file
> *
> * @name: [in] name of the "class" of the new file
> * @fops: [in] file operations for the new file
> @@ -118,12 +165,23 @@ EXPORT_SYMBOL_GPL(anon_inode_getfile);
> *
> * Creates a new file by hooking it on a single inode. This is useful for files
> * that do not need to have a full-fledged inode in order to operate correctly.
> - * All the files created with anon_inode_getfd() will share a single inode,
> + * All the files created with anon_inode_getfile() will share a single inode,
> * hence saving memory and avoiding code duplication for the file/inode/dentry
> - * setup. Returns new descriptor or an error code.
> + * setup. Returns the newly created file* or an error pointer.
> */
> -int anon_inode_getfd(const char *name, const struct file_operations *fops,
> - void *priv, int flags)
> +struct file *anon_inode_getfile(const char *name,
> + const struct file_operations *fops,
> + void *priv, int flags)
> +{
> + return _anon_inode_getfile(name, fops, priv, flags, NULL, false);
> +}
> +EXPORT_SYMBOL_GPL(anon_inode_getfile);
> +
> +static int _anon_inode_getfd(const char *name,
> + const struct file_operations *fops,
> + void *priv, int flags,
> + const struct inode *context_inode,
> + bool secure)
> {
> int error, fd;
> struct file *file;
> @@ -133,7 +191,8 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops,
> return error;
> fd = error;
>
> - file = anon_inode_getfile(name, fops, priv, flags);
> + file = _anon_inode_getfile(name, fops, priv, flags, context_inode,
> + secure);
> if (IS_ERR(file)) {
> error = PTR_ERR(file);
> goto err_put_unused_fd;
> @@ -146,6 +205,57 @@ int anon_inode_getfd(const char *name, const struct file_operations *fops,
> put_unused_fd(fd);
> return error;
> }
> +
> +/**
> + * anon_inode_getfd_secure - creates a new file instance by hooking it
> + * up to a new anonymous inode and a dentry
> + * that describe the "class" of the file.
> + * Make it possible to use security modules
> + * to control access to the new file.
> + *
> + * @name: [in] name of the "class" of the new file
> + * @fops: [in] file operations for the new file
> + * @priv: [in] private data for the new file (will be file's private_data)
> + * @flags: [in] flags
> + *
> + * Creates a new file by hooking it on an unspecified inode. This is
> + * useful for files that do not need to have a full-fledged filesystem
> + * to operate correctly. All the files created with
> + * anon_inode_getfile_secure() will have distinct inodes, avoiding
> + * code duplication for the file/inode/dentry setup. Returns a newly
> + * created file descriptor or an error code.
> + */
> +int anon_inode_getfd_secure(const char *name, const struct file_operations *fops,
> + void *priv, int flags,
> + const struct inode *context_inode)
> +{
> + return _anon_inode_getfd(name, fops, priv, flags,
> + context_inode, true);
> +}
> +EXPORT_SYMBOL_GPL(anon_inode_getfd_secure);
> +
> +/**
> + * anon_inode_getfd - creates a new file instance by hooking it up to
> + * an anonymous inode and a dentry that describe
> + * the "class" of the file
> + *
> + * @name: [in] name of the "class" of the new file
> + * @fops: [in] file operations for the new file
> + * @priv: [in] private data for the new file (will be file's private_data)
> + * @flags: [in] flags
> + *
> + * Creates a new file by hooking it on a single inode. This is
> + * useful for files that do not need to have a full-fledged inode in
> + * order to operate correctly. All the files created with
> + * anon_inode_getfile() will use the same singleton inode, reducing
> + * memory use and avoiding code duplication for the file/inode/dentry
> + * setup. Returns a newly created file descriptor or an error code.
> + */
> +int anon_inode_getfd(const char *name, const struct file_operations *fops,
> + void *priv, int flags)
> +{
> + return _anon_inode_getfd(name, fops, priv, flags, NULL, false);
> +}
> EXPORT_SYMBOL_GPL(anon_inode_getfd);
>
> static int __init anon_inode_init(void)
> @@ -162,4 +272,3 @@ static int __init anon_inode_init(void)
> }
>
> fs_initcall(anon_inode_init);
> -
> diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
> index d0d7d96261ad..67bd85d92dca 100644
> --- a/include/linux/anon_inodes.h
> +++ b/include/linux/anon_inodes.h
> @@ -10,12 +10,25 @@
> #define _LINUX_ANON_INODES_H
>
> struct file_operations;
> +struct inode;
> +
> +struct file *anon_inode_getfile_secure(const char *name,
> + const struct file_operations *fops,
> + void *priv, int flags,
> + const struct inode *context_inode);
>
> struct file *anon_inode_getfile(const char *name,
> const struct file_operations *fops,
> void *priv, int flags);
> +
> +int anon_inode_getfd_secure(const char *name,
> + const struct file_operations *fops,
> + void *priv, int flags,
> + const struct inode *context_inode);
> +
> int anon_inode_getfd(const char *name, const struct file_operations *fops,
> void *priv, int flags);
>
> +
> #endif /* _LINUX_ANON_INODES_H */
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 20d8cf194fb7..5434c1d285b2 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -215,6 +215,13 @@
> * Returns 0 if @name and @value have been successfully set,
> * -EOPNOTSUPP if no security attribute is needed, or
> * -ENOMEM on memory allocation failure.
> + * @inode_init_security_anon:
> + * Set up a secure anonymous inode.
> + * @inode contains the inode structure
> + * @name name of the anonymous inode class
> + * @context_inode optional related inode
> + * Returns 0 on success. Returns -EPERM if the security module denies
> + * the creation of this inode.
> * @inode_create:
> * Check permission to create a regular file.
> * @dir contains inode structure of the parent of the new file.
> @@ -1552,6 +1559,9 @@ union security_list_options {
> const struct qstr *qstr,
> const char **name, void **value,
> size_t *len);
> + int (*inode_init_security_anon)(struct inode *inode,
> + const struct qstr *name,
> + const struct inode *context_inode);
> int (*inode_create)(struct inode *dir, struct dentry *dentry,
> umode_t mode);
> int (*inode_link)(struct dentry *old_dentry, struct inode *dir,
> @@ -1884,6 +1894,7 @@ struct security_hook_heads {
> struct hlist_head inode_alloc_security;
> struct hlist_head inode_free_security;
> struct hlist_head inode_init_security;
> + struct hlist_head inode_init_security_anon;
> struct hlist_head inode_create;
> struct hlist_head inode_link;
> struct hlist_head inode_unlink;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 64b19f050343..2108c3ce0666 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -320,6 +320,9 @@ void security_inode_free(struct inode *inode);
> int security_inode_init_security(struct inode *inode, struct inode *dir,
> const struct qstr *qstr,
> initxattrs initxattrs, void *fs_data);
> +int security_inode_init_security_anon(struct inode *inode,
> + const struct qstr *name,
> + const struct inode *context_inode);
> int security_old_inode_init_security(struct inode *inode, struct inode *dir,
> const struct qstr *qstr, const char **name,
> void **value, size_t *len);
> diff --git a/security/security.c b/security/security.c
> index 565bc9b67276..70bfebada024 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1033,6 +1033,15 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> }
> EXPORT_SYMBOL(security_inode_init_security);
>
> +int
> +security_inode_init_security_anon(struct inode *inode,
> + const struct qstr *name,
> + const struct inode *context_inode)
> +{
> + return call_int_hook(inode_init_security_anon, 0, inode, name,
> + context_inode);
> +}
> +
> int security_old_inode_init_security(struct inode *inode, struct inode *dir,
> const struct qstr *qstr, const char **name,
> void **value, size_t *len)
>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH v5 1/3] Add a new LSM-supporting anonymous inode interface
2020-04-01 21:39 ` [PATCH v5 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
2020-05-07 16:02 ` James Morris
@ 2020-08-04 21:22 ` Eric Biggers
1 sibling, 0 replies; 60+ messages in thread
From: Eric Biggers @ 2020-08-04 21:22 UTC (permalink / raw)
To: Daniel Colascione
Cc: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra, jmorris
On Wed, Apr 01, 2020 at 02:39:01PM -0700, Daniel Colascione wrote:
> This change adds two new functions, anon_inode_getfile_secure and
> anon_inode_getfd_secure, that create anonymous-node files with
> individual non-S_PRIVATE inodes to which security modules can apply
> policy. Existing callers continue using the original singleton-inode
> kind of anonymous-inode file. We can transition anonymous inode users
> to the new kind of anonymous inode in individual patches for the sake
> of bisection and review.
>
> The new functions accept an optional context_inode parameter that
> callers can use to provide additional contextual information to
> security modules, e.g., indicating that one anonymous struct file is a
> logical child of another, allowing a security model to propagate
> security information from one to the other.
>
> Signed-off-by: Daniel Colascione <dancol@google.com>
> ---
> fs/anon_inodes.c | 191 ++++++++++++++++++++++++++++--------
> include/linux/anon_inodes.h | 13 +++
> include/linux/lsm_hooks.h | 11 +++
> include/linux/security.h | 3 +
> security/security.c | 9 ++
> 5 files changed, 186 insertions(+), 41 deletions(-)
>
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index 89714308c25b..f87f221167cf 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -55,61 +55,108 @@ static struct file_system_type anon_inode_fs_type = {
> .kill_sb = kill_anon_super,
> };
>
> -/**
> - * anon_inode_getfile - creates a new file instance by hooking it up to an
> - * anonymous inode, and a dentry that describe the "class"
> - * of the file
> - *
> - * @name: [in] name of the "class" of the new file
> - * @fops: [in] file operations for the new file
> - * @priv: [in] private data for the new file (will be file's private_data)
> - * @flags: [in] flags
> - *
> - * Creates a new file by hooking it on a single inode. This is useful for files
> - * that do not need to have a full-fledged inode in order to operate correctly.
> - * All the files created with anon_inode_getfile() will share a single inode,
> - * hence saving memory and avoiding code duplication for the file/inode/dentry
> - * setup. Returns the newly created file* or an error pointer.
> - */
> -struct file *anon_inode_getfile(const char *name,
> - const struct file_operations *fops,
> - void *priv, int flags)
> +static struct inode *anon_inode_make_secure_inode(
> + const char *name,
> + const struct inode *context_inode)
> +{
> + struct inode *inode;
> + const struct qstr qname = QSTR_INIT(name, strlen(name));
> + int error;
> +
> + inode = alloc_anon_inode(anon_inode_mnt->mnt_sb);
> + if (IS_ERR(inode))
> + return inode;
> + inode->i_flags &= ~S_PRIVATE;
> + error = security_inode_init_security_anon(
> + inode, &qname, context_inode);
> + if (error) {
> + iput(inode);
> + return ERR_PTR(error);
> + }
> + return inode;
> +}
> +
> +struct file *_anon_inode_getfile(const char *name,
> + const struct file_operations *fops,
> + void *priv, int flags,
> + const struct inode *context_inode,
> + bool secure)
Unnecessarily global function.
> {
> + struct inode *inode;
> struct file *file;
>
> - if (IS_ERR(anon_inode_inode))
> - return ERR_PTR(-ENODEV);
> + if (secure) {
> + inode = anon_inode_make_secure_inode(
> + name, context_inode);
> + if (IS_ERR(inode))
> + return ERR_PTR(PTR_ERR(inode));
Use ERR_CAST(), not ERR_PTR(PTR_ERR()).
> /**
> - * anon_inode_getfd - creates a new file instance by hooking it up to an
> - * anonymous inode, and a dentry that describe the "class"
> - * of the file
> + * anon_inode_getfile_secure - creates a new file instance by hooking
> + * it up to a new anonymous inode and a
> + * dentry that describe the "class" of the
> + * file. Make it possible to use security
> + * modules to control access to the
> + * new file.
> + *
> + * @name: [in] name of the "class" of the new file
> + * @fops: [in] file operations for the new file
> + * @priv: [in] private data for the new file (will be file's private_data)
> + * @flags: [in] flags
> + *
> + * Creates a new file by hooking it on an unspecified inode. This is
> + * useful for files that do not need to have a full-fledged filesystem
> + * to operate correctly. All the files created with
> + * anon_inode_getfile_secure() will have distinct inodes, avoiding
> + * code duplication for the file/inode/dentry setup. Returns the
> + * newly created file* or an error pointer.
> + */
> +struct file *anon_inode_getfile_secure(const char *name,
> + const struct file_operations *fops,
> + void *priv, int flags,
> + const struct inode *context_inode)
Why copy-and-paste this long comment if it's not even updated to document the
new argument?
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 20d8cf194fb7..5434c1d285b2 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -215,6 +215,13 @@
> * Returns 0 if @name and @value have been successfully set,
> * -EOPNOTSUPP if no security attribute is needed, or
> * -ENOMEM on memory allocation failure.
> + * @inode_init_security_anon:
> + * Set up a secure anonymous inode.
> + * @inode contains the inode structure
> + * @name name of the anonymous inode class
> + * @context_inode optional related inode
> + * Returns 0 on success. Returns -EPERM if the security module denies
> + * the creation of this inode.
Shouldn't it be EACCES?
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v5 2/3] Teach SELinux about anonymous inodes
2020-04-01 21:39 ` [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
2020-04-01 21:39 ` [PATCH v5 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
@ 2020-04-01 21:39 ` Daniel Colascione
2020-04-01 21:39 ` [PATCH v5 3/3] Wire UFFD up to SELinux Daniel Colascione
` (2 subsequent siblings)
4 siblings, 0 replies; 60+ messages in thread
From: Daniel Colascione @ 2020-04-01 21:39 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra, jmorris
Cc: Daniel Colascione
This change uses the anon_inodes and LSM infrastructure introduced in
the previous patch to give SELinux the ability to control
anonymous-inode files that are created using the new _secure()
anon_inodes functions.
A SELinux policy author detects and controls these anonymous inodes by
adding a name-based type_transition rule that assigns a new security
type to anonymous-inode files created in some domain. The name used
for the name-based transition is the name associated with the
anonymous inode for file listings --- e.g., "[userfaultfd]" or
"[perf_event]".
Example:
type uffd_t;
type_transition sysadm_t sysadm_t : anon_inode uffd_t "[userfaultfd]";
allow sysadm_t uffd_t:anon_inode { create };
(The next patch in this series is necessary for making userfaultfd
support this new interface. The example above is just
for exposition.)
Signed-off-by: Daniel Colascione <dancol@google.com>
---
security/selinux/hooks.c | 53 +++++++++++++++++++++++++++++
security/selinux/include/classmap.h | 2 ++
2 files changed, 55 insertions(+)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 1659b59fb5d7..6f7222d2e404 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2915,6 +2915,58 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
return 0;
}
+static int selinux_inode_init_security_anon(struct inode *inode,
+ const struct qstr *name,
+ const struct inode *context_inode)
+{
+ const struct task_security_struct *tsec = selinux_cred(current_cred());
+ struct common_audit_data ad;
+ struct inode_security_struct *isec;
+ int rc;
+
+ if (unlikely(!selinux_state.initialized))
+ return 0;
+
+ isec = selinux_inode(inode);
+
+ /*
+ * We only get here once per ephemeral inode. The inode has
+ * been initialized via inode_alloc_security but is otherwise
+ * untouched.
+ */
+
+ if (context_inode) {
+ struct inode_security_struct *context_isec =
+ selinux_inode(context_inode);
+ isec->sclass = context_isec->sclass;
+ isec->sid = context_isec->sid;
+ } else {
+ isec->sclass = SECCLASS_ANON_INODE;
+ rc = security_transition_sid(
+ &selinux_state, tsec->sid, tsec->sid,
+ isec->sclass, name, &isec->sid);
+ if (rc)
+ return rc;
+ }
+
+ isec->initialized = LABEL_INITIALIZED;
+
+ /*
+ * Now that we've initialized security, check whether we're
+ * allowed to actually create this type of anonymous inode.
+ */
+
+ ad.type = LSM_AUDIT_DATA_INODE;
+ ad.u.inode = inode;
+
+ return avc_has_perm(&selinux_state,
+ tsec->sid,
+ isec->sid,
+ isec->sclass,
+ FILE__CREATE,
+ &ad);
+}
+
static int selinux_inode_create(struct inode *dir, struct dentry *dentry, umode_t mode)
{
return may_create(dir, dentry, SECCLASS_FILE);
@@ -6923,6 +6975,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(inode_free_security, selinux_inode_free_security),
LSM_HOOK_INIT(inode_init_security, selinux_inode_init_security),
+ LSM_HOOK_INIT(inode_init_security_anon, selinux_inode_init_security_anon),
LSM_HOOK_INIT(inode_create, selinux_inode_create),
LSM_HOOK_INIT(inode_link, selinux_inode_link),
LSM_HOOK_INIT(inode_unlink, selinux_inode_unlink),
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 986f3ac14282..263750b6aaac 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -248,6 +248,8 @@ struct security_class_mapping secclass_map[] = {
{"open", "cpu", "kernel", "tracepoint", "read", "write"} },
{ "lockdown",
{ "integrity", "confidentiality", NULL } },
+ { "anon_inode",
+ { COMMON_FILE_PERMS, NULL } },
{ NULL }
};
--
2.26.0.rc2.310.g2932bb562d-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH v5 3/3] Wire UFFD up to SELinux
2020-04-01 21:39 ` [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
2020-04-01 21:39 ` [PATCH v5 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione
2020-04-01 21:39 ` [PATCH v5 2/3] Teach SELinux about anonymous inodes Daniel Colascione
@ 2020-04-01 21:39 ` Daniel Colascione
2020-08-04 21:16 ` Eric Biggers
2020-04-13 13:29 ` [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
2020-06-04 3:56 ` James Morris
4 siblings, 1 reply; 60+ messages in thread
From: Daniel Colascione @ 2020-04-01 21:39 UTC (permalink / raw)
To: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra, jmorris
Cc: Daniel Colascione
This change gives userfaultfd file descriptors a real security
context, allowing policy to act on them.
Signed-off-by: Daniel Colascione <dancol@google.com>
---
fs/userfaultfd.c | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 37df7c9eedb1..78ff5d898733 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -76,6 +76,8 @@ struct userfaultfd_ctx {
bool mmap_changing;
/* mm with one ore more vmas attached to this userfaultfd_ctx */
struct mm_struct *mm;
+ /* The inode that owns this context --- not a strong reference. */
+ const struct inode *owner;
};
struct userfaultfd_fork_ctx {
@@ -1022,8 +1024,10 @@ static int resolve_userfault_fork(struct userfaultfd_ctx *ctx,
{
int fd;
- fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, new,
- O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS));
+ fd = anon_inode_getfd_secure(
+ "[userfaultfd]", &userfaultfd_fops, new,
+ O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS),
+ ctx->owner);
if (fd < 0)
return fd;
@@ -1945,6 +1949,7 @@ static void init_once_userfaultfd_ctx(void *mem)
SYSCALL_DEFINE1(userfaultfd, int, flags)
{
+ struct file *file;
struct userfaultfd_ctx *ctx;
int fd;
@@ -1974,8 +1979,25 @@ SYSCALL_DEFINE1(userfaultfd, int, flags)
/* prevent the mm struct to be freed */
mmgrab(ctx->mm);
- fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, ctx,
- O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS));
+ file = anon_inode_getfile_secure(
+ "[userfaultfd]", &userfaultfd_fops, ctx,
+ O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS),
+ NULL);
+ if (IS_ERR(file)) {
+ fd = PTR_ERR(file);
+ goto out;
+ }
+
+ fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
+ if (fd < 0) {
+ fput(file);
+ goto out;
+ }
+
+ ctx->owner = file_inode(file);
+ fd_install(fd, file);
+
+out:
if (fd < 0) {
mmdrop(ctx->mm);
kmem_cache_free(userfaultfd_ctx_cachep, ctx);
--
2.26.0.rc2.310.g2932bb562d-goog
^ permalink raw reply related [flat|nested] 60+ messages in thread* Re: [PATCH v5 3/3] Wire UFFD up to SELinux
2020-04-01 21:39 ` [PATCH v5 3/3] Wire UFFD up to SELinux Daniel Colascione
@ 2020-08-04 21:16 ` Eric Biggers
0 siblings, 0 replies; 60+ messages in thread
From: Eric Biggers @ 2020-08-04 21:16 UTC (permalink / raw)
To: Daniel Colascione
Cc: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, paul, nnk, sds, lokeshgidra, jmorris
On Wed, Apr 01, 2020 at 02:39:03PM -0700, Daniel Colascione wrote:
> This change gives userfaultfd file descriptors a real security
> context, allowing policy to act on them.
>
> Signed-off-by: Daniel Colascione <dancol@google.com>
> ---
> fs/userfaultfd.c | 30 ++++++++++++++++++++++++++----
> 1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index 37df7c9eedb1..78ff5d898733 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -76,6 +76,8 @@ struct userfaultfd_ctx {
> bool mmap_changing;
> /* mm with one ore more vmas attached to this userfaultfd_ctx */
> struct mm_struct *mm;
> + /* The inode that owns this context --- not a strong reference. */
> + const struct inode *owner;
> };
Adding this field seems wrong. There's no reference held to it, so it can only
be used if the caller holds a reference to the inode anyway. The only user of
this field is via userfafultfd_read(), so why not just use the inode of the
struct file passed to userfaultfd_read()?
> SYSCALL_DEFINE1(userfaultfd, int, flags)
> {
> + struct file *file;
> struct userfaultfd_ctx *ctx;
> int fd;
>
> @@ -1974,8 +1979,25 @@ SYSCALL_DEFINE1(userfaultfd, int, flags)
> /* prevent the mm struct to be freed */
> mmgrab(ctx->mm);
>
> - fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, ctx,
> - O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS));
> + file = anon_inode_getfile_secure(
> + "[userfaultfd]", &userfaultfd_fops, ctx,
> + O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS),
> + NULL);
> + if (IS_ERR(file)) {
> + fd = PTR_ERR(file);
> + goto out;
> + }
> +
> + fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC);
> + if (fd < 0) {
> + fput(file);
> + goto out;
> + }
> +
> + ctx->owner = file_inode(file);
> + fd_install(fd, file);
> +
> +out:
> if (fd < 0) {
> mmdrop(ctx->mm);
> kmem_cache_free(userfaultfd_ctx_cachep, ctx);
What's the point of anon_inode_getfile_secure()? anon_inode_getfd_secure()
would work here too.
- Eric
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD
2020-04-01 21:39 ` [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
` (2 preceding siblings ...)
2020-04-01 21:39 ` [PATCH v5 3/3] Wire UFFD up to SELinux Daniel Colascione
@ 2020-04-13 13:29 ` Daniel Colascione
2020-04-22 16:55 ` James Morris
2020-06-04 3:56 ` James Morris
4 siblings, 1 reply; 60+ messages in thread
From: Daniel Colascione @ 2020-04-13 13:29 UTC (permalink / raw)
To: Tim Murray, SElinux list, LSM List, Linux FS Devel, linux-kernel,
kvm, Al Viro, Paul Moore, Nick Kralevich, Stephen Smalley,
Lokesh Gidra, jmorris
On Wed, Apr 1, 2020 at 2:39 PM Daniel Colascione <dancol@google.com> wrote:
>
> Changes from the fourth version of the patch:
Is there anything else that needs to be done before merging this patch series?
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD
2020-04-13 13:29 ` [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
@ 2020-04-22 16:55 ` James Morris
2020-04-22 17:12 ` Casey Schaufler
2020-04-27 17:15 ` Casey Schaufler
0 siblings, 2 replies; 60+ messages in thread
From: James Morris @ 2020-04-22 16:55 UTC (permalink / raw)
To: Daniel Colascione
Cc: Tim Murray, SElinux list, LSM List, Linux FS Devel, linux-kernel,
kvm, Al Viro, Paul Moore, Nick Kralevich, Stephen Smalley,
Lokesh Gidra, John Johansen, Casey Schaufler
On Mon, 13 Apr 2020, Daniel Colascione wrote:
> On Wed, Apr 1, 2020 at 2:39 PM Daniel Colascione <dancol@google.com> wrote:
> >
> > Changes from the fourth version of the patch:
>
>
> Is there anything else that needs to be done before merging this patch series?
The vfs changes need review and signoff from the vfs folk, the SELinux
changes by either Paul or Stephen, and we also need signoff on the LSM
hooks from other major LSM authors (Casey and John, at a minimum).
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD
2020-04-22 16:55 ` James Morris
@ 2020-04-22 17:12 ` Casey Schaufler
2020-04-23 22:24 ` Casey Schaufler
2020-04-27 17:15 ` Casey Schaufler
1 sibling, 1 reply; 60+ messages in thread
From: Casey Schaufler @ 2020-04-22 17:12 UTC (permalink / raw)
To: James Morris, Daniel Colascione
Cc: Tim Murray, SElinux list, LSM List, Linux FS Devel, linux-kernel,
kvm, Al Viro, Paul Moore, Nick Kralevich, Stephen Smalley,
Lokesh Gidra, John Johansen, Casey Schaufler
On 4/22/2020 9:55 AM, James Morris wrote:
> On Mon, 13 Apr 2020, Daniel Colascione wrote:
>
>> On Wed, Apr 1, 2020 at 2:39 PM Daniel Colascione <dancol@google.com> wrote:
>>> Changes from the fourth version of the patch:
>>
>> Is there anything else that needs to be done before merging this patch series?
> The vfs changes need review and signoff from the vfs folk, the SELinux
> changes by either Paul or Stephen, and we also need signoff on the LSM
> hooks from other major LSM authors (Casey and John, at a minimum).
I haven't had the opportunity to test this relative to Smack.
It's unclear whether the change would impact security modules that
don't provide hooks for it. I will bump my priority on this, but it's
still going to be a bit before I can get to it.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD
2020-04-22 17:12 ` Casey Schaufler
@ 2020-04-23 22:24 ` Casey Schaufler
2020-04-27 16:18 ` Casey Schaufler
0 siblings, 1 reply; 60+ messages in thread
From: Casey Schaufler @ 2020-04-23 22:24 UTC (permalink / raw)
To: Daniel Colascione
Cc: James Morris, Tim Murray, SElinux list, LSM List, Linux FS Devel,
linux-kernel, kvm, Al Viro, Paul Moore, Nick Kralevich,
Stephen Smalley, Lokesh Gidra, John Johansen, Casey Schaufler
On 4/22/2020 10:12 AM, Casey Schaufler wrote:
> On 4/22/2020 9:55 AM, James Morris wrote:
>> On Mon, 13 Apr 2020, Daniel Colascione wrote:
>>
>>> On Wed, Apr 1, 2020 at 2:39 PM Daniel Colascione <dancol@google.com> wrote:
>>>> Changes from the fourth version of the patch:
>>> Is there anything else that needs to be done before merging this patch series?
Do you have a test case that exercises this feature?
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD
2020-04-23 22:24 ` Casey Schaufler
@ 2020-04-27 16:18 ` Casey Schaufler
2020-04-27 16:48 ` Stephen Smalley
0 siblings, 1 reply; 60+ messages in thread
From: Casey Schaufler @ 2020-04-27 16:18 UTC (permalink / raw)
To: Daniel Colascione
Cc: James Morris, Tim Murray, SElinux list, LSM List, Linux FS Devel,
linux-kernel, kvm, Al Viro, Paul Moore, Nick Kralevich,
Stephen Smalley, Lokesh Gidra, John Johansen, Casey Schaufler
On 4/23/2020 3:24 PM, Casey Schaufler wrote:
> On 4/22/2020 10:12 AM, Casey Schaufler wrote:
>> On 4/22/2020 9:55 AM, James Morris wrote:
>>> On Mon, 13 Apr 2020, Daniel Colascione wrote:
>>>
>>>> On Wed, Apr 1, 2020 at 2:39 PM Daniel Colascione <dancol@google.com> wrote:
>>>>> Changes from the fourth version of the patch:
>>>> Is there anything else that needs to be done before merging this patch series?
> Do you have a test case that exercises this feature?
I haven't heard anything back. What would cause this code to be executed?
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD
2020-04-27 16:18 ` Casey Schaufler
@ 2020-04-27 16:48 ` Stephen Smalley
2020-04-27 17:12 ` Casey Schaufler
2020-04-29 17:02 ` Stephen Smalley
0 siblings, 2 replies; 60+ messages in thread
From: Stephen Smalley @ 2020-04-27 16:48 UTC (permalink / raw)
To: Casey Schaufler
Cc: Daniel Colascione, James Morris, Tim Murray, SElinux list,
LSM List, Linux FS Devel, linux-kernel, kvm, Al Viro, Paul Moore,
Nick Kralevich, Stephen Smalley, Lokesh Gidra, John Johansen
On Mon, Apr 27, 2020 at 12:19 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 4/23/2020 3:24 PM, Casey Schaufler wrote:
> > On 4/22/2020 10:12 AM, Casey Schaufler wrote:
> >> On 4/22/2020 9:55 AM, James Morris wrote:
> >>> On Mon, 13 Apr 2020, Daniel Colascione wrote:
> >>>
> >>>> On Wed, Apr 1, 2020 at 2:39 PM Daniel Colascione <dancol@google.com> wrote:
> >>>>> Changes from the fourth version of the patch:
> >>>> Is there anything else that needs to be done before merging this patch series?
> > Do you have a test case that exercises this feature?
>
> I haven't heard anything back. What would cause this code to be executed?
See https://lore.kernel.org/selinux/513f6230-1fb3-dbb5-5f75-53cd02b91b28@tycho.nsa.gov/
for example.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD
2020-04-27 16:48 ` Stephen Smalley
@ 2020-04-27 17:12 ` Casey Schaufler
2020-04-29 17:02 ` Stephen Smalley
1 sibling, 0 replies; 60+ messages in thread
From: Casey Schaufler @ 2020-04-27 17:12 UTC (permalink / raw)
To: Stephen Smalley
Cc: Daniel Colascione, James Morris, Tim Murray, SElinux list,
LSM List, Linux FS Devel, linux-kernel, kvm, Al Viro, Paul Moore,
Nick Kralevich, Stephen Smalley, Lokesh Gidra, John Johansen
On 4/27/2020 9:48 AM, Stephen Smalley wrote:
> On Mon, Apr 27, 2020 at 12:19 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 4/23/2020 3:24 PM, Casey Schaufler wrote:
>>> On 4/22/2020 10:12 AM, Casey Schaufler wrote:
>>>> On 4/22/2020 9:55 AM, James Morris wrote:
>>>>> On Mon, 13 Apr 2020, Daniel Colascione wrote:
>>>>>
>>>>>> On Wed, Apr 1, 2020 at 2:39 PM Daniel Colascione <dancol@google.com> wrote:
>>>>>>> Changes from the fourth version of the patch:
>>>>>> Is there anything else that needs to be done before merging this patch series?
>>> Do you have a test case that exercises this feature?
>> I haven't heard anything back. What would cause this code to be executed?
> See https://lore.kernel.org/selinux/513f6230-1fb3-dbb5-5f75-53cd02b91b28@tycho.nsa.gov/
> for example.
Great. Thanks, that's what I needed. I'll Ack the patch set.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD
2020-04-27 16:48 ` Stephen Smalley
2020-04-27 17:12 ` Casey Schaufler
@ 2020-04-29 17:02 ` Stephen Smalley
1 sibling, 0 replies; 60+ messages in thread
From: Stephen Smalley @ 2020-04-29 17:02 UTC (permalink / raw)
To: Casey Schaufler
Cc: Daniel Colascione, James Morris, Tim Murray, SElinux list,
LSM List, Linux FS Devel, linux-kernel, kvm, Al Viro, Paul Moore,
Nick Kralevich, Stephen Smalley, Lokesh Gidra, John Johansen
On Mon, Apr 27, 2020 at 12:48 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Mon, Apr 27, 2020 at 12:19 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >
> > On 4/23/2020 3:24 PM, Casey Schaufler wrote:
> > > On 4/22/2020 10:12 AM, Casey Schaufler wrote:
> > >> On 4/22/2020 9:55 AM, James Morris wrote:
> > >>> On Mon, 13 Apr 2020, Daniel Colascione wrote:
> > >>>
> > >>>> On Wed, Apr 1, 2020 at 2:39 PM Daniel Colascione <dancol@google.com> wrote:
> > >>>>> Changes from the fourth version of the patch:
> > >>>> Is there anything else that needs to be done before merging this patch series?
> > > Do you have a test case that exercises this feature?
> >
> > I haven't heard anything back. What would cause this code to be executed?
>
> See https://lore.kernel.org/selinux/513f6230-1fb3-dbb5-5f75-53cd02b91b28@tycho.nsa.gov/
> for example.
NB The example cited above needs to be tweaked for changes in the
logic from the original RFC patch on which the example was
based. In particular, the userfaultfd CIL policy needs to be updated
to define and use the new anon_inode class and to allow create
permission as follows.
$ cat userfaultfd.cil
(class anon_inode ())
(classcommon anon_inode file)
(classorder (unordered anon_inode))
(type uffd_t)
; Label the UFFD with uffd_t; this can be specialized per domain
(typetransition unconfined_t unconfined_t anon_inode "[userfaultfd]" uffd_t)
(allow unconfined_t uffd_t (anon_inode (create)))
; Permit read() and ioctl() on the UFFD.
; Comment out if you want to test read or basic ioctl enforcement.
(allow unconfined_t uffd_t (anon_inode (read)))
(allow unconfined_t uffd_t (anon_inode (ioctl)))
; Uncomment one of the allowx lines below to test ioctl whitelisting.
; Currently the 1st one is uncommented; comment that out if trying another.
; None
(allowx unconfined_t uffd_t (ioctl anon_inode ((0x00))))
; UFFDIO_API
;(allowx unconfined_t uffd_t (ioctl anon_inode ((0xaa3f))))
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD
2020-04-22 16:55 ` James Morris
2020-04-22 17:12 ` Casey Schaufler
@ 2020-04-27 17:15 ` Casey Schaufler
2020-04-27 19:40 ` Stephen Smalley
1 sibling, 1 reply; 60+ messages in thread
From: Casey Schaufler @ 2020-04-27 17:15 UTC (permalink / raw)
To: James Morris, Daniel Colascione
Cc: Tim Murray, SElinux list, LSM List, Linux FS Devel, linux-kernel,
kvm, Al Viro, Paul Moore, Nick Kralevich, Stephen Smalley,
Lokesh Gidra, John Johansen, Casey Schaufler
On 4/22/2020 9:55 AM, James Morris wrote:
> On Mon, 13 Apr 2020, Daniel Colascione wrote:
>
>> On Wed, Apr 1, 2020 at 2:39 PM Daniel Colascione <dancol@google.com> wrote:
>>> Changes from the fourth version of the patch:
>>
>> Is there anything else that needs to be done before merging this patch series?
> The vfs changes need review and signoff from the vfs folk, the SELinux
> changes by either Paul or Stephen, and we also need signoff on the LSM
> hooks from other major LSM authors (Casey and John, at a minimum).
You can add my
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
for this patchset.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD
2020-04-27 17:15 ` Casey Schaufler
@ 2020-04-27 19:40 ` Stephen Smalley
0 siblings, 0 replies; 60+ messages in thread
From: Stephen Smalley @ 2020-04-27 19:40 UTC (permalink / raw)
To: Casey Schaufler
Cc: James Morris, Daniel Colascione, Tim Murray, SElinux list,
LSM List, Linux FS Devel, linux-kernel, kvm, Al Viro, Paul Moore,
Nick Kralevich, Stephen Smalley, Lokesh Gidra, John Johansen
On Mon, Apr 27, 2020 at 1:17 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 4/22/2020 9:55 AM, James Morris wrote:
> > On Mon, 13 Apr 2020, Daniel Colascione wrote:
> >
> >> On Wed, Apr 1, 2020 at 2:39 PM Daniel Colascione <dancol@google.com> wrote:
> >>> Changes from the fourth version of the patch:
> >>
> >> Is there anything else that needs to be done before merging this patch series?
> > The vfs changes need review and signoff from the vfs folk, the SELinux
> > changes by either Paul or Stephen, and we also need signoff on the LSM
> > hooks from other major LSM authors (Casey and John, at a minimum).
>
> You can add my
>
> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
>
> for this patchset.
This version of the series addresses all of my comments, so you can add my
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
I don't know though how to get a response from the vfs folks; the
series has been posted repeatedly without any
response by them.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD
2020-04-01 21:39 ` [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
` (3 preceding siblings ...)
2020-04-13 13:29 ` [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione
@ 2020-06-04 3:56 ` James Morris
2020-06-04 18:51 ` Stephen Smalley
4 siblings, 1 reply; 60+ messages in thread
From: James Morris @ 2020-06-04 3:56 UTC (permalink / raw)
To: Daniel Colascione
Cc: timmurray, selinux, linux-security-module, linux-fsdevel,
linux-kernel, kvm, viro, Paul Moore, nnk, Stephen Smalley,
lokeshgidra, Andrew Morton
On Wed, 1 Apr 2020, Daniel Colascione wrote:
> Daniel Colascione (3):
> Add a new LSM-supporting anonymous inode interface
> Teach SELinux about anonymous inodes
> Wire UFFD up to SELinux
>
> fs/anon_inodes.c | 191 ++++++++++++++++++++++------
> fs/userfaultfd.c | 30 ++++-
> include/linux/anon_inodes.h | 13 ++
> include/linux/lsm_hooks.h | 11 ++
> include/linux/security.h | 3 +
> security/security.c | 9 ++
> security/selinux/hooks.c | 53 ++++++++
> security/selinux/include/classmap.h | 2 +
> 8 files changed, 267 insertions(+), 45 deletions(-)
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git secure_uffd_v5.9
and next-testing.
This will provide test coverage in linux-next, as we aim to get this
upstream for v5.9.
I had to make some minor fixups, please review.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD
2020-06-04 3:56 ` James Morris
@ 2020-06-04 18:51 ` Stephen Smalley
2020-06-04 19:24 ` Lokesh Gidra
0 siblings, 1 reply; 60+ messages in thread
From: Stephen Smalley @ 2020-06-04 18:51 UTC (permalink / raw)
To: James Morris
Cc: Daniel Colascione, Tim Murray, SElinux list, LSM List,
Linux FS Devel, linux-kernel, kvm, Al Viro, Paul Moore,
Nick Kralevich, Stephen Smalley, Lokesh Gidra, Andrew Morton
On Wed, Jun 3, 2020 at 11:59 PM James Morris <jmorris@namei.org> wrote:
>
> On Wed, 1 Apr 2020, Daniel Colascione wrote:
>
> > Daniel Colascione (3):
> > Add a new LSM-supporting anonymous inode interface
> > Teach SELinux about anonymous inodes
> > Wire UFFD up to SELinux
> >
> > fs/anon_inodes.c | 191 ++++++++++++++++++++++------
> > fs/userfaultfd.c | 30 ++++-
> > include/linux/anon_inodes.h | 13 ++
> > include/linux/lsm_hooks.h | 11 ++
> > include/linux/security.h | 3 +
> > security/security.c | 9 ++
> > security/selinux/hooks.c | 53 ++++++++
> > security/selinux/include/classmap.h | 2 +
> > 8 files changed, 267 insertions(+), 45 deletions(-)
>
> Applied to
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git secure_uffd_v5.9
> and next-testing.
>
> This will provide test coverage in linux-next, as we aim to get this
> upstream for v5.9.
>
> I had to make some minor fixups, please review.
LGTM and my userfaultfd test case worked.
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD
2020-06-04 18:51 ` Stephen Smalley
@ 2020-06-04 19:24 ` Lokesh Gidra
0 siblings, 0 replies; 60+ messages in thread
From: Lokesh Gidra @ 2020-06-04 19:24 UTC (permalink / raw)
To: Stephen Smalley
Cc: James Morris, Daniel Colascione, Tim Murray, SElinux list,
LSM List, Linux FS Devel, linux-kernel, kvm, Al Viro, Paul Moore,
Nick Kralevich, Stephen Smalley, Andrew Morton,
Suren Baghdasaryan
Adding a colleague from the Android kernel team.
On Thu, Jun 4, 2020 at 11:52 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Wed, Jun 3, 2020 at 11:59 PM James Morris <jmorris@namei.org> wrote:
> >
> > On Wed, 1 Apr 2020, Daniel Colascione wrote:
> >
> > > Daniel Colascione (3):
> > > Add a new LSM-supporting anonymous inode interface
> > > Teach SELinux about anonymous inodes
> > > Wire UFFD up to SELinux
> > >
> > > fs/anon_inodes.c | 191 ++++++++++++++++++++++------
> > > fs/userfaultfd.c | 30 ++++-
> > > include/linux/anon_inodes.h | 13 ++
> > > include/linux/lsm_hooks.h | 11 ++
> > > include/linux/security.h | 3 +
> > > security/security.c | 9 ++
> > > security/selinux/hooks.c | 53 ++++++++
> > > security/selinux/include/classmap.h | 2 +
> > > 8 files changed, 267 insertions(+), 45 deletions(-)
> >
> > Applied to
> > git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git secure_uffd_v5.9
> > and next-testing.
> >
> > This will provide test coverage in linux-next, as we aim to get this
> > upstream for v5.9.
> >
> > I had to make some minor fixups, please review.
>
> LGTM and my userfaultfd test case worked.
^ permalink raw reply [flat|nested] 60+ messages in thread