diff mbox series

[v28,06/12] fs,security: Add sb_delete hook

Message ID 20210202162710.657398-7-mic@digikod.net
State Superseded
Headers show
Series Landlock LSM | expand

Commit Message

Mickaël Salaün Feb. 2, 2021, 4:27 p.m. UTC
From: Mickaël Salaün <mic@linux.microsoft.com>

The sb_delete security hook is called when shutting down a superblock,
which may be useful to release kernel objects tied to the superblock's
lifetime (e.g. inodes).

This new hook is needed by Landlock to release (ephemerally) tagged
struct inodes.  This comes from the unprivileged nature of Landlock
described in the next commit.

Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: James Morris <jmorris@namei.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Serge E. Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Reviewed-by: Jann Horn <jannh@google.com>
---

Changes since v22:
* Add Reviewed-by: Jann Horn <jannh@google.com>

Changes since v17:
* Initial patch to replace the direct call to landlock_release_inodes()
  (requested by James Morris).
  https://lore.kernel.org/lkml/alpine.LRH.2.21.2005150536440.7929@namei.org/
---
 fs/super.c                    | 1 +
 include/linux/lsm_hook_defs.h | 1 +
 include/linux/lsm_hooks.h     | 2 ++
 include/linux/security.h      | 4 ++++
 security/security.c           | 5 +++++
 5 files changed, 13 insertions(+)

Comments

Serge E. Hallyn Feb. 5, 2021, 2:21 p.m. UTC | #1
On Tue, Feb 02, 2021 at 05:27:04PM +0100, Mickaël Salaün wrote:
> From: Mickaël Salaün <mic@linux.microsoft.com>

> 

> The sb_delete security hook is called when shutting down a superblock,

> which may be useful to release kernel objects tied to the superblock's

> lifetime (e.g. inodes).

> 

> This new hook is needed by Landlock to release (ephemerally) tagged

> struct inodes.  This comes from the unprivileged nature of Landlock

> described in the next commit.

> 

> Cc: Al Viro <viro@zeniv.linux.org.uk>

> Cc: James Morris <jmorris@namei.org>

> Cc: Kees Cook <keescook@chromium.org>

> Cc: Serge E. Hallyn <serge@hallyn.com>


One note below, but

Acked-by: Serge Hallyn <serge@hallyn.com>


> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>

> Reviewed-by: Jann Horn <jannh@google.com>

> ---

> 

> Changes since v22:

> * Add Reviewed-by: Jann Horn <jannh@google.com>

> 

> Changes since v17:

> * Initial patch to replace the direct call to landlock_release_inodes()

>   (requested by James Morris).

>   https://lore.kernel.org/lkml/alpine.LRH.2.21.2005150536440.7929@namei.org/

> ---

>  fs/super.c                    | 1 +

>  include/linux/lsm_hook_defs.h | 1 +

>  include/linux/lsm_hooks.h     | 2 ++

>  include/linux/security.h      | 4 ++++

>  security/security.c           | 5 +++++

>  5 files changed, 13 insertions(+)

> 

> diff --git a/fs/super.c b/fs/super.c

> index 2c6cdea2ab2d..c3c5178cde65 100644

> --- a/fs/super.c

> +++ b/fs/super.c

> @@ -454,6 +454,7 @@ void generic_shutdown_super(struct super_block *sb)

>  		evict_inodes(sb);

>  		/* only nonzero refcount inodes can have marks */

>  		fsnotify_sb_delete(sb);

> +		security_sb_delete(sb);

>  

