Re: [PATCH v11 2/4] fs: add LSM-supporting anon-inode interface
From: Eric Biggers <ebiggers@kernel.org>
Date: 2020-11-05 22:24:28
Also in:
linux-fsdevel, linux-mm, lkml, selinux
On Thu, Nov 05, 2020 at 01:33:22PM -0800, Lokesh Gidra wrote:
+/**
+ * Like anon_inode_getfd(), but creates a new !S_PRIVATE anon inode rather than
+ * reuse the singleton anon inode, and call the init_security_anon() LSM hook.
+ * This allows the inode to have its own security context and for a LSM to
+ * reject creation of the inode. An optional @context_inode argument is also
+ * added to provide the logical relationship with the new inode. The LSM may use
+ * @context_inode in init_security_anon(), but a reference to it is not held.
+ */
+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);inode_init_security_anon(), not init_security_anon(). Also please use a consistent line width (preferably 80 characters).
quoted hunk ↗ jump to hunk
diff --git a/fs/libfs.c b/fs/libfs.c index fc34361c1489..5b12228ecc81 100644 --- a/fs/libfs.c +++ b/fs/libfs.c@@ -1213,9 +1213,9 @@ static int anon_set_page_dirty(struct page *page) }; /* - * A single inode exists for all anon_inode files. Contrary to pipes, - * anon_inode inodes have no associated per-instance data, so we need - * only allocate one of them. + * A single inode exists for all anon_inode files, except for the secure ones. + * Contrary to pipes and secure anon_inode inodes, ordinary anon_inode inodes + * have no associated per-instance data, so we need only allocate one of them. */ struct inode *alloc_anon_inode(struct super_block *s) {
This comment is still wrong, and the first sentence contradicts the second one. There are a lot of callers of alloc_anon_inode() and none of them use the singleton inode, since alloc_anon_inode() doesn't actually use it. The singleton inode is only used by anon_inode_getfile() and anon_inode_getfd(), which already have comments describing how they use a singleton inode. IMO, just deleting this comment would be much better than either the original version or your proposed version.
quoted hunk ↗ jump to hunk
diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h index d0d7d96261ad..6cf447cfceed 100644 --- a/include/linux/anon_inodes.h +++ b/include/linux/anon_inodes.h@@ -10,10 +10,15 @@ #define _LINUX_ANON_INODES_H struct file_operations; +struct 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);
Keeping declarations in the same order as the definitions can be helpful. - Eric