Re: [PATCH 1/8] apparmor: just use vfs_kern_mount to make .null

2020-04-20 Thread Emanuele Giuseppe Esposito 




On 4/16/20 8:44 AM, Luis Chamberlain wrote:

On Tue, Apr 14, 2020 at 02:42:55PM +0200, Emanuele Giuseppe Esposito wrote:

aa_mk_null_file is using simple_pin_fs/simple_release_fs with local
variables as arguments, for what would amount to a simple
vfs_kern_mount/mntput pair if everything was inlined.  Just use
the normal filesystem API since the reference counting is not needed
here.


*Why* is refcounting not needed here?


The refcount is a local variable and is always 0 on entry and exit, so 
it is not necessary to have refcounting across function invocations, 
such as what simple_pin_fs and simple_release_fs provide.


Thank you,

Emanuele


Luis



Signed-off-by: Emanuele Giuseppe Esposito 
---
  security/apparmor/apparmorfs.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 280741fc0f5f..828bb1eb77ea 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -2525,14 +2525,14 @@ struct path aa_null;
  
  static int aa_mk_null_file(struct dentry *parent)

  {
-   struct vfsmount *mount = NULL;
+   struct file_system_type *type = parent->d_sb->s_type;
+   struct vfsmount *mount;
struct dentry *dentry;
struct inode *inode;
-   int count = 0;
-   int error = simple_pin_fs(parent->d_sb->s_type, &mount, &count);
  
-	if (error)

-   return error;
+   mount = vfs_kern_mount(type, SB_KERNMOUNT, type->name, NULL);
+   if (IS_ERR(mount))
+   return PTR_ERR(mount);
  
  	inode_lock(d_inode(parent));

dentry = lookup_one_len(NULL_FILE_NAME, parent, strlen(NULL_FILE_NAME));
@@ -2561,7 +2561,7 @@ static int aa_mk_null_file(struct dentry *parent)
dput(dentry);
  out:
inode_unlock(d_inode(parent));
-   simple_release_fs(&mount, &count);
+   mntput(mount);
return error;
  }
  
--

2.25.2

Re: [PATCH 1/8] apparmor: just use vfs_kern_mount to make .null

2020-04-16 Thread Luis Chamberlain 
On Tue, Apr 14, 2020 at 02:42:55PM +0200, Emanuele Giuseppe Esposito wrote:
> aa_mk_null_file is using simple_pin_fs/simple_release_fs with local
> variables as arguments, for what would amount to a simple
> vfs_kern_mount/mntput pair if everything was inlined.  Just use
> the normal filesystem API since the reference counting is not needed
> here.

*Why* is refcounting not needed here?

   Luis

> 
> Signed-off-by: Emanuele Giuseppe Esposito 
> ---
>  security/apparmor/apparmorfs.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 280741fc0f5f..828bb1eb77ea 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -2525,14 +2525,14 @@ struct path aa_null;
>  
>  static int aa_mk_null_file(struct dentry *parent)
>  {
> - struct vfsmount *mount = NULL;
> + struct file_system_type *type = parent->d_sb->s_type;
> + struct vfsmount *mount;
>   struct dentry *dentry;
>   struct inode *inode;
> - int count = 0;
> - int error = simple_pin_fs(parent->d_sb->s_type, &mount, &count);
>  
> - if (error)
> - return error;
> + mount = vfs_kern_mount(type, SB_KERNMOUNT, type->name, NULL);
> + if (IS_ERR(mount))
> + return PTR_ERR(mount);
>  
>   inode_lock(d_inode(parent));
>   dentry = lookup_one_len(NULL_FILE_NAME, parent, strlen(NULL_FILE_NAME));
> @@ -2561,7 +2561,7 @@ static int aa_mk_null_file(struct dentry *parent)
>   dput(dentry);
>  out:
>   inode_unlock(d_inode(parent));
> - simple_release_fs(&mount, &count);
> + mntput(mount);
>   return error;
>  }
>  
> -- 
> 2.25.2
>

[PATCH 1/8] apparmor: just use vfs_kern_mount to make .null

2020-04-14 Thread Emanuele Giuseppe Esposito 
aa_mk_null_file is using simple_pin_fs/simple_release_fs with local
variables as arguments, for what would amount to a simple
vfs_kern_mount/mntput pair if everything was inlined.  Just use
the normal filesystem API since the reference counting is not needed
here.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 security/apparmor/apparmorfs.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 280741fc0f5f..828bb1eb77ea 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -2525,14 +2525,14 @@ struct path aa_null;
 
 static int aa_mk_null_file(struct dentry *parent)
 {
-   struct vfsmount *mount = NULL;
+   struct file_system_type *type = parent->d_sb->s_type;
+   struct vfsmount *mount;
struct dentry *dentry;
struct inode *inode;
-   int count = 0;
-   int error = simple_pin_fs(parent->d_sb->s_type, &mount, &count);
 
-   if (error)
-   return error;
+   mount = vfs_kern_mount(type, SB_KERNMOUNT, type->name, NULL);
+   if (IS_ERR(mount))
+   return PTR_ERR(mount);
 
inode_lock(d_inode(parent));
dentry = lookup_one_len(NULL_FILE_NAME, parent, strlen(NULL_FILE_NAME));
@@ -2561,7 +2561,7 @@ static int aa_mk_null_file(struct dentry *parent)
dput(dentry);
 out:
inode_unlock(d_inode(parent));
-   simple_release_fs(&mount, &count);
+   mntput(mount);
return error;
 }
 
-- 
2.25.2