>  		if (sb->s_dio_done_wq) {

>  			destroy_workqueue(sb->s_dio_done_wq);

> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h

> index 7aaa753b8608..32472b3849bc 100644

> --- a/include/linux/lsm_hook_defs.h

> +++ b/include/linux/lsm_hook_defs.h

> @@ -59,6 +59,7 @@ LSM_HOOK(int, 0, fs_context_dup, struct fs_context *fc,

>  LSM_HOOK(int, -ENOPARAM, fs_context_parse_param, struct fs_context *fc,

>  	 struct fs_parameter *param)

>  LSM_HOOK(int, 0, sb_alloc_security, struct super_block *sb)

> +LSM_HOOK(void, LSM_RET_VOID, sb_delete, struct super_block *sb)

>  LSM_HOOK(void, LSM_RET_VOID, sb_free_security, struct super_block *sb)

>  LSM_HOOK(void, LSM_RET_VOID, sb_free_mnt_opts, void *mnt_opts)

>  LSM_HOOK(int, 0, sb_eat_lsm_opts, char *orig, void **mnt_opts)

> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h

> index 970106d98306..e339b201f79b 100644

> --- a/include/linux/lsm_hooks.h

> +++ b/include/linux/lsm_hooks.h

> @@ -108,6 +108,8 @@

>   *	allocated.

>   *	@sb contains the super_block structure to be modified.

>   *	Return 0 if operation was successful.

> + * @sb_delete:

> + *	Release objects tied to a superblock (e.g. inodes).


It's customary here to add the line detailing the @sb argument.

>   * @sb_free_security:

>   *	Deallocate and clear the sb->s_security field.

>   *	@sb contains the super_block structure to be modified.

> diff --git a/include/linux/security.h b/include/linux/security.h

> index c35ea0ffccd9..c41a94e29b62 100644

> --- a/include/linux/security.h

> +++ b/include/linux/security.h

> @@ -288,6 +288,7 @@ void security_bprm_committed_creds(struct linux_binprm *bprm);

>  int security_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc);

>  int security_fs_context_parse_param(struct fs_context *fc, struct fs_parameter *param);

>  int security_sb_alloc(struct super_block *sb);

> +void security_sb_delete(struct super_block *sb);

>  void security_sb_free(struct super_block *sb);

>  void security_free_mnt_opts(void **mnt_opts);

>  int security_sb_eat_lsm_opts(char *options, void **mnt_opts);

> @@ -620,6 +621,9 @@ static inline int security_sb_alloc(struct super_block *sb)

>  	return 0;

>  }

>  

> +static inline void security_sb_delete(struct super_block *sb)

> +{ }

> +

>  static inline void security_sb_free(struct super_block *sb)

>  { }

>  

> diff --git a/security/security.c b/security/security.c

> index 9f979d4afe6c..1b4a73b2549a 100644

> --- a/security/security.c

> +++ b/security/security.c

> @@ -900,6 +900,11 @@ int security_sb_alloc(struct super_block *sb)

>  	return rc;

>  }

>  

> +void security_sb_delete(struct super_block *sb)

> +{

> +	call_void_hook(sb_delete, sb);

> +}

> +

>  void security_sb_free(struct super_block *sb)

