diff mbox series

[v6,12/17] securityfs: Extend securityfs with namespacing support

Message ID 20211210194736.1538863-13-stefanb@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series ima: Namespace IMA with audit support in IMA-ns | expand

Commit Message

Stefan Berger Dec. 10, 2021, 7:47 p.m. UTC 
Extend 'securityfs' for support of IMA namespacing so that each
IMA (user) namespace can have its own front-end for showing the currently
active policy, the measurement list, number of violations and so on.

Drop the addition dentry reference and simplify cleanup to work without
the additional reference. This enables simple cleanup of dentries upon
umount.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 security/inode.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

Comments

Christian Brauner Dec. 11, 2021, 10:50 a.m. UTC | #1 
On Fri, Dec 10, 2021 at 02:47:31PM -0500, Stefan Berger wrote:
> Extend 'securityfs' for support of IMA namespacing so that each
> IMA (user) namespace can have its own front-end for showing the currently
> active policy, the measurement list, number of violations and so on.
> 
> Drop the addition dentry reference and simplify cleanup to work without
> the additional reference. This enables simple cleanup of dentries upon
> umount.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>  security/inode.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/security/inode.c b/security/inode.c
> index fee01ff4d831..c77ae8ecc464 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -54,7 +54,7 @@ static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc)
>  
>  static int securityfs_get_tree(struct fs_context *fc)
>  {
> -	return get_tree_single(fc, securityfs_fill_super);
> +	return get_tree_keyed(fc, securityfs_fill_super, fc->user_ns);
>  }
>  
>  static const struct fs_context_operations securityfs_context_ops = {
> @@ -72,6 +72,7 @@ static struct file_system_type fs_type = {
>  	.name =		"securityfs",
>  	.init_fs_context = securityfs_init_fs_context,
>  	.kill_sb =	kill_litter_super,
> +	.fs_flags =	FS_USERNS_MOUNT,
>  };
>  
>  /**
> @@ -168,7 +169,6 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
>  		inode->i_fop = fops;
>  	}
>  	d_instantiate(dentry, inode);
> -	dget(dentry);
>  	inode_unlock(dir);
>  	return dentry;
>  
> @@ -306,23 +306,17 @@ EXPORT_SYMBOL_GPL(securityfs_create_symlink);
>  void securityfs_remove(struct dentry *dentry)
>  {
>  	struct user_namespace *ns;
> -	struct inode *dir;
>  
>  	if (!dentry || IS_ERR(dentry))
>  		return;
>  
>  	ns = dentry->d_sb->s_user_ns;
>  
> -	dir = d_inode(dentry->d_parent);
> -	inode_lock(dir);
>  	if (simple_positive(dentry)) {
> -		if (d_is_dir(dentry))
> -			simple_rmdir(dir, dentry);
> -		else
> -			simple_unlink(dir, dentry);
> +		d_delete(dentry);
>  		dput(dentry);

In
https://lore.kernel.org/lkml/20211210114934.tacjnwryihrsx6ln@wittgenstein
I explained why d_delete()+d_put() only is wrong and how to fix it.
Stefan Berger Dec. 11, 2021, 10:31 p.m. UTC | #2 
On 12/11/21 05:50, Christian Brauner wrote:
> On Fri, Dec 10, 2021 at 02:47:31PM -0500, Stefan Berger wrote:
>> Extend 'securityfs' for support of IMA namespacing so that each
>> IMA (user) namespace can have its own front-end for showing the currently
>> active policy, the measurement list, number of violations and so on.
>>
>> Drop the addition dentry reference and simplify cleanup to work without
>> the additional reference. This enables simple cleanup of dentries upon
>> umount.
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
>> ---
>>   security/inode.c | 14 ++++----------
>>   1 file changed, 4 insertions(+), 10 deletions(-)
>>
>> diff --git a/security/inode.c b/security/inode.c
>> index fee01ff4d831..c77ae8ecc464 100644
>> --- a/security/inode.c
>> +++ b/security/inode.c
>> @@ -54,7 +54,7 @@ static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc)
>>   
>>   static int securityfs_get_tree(struct fs_context *fc)
>>   {
>> -	return get_tree_single(fc, securityfs_fill_super);
>> +	return get_tree_keyed(fc, securityfs_fill_super, fc->user_ns);
>>   }
>>   
>>   static const struct fs_context_operations securityfs_context_ops = {
>> @@ -72,6 +72,7 @@ static struct file_system_type fs_type = {
>>   	.name =		"securityfs",
>>   	.init_fs_context = securityfs_init_fs_context,
>>   	.kill_sb =	kill_litter_super,
>> +	.fs_flags =	FS_USERNS_MOUNT,
>>   };
>>   
>>   /**
>> @@ -168,7 +169,6 @@ static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
>>   		inode->i_fop = fops;
>>   	}
>>   	d_instantiate(dentry, inode);
>> -	dget(dentry);
>>   	inode_unlock(dir);
>>   	return dentry;
>>   
>> @@ -306,23 +306,17 @@ EXPORT_SYMBOL_GPL(securityfs_create_symlink);
>>   void securityfs_remove(struct dentry *dentry)
>>   {
>>   	struct user_namespace *ns;
>> -	struct inode *dir;
>>   
>>   	if (!dentry || IS_ERR(dentry))
>>   		return;
>>   
>>   	ns = dentry->d_sb->s_user_ns;
>>   
>> -	dir = d_inode(dentry->d_parent);
>> -	inode_lock(dir);
>>   	if (simple_positive(dentry)) {
>> -		if (d_is_dir(dentry))
>> -			simple_rmdir(dir, dentry);
>> -		else
>> -			simple_unlink(dir, dentry);
>> +		d_delete(dentry);
>>   		dput(dentry);
> In
> https://lore.kernel.org/lkml/20211210114934.tacjnwryihrsx6ln@wittgenstein
> I explained why d_delete()+d_put() only is wrong and how to fix it.


Fixed for v7.
diff mbox series

Patch

diff --git a/security/inode.c b/security/inode.c
index fee01ff4d831..c77ae8ecc464 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -54,7 +54,7 @@  static int securityfs_fill_super(struct super_block *sb, struct fs_context *fc)
 
 static int securityfs_get_tree(struct fs_context *fc)
 {
-	return get_tree_single(fc, securityfs_fill_super);
+	return get_tree_keyed(fc, securityfs_fill_super, fc->user_ns);
 }
 
 static const struct fs_context_operations securityfs_context_ops = {
@@ -72,6 +72,7 @@  static struct file_system_type fs_type = {
 	.name =		"securityfs",
 	.init_fs_context = securityfs_init_fs_context,
 	.kill_sb =	kill_litter_super,
+	.fs_flags =	FS_USERNS_MOUNT,
 };
 
 /**
@@ -168,7 +169,6 @@  static struct dentry *securityfs_create_dentry(const char *name, umode_t mode,
 		inode->i_fop = fops;
 	}
 	d_instantiate(dentry, inode);
-	dget(dentry);
 	inode_unlock(dir);
 	return dentry;
 
@@ -306,23 +306,17 @@  EXPORT_SYMBOL_GPL(securityfs_create_symlink);
 void securityfs_remove(struct dentry *dentry)
 {
 	struct user_namespace *ns;
-	struct inode *dir;
 
 	if (!dentry || IS_ERR(dentry))
 		return;
 
 	ns = dentry->d_sb->s_user_ns;
 
-	dir = d_inode(dentry->d_parent);
-	inode_lock(dir);
 	if (simple_positive(dentry)) {
-		if (d_is_dir(dentry))
-			simple_rmdir(dir, dentry);
-		else
-			simple_unlink(dir, dentry);
+		d_delete(dentry);
 		dput(dentry);
 	}
-	inode_unlock(dir);
+
 	if (ns == &init_user_ns)
 		simple_release_fs(&init_securityfs_mount,
 				  &init_securityfs_mount_count);