Messages in this thread | | | Subject | Re: [PATCH v2 1/3] Add a new LSM-supporting anonymous inode interface | From | Stephen Smalley <> | Date | Thu, 26 Mar 2020 09:53:59 -0400 |
| |
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.
| |