>  {

>  	call_void_hook(sb_free_security, sb);

> -- 

> 2.30.0
Mickaël Salaün Feb. 5, 2021, 2:57 p.m. UTC | #2
On 05/02/2021 15:21, Serge E. Hallyn wrote:
> On Tue, Feb 02, 2021 at 05:27:04PM +0100, Mickaël Salaün wrote:

>> From: Mickaël Salaün <mic@linux.microsoft.com>

>>

>> The sb_delete security hook is called when shutting down a superblock,

>> which may be useful to release kernel objects tied to the superblock's

>> lifetime (e.g. inodes).

>>

>> This new hook is needed by Landlock to release (ephemerally) tagged

>> struct inodes.  This comes from the unprivileged nature of Landlock

>> described in the next commit.

>>

>> Cc: Al Viro <viro@zeniv.linux.org.uk>

>> Cc: James Morris <jmorris@namei.org>

>> Cc: Kees Cook <keescook@chromium.org>

>> Cc: Serge E. Hallyn <serge@hallyn.com>

> 

> One note below, but

> 

> Acked-by: Serge Hallyn <serge@hallyn.com>

> 

>> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>

>> Reviewed-by: Jann Horn <jannh@google.com>

>> ---

>>

>> Changes since v22:

>> * Add Reviewed-by: Jann Horn <jannh@google.com>

>>

>> Changes since v17:

>> * Initial patch to replace the direct call to landlock_release_inodes()

>>   (requested by James Morris).

>>   https://lore.kernel.org/lkml/alpine.LRH.2.21.2005150536440.7929@namei.org/

>> ---

>>  fs/super.c                    | 1 +

>>  include/linux/lsm_hook_defs.h | 1 +

>>  include/linux/lsm_hooks.h     | 2 ++

>>  include/linux/security.h      | 4 ++++

>>  security/security.c           | 5 +++++

>>  5 files changed, 13 insertions(+)

>>

>> diff --git a/fs/super.c b/fs/super.c

>> index 2c6cdea2ab2d..c3c5178cde65 100644

>> --- a/fs/super.c

>> +++ b/fs/super.c

>> @@ -454,6 +454,7 @@ void generic_shutdown_super(struct super_block *sb)

>>  		evict_inodes(sb);

>>  		/* only nonzero refcount inodes can have marks */

>>  		fsnotify_sb_delete(sb);

>> +		security_sb_delete(sb);

>>  

>>  		if (sb->s_dio_done_wq) {

>>  			destroy_workqueue(sb->s_dio_done_wq);

>> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h

>> index 7aaa753b8608..32472b3849bc 100644

>> --- a/include/linux/lsm_hook_defs.h

>> +++ b/include/linux/lsm_hook_defs.h

>> @@ -59,6 +59,7 @@ LSM_HOOK(int, 0, fs_context_dup, struct fs_context *fc,

>>  LSM_HOOK(int, -ENOPARAM, fs_context_parse_param, struct fs_context *fc,

>>  	 struct fs_parameter *param)

>>  LSM_HOOK(int, 0, sb_alloc_security, struct super_block *sb)

>> +LSM_HOOK(void, LSM_RET_VOID, sb_delete, struct super_block *sb)

>>  LSM_HOOK(void, LSM_RET_VOID, sb_free_security, struct super_block *sb)

>>  LSM_HOOK(void, LSM_RET_VOID, sb_free_mnt_opts, void *mnt_opts)

>>  LSM_HOOK(int, 0, sb_eat_lsm_opts, char *orig, void **mnt_opts)

>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h

>> index 970106d98306..e339b201f79b 100644

>> --- a/include/linux/lsm_hooks.h

>> +++ b/include/linux/lsm_hooks.h

>> @@ -108,6 +108,8 @@

>>   *	allocated.

>>   *	@sb contains the super_block structure to be modified.

>>   *	Return 0 if operation was successful.

>> + * @sb_delete:

>> + *	Release objects tied to a superblock (e.g. inodes).

> 

> It's customary here to add the line detailing the @sb argument.


What about "@sb contains the super_block structure being released."?

> 

>>   * @sb_free_security:

>>   *	Deallocate and clear the sb->s_security field.

>>   *	@sb contains the super_block structure to be modified.

>> diff --git a/include/linux/security.h b/include/linux/security.h

>> index c35ea0ffccd9..c41a94e29b62 100644

>> --- a/include/linux/security.h

>> +++ b/include/linux/security.h

>> @@ -288,6 +288,7 @@ void security_bprm_committed_creds(struct linux_binprm *bprm);

>>  int security_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc);

>>  int security_fs_context_parse_param(struct fs_context *fc, struct fs_parameter *param);

>>  int security_sb_alloc(struct super_block *sb);

>> +void security_sb_delete(struct super_block *sb);

>>  void security_sb_free(struct super_block *sb);

>>  void security_free_mnt_opts(void **mnt_opts);

>>  int security_sb_eat_lsm_opts(char *options, void **mnt_opts);

>> @@ -620,6 +621,9 @@ static inline int security_sb_alloc(struct super_block *sb)

>>  	return 0;

>>  }

>>  

>> +static inline void security_sb_delete(struct super_block *sb)

>> +{ }

>> +

>>  static inline void security_sb_free(struct super_block *sb)

>>  { }

>>  

>> diff --git a/security/security.c b/security/security.c

>> index 9f979d4afe6c..1b4a73b2549a 100644

>> --- a/security/security.c

>> +++ b/security/security.c

>> @@ -900,6 +900,11 @@ int security_sb_alloc(struct super_block *sb)

>>  	return rc;

>>  }

>>  

>> +void security_sb_delete(struct super_block *sb)

>> +{

>> +	call_void_hook(sb_delete, sb);

>> +}

>> +

>>  void security_sb_free(struct super_block *sb)

