Re: [PATCH 1/3] securityfs: add the ability to support symlinks

[Kees Cook]

On Wed, May 10, 2017 at 7:46 PM, John Johansen
 wrote:
> Signed-off-by: John Johansen 
> Reviewed-by: Seth Arnold 

This looks correct to me; it matches what other pseudo-filesystems do,
and provides the expected interfaces.

Acked-by: Kees Cook 

-Kees

> ---
>  include/linux/security.h |  12 
>  security/inode.c | 140 
> +--
>  2 files changed, 134 insertions(+), 18 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 78d8e03be5d3..28e2be5dd6df 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1645,6 +1645,10 @@ extern struct dentry *securityfs_create_file(const 
> char *name, umode_t mode,
>  struct dentry *parent, void 
> *data,
>  const struct file_operations 
> *fops);
>  extern struct dentry *securityfs_create_dir(const char *name, struct dentry 
> *parent);
> +struct dentry *securityfs_create_symlink(const char *name,
> +struct dentry *parent,
> +const char *target,
> +const struct inode_operations *iops);
>  extern void securityfs_remove(struct dentry *dentry);
>
>  #else /* CONFIG_SECURITYFS */
> @@ -1664,6 +1668,14 @@ static inline struct dentry 
> *securityfs_create_file(const char *name,
> return ERR_PTR(-ENODEV);
>  }
>
> +struct dentry *securityfs_create_symlink(const char *name,
> +struct dentry *parent,
> +const char *target,
> +const struct inode_operations *iops)
> +{
> +   return ERR_PTR(-ENODEV);
> +}
> +
>  static inline void securityfs_remove(struct dentry *dentry)
>  {}
>
> diff --git a/security/inode.c b/security/inode.c
> index 2cb14162ff8d..10c4955d0bed 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -26,11 +26,31 @@
>  static struct vfsmount *mount;
>  static int mount_count;
>
> +static void securityfs_evict_inode(struct inode *inode)
> +{
> +   truncate_inode_pages_final(>i_data);
> +   clear_inode(inode);
> +   if (S_ISLNK(inode->i_mode))
> +   kfree(inode->i_link);
> +}
> +
> +static const struct super_operations securityfs_super_operations = {
> +   .statfs = simple_statfs,
> +   .evict_inode= securityfs_evict_inode,
> +};
> +
>  static int fill_super(struct super_block *sb, void *data, int silent)
>  {
> static struct tree_descr files[] = {{""}};
> +   int error;
> +
> +   error = simple_fill_super(sb, SECURITYFS_MAGIC, files);
> +   if (error)
> +   return error;
> +
> +   sb->s_op = _super_operations;
>
> -   return simple_fill_super(sb, SECURITYFS_MAGIC, files);
> +   return 0;
>  }
>
>  static struct dentry *get_sb(struct file_system_type *fs_type,
> @@ -48,7 +68,7 @@ static struct file_system_type fs_type = {
>  };
>
>  /**
> - * securityfs_create_file - create a file in the securityfs filesystem
> + * securityfs_create_dentry - create a dentry in the securityfs filesystem
>   *
>   * @name: a pointer to a string containing the name of the file to create.
>   * @mode: the permission that the file should have
> @@ -60,34 +80,35 @@ static struct file_system_type fs_type = {
>   *the open() call.
>   * @fops: a pointer to a struct file_operations that should be used for
>   *this file.
> + * @iops: a point to a struct of inode_operations that should be used for
> + *this file/dir
>   *
> - * This is the basic "create a file" function for securityfs.  It allows for 
> a
> - * wide range of flexibility in creating a file, or a directory (if you
> - * want to create a directory, the securityfs_create_dir() function is
> - * recommended to be used instead).
> + * This is the basic "create a file/dir/symlink" function for
> + * securityfs.  It allows for a wide range of flexibility in creating
> + * a file, or a directory (if you want to create a directory, the
> + * securityfs_create_dir() function is recommended to be used
> + * instead).
>   *
>   * This function returns a pointer to a dentry if it succeeds.  This
> - * pointer must be passed to the securityfs_remove() function when the file 
> is
> - * to be removed (no automatic cleanup happens if your module is unloaded,
> - * you are responsible here).  If an error occurs, the function will return
> - * the error value (via ERR_PTR).
> + * pointer must be passed to the securityfs_remove() function when the
> + * file is to be removed (no automatic cleanup happens if your module
> + * is unloaded, you are responsible here).  If an error occurs, the
> + * function will return the error value (via ERR_PTR).
>   *
>   * If securityfs is 

Re: [PATCH 1/3] securityfs: add the ability to support symlinks

[Kees Cook]

On Wed, May 10, 2017 at 7:46 PM, John Johansen
 wrote:
> Signed-off-by: John Johansen 
> Reviewed-by: Seth Arnold 

This looks correct to me; it matches what other pseudo-filesystems do,
and provides the expected interfaces.

Acked-by: Kees Cook 

-Kees

> ---
>  include/linux/security.h |  12 
>  security/inode.c | 140 
> +--
>  2 files changed, 134 insertions(+), 18 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 78d8e03be5d3..28e2be5dd6df 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1645,6 +1645,10 @@ extern struct dentry *securityfs_create_file(const 
> char *name, umode_t mode,
>  struct dentry *parent, void 
> *data,
>  const struct file_operations 
> *fops);
>  extern struct dentry *securityfs_create_dir(const char *name, struct dentry 
> *parent);
> +struct dentry *securityfs_create_symlink(const char *name,
> +struct dentry *parent,
> +const char *target,
> +const struct inode_operations *iops);
>  extern void securityfs_remove(struct dentry *dentry);
>
>  #else /* CONFIG_SECURITYFS */
> @@ -1664,6 +1668,14 @@ static inline struct dentry 
> *securityfs_create_file(const char *name,
> return ERR_PTR(-ENODEV);
>  }
>
> +struct dentry *securityfs_create_symlink(const char *name,
> +struct dentry *parent,
> +const char *target,
> +const struct inode_operations *iops)
> +{
> +   return ERR_PTR(-ENODEV);
> +}
> +
>  static inline void securityfs_remove(struct dentry *dentry)
>  {}
>
> diff --git a/security/inode.c b/security/inode.c
> index 2cb14162ff8d..10c4955d0bed 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -26,11 +26,31 @@
>  static struct vfsmount *mount;
>  static int mount_count;
>
> +static void securityfs_evict_inode(struct inode *inode)
> +{
> +   truncate_inode_pages_final(>i_data);
> +   clear_inode(inode);
> +   if (S_ISLNK(inode->i_mode))
> +   kfree(inode->i_link);
> +}
> +
> +static const struct super_operations securityfs_super_operations = {
> +   .statfs = simple_statfs,
> +   .evict_inode= securityfs_evict_inode,
> +};
> +
>  static int fill_super(struct super_block *sb, void *data, int silent)
>  {
> static struct tree_descr files[] = {{""}};
> +   int error;
> +
> +   error = simple_fill_super(sb, SECURITYFS_MAGIC, files);
> +   if (error)
> +   return error;
> +
> +   sb->s_op = _super_operations;
>
> -   return simple_fill_super(sb, SECURITYFS_MAGIC, files);
> +   return 0;
>  }
>
>  static struct dentry *get_sb(struct file_system_type *fs_type,
> @@ -48,7 +68,7 @@ static struct file_system_type fs_type = {
>  };
>
>  /**
> - * securityfs_create_file - create a file in the securityfs filesystem
> + * securityfs_create_dentry - create a dentry in the securityfs filesystem
>   *
>   * @name: a pointer to a string containing the name of the file to create.
>   * @mode: the permission that the file should have
> @@ -60,34 +80,35 @@ static struct file_system_type fs_type = {
>   *the open() call.
>   * @fops: a pointer to a struct file_operations that should be used for
>   *this file.
> + * @iops: a point to a struct of inode_operations that should be used for
> + *this file/dir
>   *
> - * This is the basic "create a file" function for securityfs.  It allows for 
> a
> - * wide range of flexibility in creating a file, or a directory (if you
> - * want to create a directory, the securityfs_create_dir() function is
> - * recommended to be used instead).
> + * This is the basic "create a file/dir/symlink" function for
> + * securityfs.  It allows for a wide range of flexibility in creating
> + * a file, or a directory (if you want to create a directory, the
> + * securityfs_create_dir() function is recommended to be used
> + * instead).
>   *
>   * This function returns a pointer to a dentry if it succeeds.  This
> - * pointer must be passed to the securityfs_remove() function when the file 
> is
> - * to be removed (no automatic cleanup happens if your module is unloaded,
> - * you are responsible here).  If an error occurs, the function will return
> - * the error value (via ERR_PTR).
> + * pointer must be passed to the securityfs_remove() function when the
> + * file is to be removed (no automatic cleanup happens if your module
> + * is unloaded, you are responsible here).  If an error occurs, the
> + * function will return the error value (via ERR_PTR).
>   *
>   * If securityfs is not enabled in the kernel, the value %-ENODEV is
>   * returned.
>   */
> -struct dentry 

[PATCH 1/3] securityfs: add the ability to support symlinks

[John Johansen]

Signed-off-by: John Johansen 
Reviewed-by: Seth Arnold 
---
 include/linux/security.h |  12 
 security/inode.c | 140 +--
 2 files changed, 134 insertions(+), 18 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 78d8e03be5d3..28e2be5dd6df 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1645,6 +1645,10 @@ extern struct dentry *securityfs_create_file(const char 
*name, umode_t mode,
 struct dentry *parent, void *data,
 const struct file_operations 
*fops);
 extern struct dentry *securityfs_create_dir(const char *name, struct dentry 
*parent);
+struct dentry *securityfs_create_symlink(const char *name,
+struct dentry *parent,
+const char *target,
+const struct inode_operations *iops);
 extern void securityfs_remove(struct dentry *dentry);
 
 #else /* CONFIG_SECURITYFS */
@@ -1664,6 +1668,14 @@ static inline struct dentry 
*securityfs_create_file(const char *name,
return ERR_PTR(-ENODEV);
 }
 
+struct dentry *securityfs_create_symlink(const char *name,
+struct dentry *parent,
+const char *target,
+const struct inode_operations *iops)
+{
+   return ERR_PTR(-ENODEV);
+}
+
 static inline void securityfs_remove(struct dentry *dentry)
 {}
 
diff --git a/security/inode.c b/security/inode.c
index 2cb14162ff8d..10c4955d0bed 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -26,11 +26,31 @@
 static struct vfsmount *mount;
 static int mount_count;
 
+static void securityfs_evict_inode(struct inode *inode)
+{
+   truncate_inode_pages_final(>i_data);
+   clear_inode(inode);
+   if (S_ISLNK(inode->i_mode))
+   kfree(inode->i_link);
+}
+
+static const struct super_operations securityfs_super_operations = {
+   .statfs = simple_statfs,
+   .evict_inode= securityfs_evict_inode,
+};
+
 static int fill_super(struct super_block *sb, void *data, int silent)
 {
static struct tree_descr files[] = {{""}};
+   int error;
+
+   error = simple_fill_super(sb, SECURITYFS_MAGIC, files);
+   if (error)
+   return error;
+
+   sb->s_op = _super_operations;
 
-   return simple_fill_super(sb, SECURITYFS_MAGIC, files);
+   return 0;
 }
 
 static struct dentry *get_sb(struct file_system_type *fs_type,
@@ -48,7 +68,7 @@ static struct file_system_type fs_type = {
 };
 
 /**
- * securityfs_create_file - create a file in the securityfs filesystem
+ * securityfs_create_dentry - create a dentry in the securityfs filesystem
  *
  * @name: a pointer to a string containing the name of the file to create.
  * @mode: the permission that the file should have
@@ -60,34 +80,35 @@ static struct file_system_type fs_type = {
  *the open() call.
  * @fops: a pointer to a struct file_operations that should be used for
  *this file.
+ * @iops: a point to a struct of inode_operations that should be used for
+ *this file/dir
  *
- * This is the basic "create a file" function for securityfs.  It allows for a
- * wide range of flexibility in creating a file, or a directory (if you
- * want to create a directory, the securityfs_create_dir() function is
- * recommended to be used instead).
+ * This is the basic "create a file/dir/symlink" function for
+ * securityfs.  It allows for a wide range of flexibility in creating
+ * a file, or a directory (if you want to create a directory, the
+ * securityfs_create_dir() function is recommended to be used
+ * instead).
  *
  * This function returns a pointer to a dentry if it succeeds.  This
- * pointer must be passed to the securityfs_remove() function when the file is
- * to be removed (no automatic cleanup happens if your module is unloaded,
- * you are responsible here).  If an error occurs, the function will return
- * the error value (via ERR_PTR).
+ * pointer must be passed to the securityfs_remove() function when the
+ * file is to be removed (no automatic cleanup happens if your module
+ * is unloaded, you are responsible here).  If an error occurs, the
+ * function will return the error value (via ERR_PTR).
  *
  * If securityfs is not enabled in the kernel, the value %-ENODEV is
  * returned.
  */
-struct dentry *securityfs_create_file(const char *name, umode_t mode,
-  struct dentry *parent, void *data,
-  const struct file_operations *fops)
+static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
+   struct dentry *parent, void *data,
+   

[PATCH 1/3] securityfs: add the ability to support symlinks

[John Johansen]

Signed-off-by: John Johansen 
Reviewed-by: Seth Arnold 
---
 include/linux/security.h |  12 
 security/inode.c | 140 +--
 2 files changed, 134 insertions(+), 18 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 78d8e03be5d3..28e2be5dd6df 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1645,6 +1645,10 @@ extern struct dentry *securityfs_create_file(const char 
*name, umode_t mode,
 struct dentry *parent, void *data,
 const struct file_operations 
*fops);
 extern struct dentry *securityfs_create_dir(const char *name, struct dentry 
*parent);
+struct dentry *securityfs_create_symlink(const char *name,
+struct dentry *parent,
+const char *target,
+const struct inode_operations *iops);
 extern void securityfs_remove(struct dentry *dentry);
 
 #else /* CONFIG_SECURITYFS */
@@ -1664,6 +1668,14 @@ static inline struct dentry 
*securityfs_create_file(const char *name,
return ERR_PTR(-ENODEV);
 }
 
+struct dentry *securityfs_create_symlink(const char *name,
+struct dentry *parent,
+const char *target,
+const struct inode_operations *iops)
+{
+   return ERR_PTR(-ENODEV);
+}
+
 static inline void securityfs_remove(struct dentry *dentry)
 {}
 
diff --git a/security/inode.c b/security/inode.c
index 2cb14162ff8d..10c4955d0bed 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -26,11 +26,31 @@
 static struct vfsmount *mount;
 static int mount_count;
 
+static void securityfs_evict_inode(struct inode *inode)
+{
+   truncate_inode_pages_final(>i_data);
+   clear_inode(inode);
+   if (S_ISLNK(inode->i_mode))
+   kfree(inode->i_link);
+}
+
+static const struct super_operations securityfs_super_operations = {
+   .statfs = simple_statfs,
+   .evict_inode= securityfs_evict_inode,
+};
+
 static int fill_super(struct super_block *sb, void *data, int silent)
 {
static struct tree_descr files[] = {{""}};
+   int error;
+
+   error = simple_fill_super(sb, SECURITYFS_MAGIC, files);
+   if (error)
+   return error;
+
+   sb->s_op = _super_operations;
 
-   return simple_fill_super(sb, SECURITYFS_MAGIC, files);
+   return 0;
 }
 
 static struct dentry *get_sb(struct file_system_type *fs_type,
@@ -48,7 +68,7 @@ static struct file_system_type fs_type = {
 };
 
 /**
- * securityfs_create_file - create a file in the securityfs filesystem
+ * securityfs_create_dentry - create a dentry in the securityfs filesystem
  *
  * @name: a pointer to a string containing the name of the file to create.
  * @mode: the permission that the file should have
@@ -60,34 +80,35 @@ static struct file_system_type fs_type = {
  *the open() call.
  * @fops: a pointer to a struct file_operations that should be used for
  *this file.
+ * @iops: a point to a struct of inode_operations that should be used for
+ *this file/dir
  *
- * This is the basic "create a file" function for securityfs.  It allows for a
- * wide range of flexibility in creating a file, or a directory (if you
- * want to create a directory, the securityfs_create_dir() function is
- * recommended to be used instead).
+ * This is the basic "create a file/dir/symlink" function for
+ * securityfs.  It allows for a wide range of flexibility in creating
+ * a file, or a directory (if you want to create a directory, the
+ * securityfs_create_dir() function is recommended to be used
+ * instead).
  *
  * This function returns a pointer to a dentry if it succeeds.  This
- * pointer must be passed to the securityfs_remove() function when the file is
- * to be removed (no automatic cleanup happens if your module is unloaded,
- * you are responsible here).  If an error occurs, the function will return
- * the error value (via ERR_PTR).
+ * pointer must be passed to the securityfs_remove() function when the
+ * file is to be removed (no automatic cleanup happens if your module
+ * is unloaded, you are responsible here).  If an error occurs, the
+ * function will return the error value (via ERR_PTR).
  *
  * If securityfs is not enabled in the kernel, the value %-ENODEV is
  * returned.
  */
-struct dentry *securityfs_create_file(const char *name, umode_t mode,
-  struct dentry *parent, void *data,
-  const struct file_operations *fops)
+static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
+   struct dentry *parent, void *data,
+   const struct file_operations *fops,
+