>>  {

>>  	call_void_hook(sb_free_security, sb);

>> -- 

>> 2.30.0
Serge E. Hallyn Feb. 7, 2021, 4:18 a.m. UTC | #3
On Fri, Feb 05, 2021 at 03:57:37PM +0100, Mickaël Salaün wrote:
> 

> On 05/02/2021 15:21, Serge E. Hallyn wrote:

> > On Tue, Feb 02, 2021 at 05:27:04PM +0100, Mickaël Salaün wrote:

> >> From: Mickaël Salaün <mic@linux.microsoft.com>

> >>

> >> The sb_delete security hook is called when shutting down a superblock,

> >> which may be useful to release kernel objects tied to the superblock's

> >> lifetime (e.g. inodes).

> >>

> >> This new hook is needed by Landlock to release (ephemerally) tagged

> >> struct inodes.  This comes from the unprivileged nature of Landlock

> >> described in the next commit.

> >>

> >> Cc: Al Viro <viro@zeniv.linux.org.uk>

> >> Cc: James Morris <jmorris@namei.org>

> >> Cc: Kees Cook <keescook@chromium.org>

> >> Cc: Serge E. Hallyn <serge@hallyn.com>

> > 

> > One note below, but

> > 

> > Acked-by: Serge Hallyn <serge@hallyn.com>

> > 

> >> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>

> >> Reviewed-by: Jann Horn <jannh@google.com>

> >> ---

> >>

> >> Changes since v22:

> >> * Add Reviewed-by: Jann Horn <jannh@google.com>

> >>

> >> Changes since v17:

> >> * Initial patch to replace the direct call to landlock_release_inodes()

> >>   (requested by James Morris).

> >>   https://lore.kernel.org/lkml/alpine.LRH.2.21.2005150536440.7929@namei.org/

> >> ---

> >>  fs/super.c                    | 1 +

> >>  include/linux/lsm_hook_defs.h | 1 +

> >>  include/linux/lsm_hooks.h     | 2 ++

> >>  include/linux/security.h      | 4 ++++

> >>  security/security.c           | 5 +++++

> >>  5 files changed, 13 insertions(+)

> >>

> >> diff --git a/fs/super.c b/fs/super.c

> >> index 2c6cdea2ab2d..c3c5178cde65 100644

> >> --- a/fs/super.c

> >> +++ b/fs/super.c

> >> @@ -454,6 +454,7 @@ void generic_shutdown_super(struct super_block *sb)

> >>  		evict_inodes(sb);

> >>  		/* only nonzero refcount inodes can have marks */

> >>  		fsnotify_sb_delete(sb);

> >> +		security_sb_delete(sb);

> >>  

> >>  		if (sb->s_dio_done_wq) {

> >>  			destroy_workqueue(sb->s_dio_done_wq);

> >> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h

> >> index 7aaa753b8608..32472b3849bc 100644

> >> --- a/include/linux/lsm_hook_defs.h

> >> +++ b/include/linux/lsm_hook_defs.h

> >> @@ -59,6 +59,7 @@ LSM_HOOK(int, 0, fs_context_dup, struct fs_context *fc,

> >>  LSM_HOOK(int, -ENOPARAM, fs_context_parse_param, struct fs_context *fc,

> >>  	 struct fs_parameter *param)

> >>  LSM_HOOK(int, 0, sb_alloc_security, struct super_block *sb)

> >> +LSM_HOOK(void, LSM_RET_VOID, sb_delete, struct super_block *sb)

> >>  LSM_HOOK(void, LSM_RET_VOID, sb_free_security, struct super_block *sb)

> >>  LSM_HOOK(void, LSM_RET_VOID, sb_free_mnt_opts, void *mnt_opts)

> >>  LSM_HOOK(int, 0, sb_eat_lsm_opts, char *orig, void **mnt_opts)

> >> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h

> >> index 970106d98306..e339b201f79b 100644

> >> --- a/include/linux/lsm_hooks.h

> >> +++ b/include/linux/lsm_hooks.h

> >> @@ -108,6 +108,8 @@

> >>   *	allocated.

> >>   *	@sb contains the super_block structure to be modified.

> >>   *	Return 0 if operation was successful.

> >> + * @sb_delete:

> >> + *	Release objects tied to a superblock (e.g. inodes).

> > 

> > It's customary here to add the line detailing the @sb argument.

> 

> What about "@sb contains the super_block structure being released."?


That's good.  Thanks.

> > 

> >>   * @sb_free_security:

> >>   *	Deallocate and clear the sb->s_security field.

> >>   *	@sb contains the super_block structure to be modified.

> >> diff --git a/include/linux/security.h b/include/linux/security.h

> >> index c35ea0ffccd9..c41a94e29b62 100644

> >> --- a/include/linux/security.h

> >> +++ b/include/linux/security.h

> >> @@ -288,6 +288,7 @@ void security_bprm_committed_creds(struct linux_binprm *bprm);

> >>  int security_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc);

> >>  int security_fs_context_parse_param(struct fs_context *fc, struct fs_parameter *param);

> >>  int security_sb_alloc(struct super_block *sb);

> >> +void security_sb_delete(struct super_block *sb);

> >>  void security_sb_free(struct super_block *sb);

> >>  void security_free_mnt_opts(void **mnt_opts);

> >>  int security_sb_eat_lsm_opts(char *options, void **mnt_opts);

> >> @@ -620,6 +621,9 @@ static inline int security_sb_alloc(struct super_block *sb)

> >>  	return 0;

> >>  }

> >>  

> >> +static inline void security_sb_delete(struct super_block *sb)

> >> +{ }

> >> +

> >>  static inline void security_sb_free(struct super_block *sb)

> >>  { }

> >>  

> >> diff --git a/security/security.c b/security/security.c

> >> index 9f979d4afe6c..1b4a73b2549a 100644

> >> --- a/security/security.c

> >> +++ b/security/security.c

> >> @@ -900,6 +900,11 @@ int security_sb_alloc(struct super_block *sb)

> >>  	return rc;

> >>  }

> >>  

> >> +void security_sb_delete(struct super_block *sb)

> >> +{

> >> +	call_void_hook(sb_delete, sb);

> >> +}

> >> +

> >>  void security_sb_free(struct super_block *sb)

> >>  {

> >>  	call_void_hook(sb_free_security, sb);

> >> -- 

> >> 2.30.0
diff mbox series

Patch

diff --git a/fs/super.c b/fs/super.c
index 2c6cdea2ab2d..c3c5178cde65 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -454,6 +454,7 @@  void generic_shutdown_super(struct super_block *sb)
 		evict_inodes(sb);
 		/* only nonzero refcount inodes can have marks */
 		fsnotify_sb_delete(sb);
+		security_sb_delete(sb);
 
 		if (sb->s_dio_done_wq) {
 			destroy_workqueue(sb->s_dio_done_wq);
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 7aaa753b8608..32472b3849bc 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -59,6 +59,7 @@  LSM_HOOK(int, 0, fs_context_dup, struct fs_context *fc,
 LSM_HOOK(int, -ENOPARAM, fs_context_parse_param, struct fs_context *fc,
 	 struct fs_parameter *param)
 LSM_HOOK(int, 0, sb_alloc_security, struct super_block *sb)
+LSM_HOOK(void, LSM_RET_VOID, sb_delete, struct super_block *sb)
 LSM_HOOK(void, LSM_RET_VOID, sb_free_security, struct super_block *sb)
 LSM_HOOK(void, LSM_RET_VOID, sb_free_mnt_opts, void *mnt_opts)
 LSM_HOOK(int, 0, sb_eat_lsm_opts, char *orig, void **mnt_opts)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 970106d98306..e339b201f79b 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -108,6 +108,8 @@ 
  *	allocated.
  *	@sb contains the super_block structure to be modified.
  *	Return 0 if operation was successful.
+ * @sb_delete:
+ *	Release objects tied to a superblock (e.g. inodes).
  * @sb_free_security:
  *	Deallocate and clear the sb->s_security field.
  *	@sb contains the super_block structure to be modified.
diff --git a/include/linux/security.h b/include/linux/security.h
index c35ea0ffccd9..c41a94e29b62 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -288,6 +288,7 @@  void security_bprm_committed_creds(struct linux_binprm *bprm);
 int security_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc);
 int security_fs_context_parse_param(struct fs_context *fc, struct fs_parameter *param);
 int security_sb_alloc(struct super_block *sb);
+void security_sb_delete(struct super_block *sb);
 void security_sb_free(struct super_block *sb);
 void security_free_mnt_opts(void **mnt_opts);
 int security_sb_eat_lsm_opts(char *options, void **mnt_opts);
@@ -620,6 +621,9 @@  static inline int security_sb_alloc(struct super_block *sb)
 	return 0;
 }
 
+static inline void security_sb_delete(struct super_block *sb)
+{ }
+
 static inline void security_sb_free(struct super_block *sb)
 { }
 
diff --git a/security/security.c b/security/security.c
index 9f979d4afe6c..1b4a73b2549a 100644
--- a/security/security.c
+++ b/security/security.c
@@ -900,6 +900,11 @@  int security_sb_alloc(struct super_block *sb)
 	return rc;
 }
 
+void security_sb_delete(struct super_block *sb)
+{
+	call_void_hook(sb_delete, sb);
+}
+
 void security_sb_free(struct super_block *sb)
 {
 	call_void_hook(sb_free_security, sb);