This patch adds two new hooks "hook_file_set_fowner" and
"hook_file_free_security" to set and release a pointer to the
domain of the file owner. This pointer "fown_domain" in
"landlock_file_security" will be used in "file_send_sigiotask"
to check if the process can send a signal.

Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
---
 security/landlock/fs.c   | 18 ++++++++++++++++++
 security/landlock/fs.h   |  6 ++++++
 security/landlock/task.c | 27 +++++++++++++++++++++++++++
 3 files changed, 51 insertions(+)

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 7877a64cc6b8..d05f0e9c5e54 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1636,6 +1636,21 @@ static int hook_file_ioctl_compat(struct file *file, unsigned int cmd,
 	return -EACCES;
 }
 
+static void hook_file_set_fowner(struct file *file)
+{
+	write_lock_irq(&file->f_owner.lock);
+	landlock_file(file)->fown_domain = landlock_get_current_domain();
+	landlock_get_ruleset(landlock_file(file)->fown_domain);
+	write_unlock_irq(&file->f_owner.lock);
+}
+
+static void hook_file_free_security(struct file *file)
+{
+	write_lock_irq(&file->f_owner.lock);
+	landlock_put_ruleset(landlock_file(file)->fown_domain);
+	write_unlock_irq(&file->f_owner.lock);
+}
+
 static struct security_hook_list landlock_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(inode_free_security, hook_inode_free_security),
 
@@ -1660,6 +1675,9 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(file_truncate, hook_file_truncate),
 	LSM_HOOK_INIT(file_ioctl, hook_file_ioctl),
 	LSM_HOOK_INIT(file_ioctl_compat, hook_file_ioctl_compat),
+
+	LSM_HOOK_INIT(file_set_fowner, hook_file_set_fowner),
+	LSM_HOOK_INIT(file_free_security, hook_file_free_security),
 };
 
 __init void landlock_add_fs_hooks(void)
diff --git a/security/landlock/fs.h b/security/landlock/fs.h
index 488e4813680a..6054563295d8 100644
--- a/security/landlock/fs.h
+++ b/security/landlock/fs.h
@@ -52,6 +52,12 @@ struct landlock_file_security {
 	 * needed to authorize later operations on the open file.
 	 */
 	access_mask_t allowed_access;
+	/**
+	 * @fown_domain: A pointer to a &landlock_ruleset of the process own
+	 * the file. This ruleset is protected by fowner_struct.lock same as
+	 * pid, uid, euid fields in fown_struct.
+	 */
+	struct landlock_ruleset *fown_domain;
 };
 
 /**
diff --git a/security/landlock/task.c b/security/landlock/task.c
index 9de96a5005c4..568292dbfe7d 100644
--- a/security/landlock/task.c
+++ b/security/landlock/task.c
@@ -18,6 +18,7 @@
 
 #include "common.h"
 #include "cred.h"
+#include "fs.h"
 #include "ruleset.h"
 #include "setup.h"
 #include "task.h"
@@ -261,12 +262,38 @@ static int hook_task_kill(struct task_struct *const p,
 	return 0;
 }
 
+static int hook_file_send_sigiotask(struct task_struct *tsk,
+				    struct fown_struct *fown, int signum)
+{
+	struct file *file;
+	bool is_scoped;
+	const struct landlock_ruleset *dom, *target_dom;
+
+	/* struct fown_struct is never outside the context of a struct file */
+	file = container_of(fown, struct file, f_owner);
+
+	read_lock_irq(&file->f_owner.lock);
+	dom = landlock_file(file)->fown_domain;
+	read_unlock_irq(&file->f_owner.lock);
+	if (!dom)
+		return 0;
+
+	rcu_read_lock();
+	target_dom = landlock_get_task_domain(tsk);
+	is_scoped = domain_is_scoped(dom, target_dom, LANDLOCK_SCOPED_SIGNAL);
+	rcu_read_unlock();
+	if (is_scoped)
+		return -EPERM;
+	return 0;
+}
+
 static struct security_hook_list landlock_hooks[] __ro_after_init = {
 	LSM_HOOK_INIT(ptrace_access_check, hook_ptrace_access_check),
 	LSM_HOOK_INIT(ptrace_traceme, hook_ptrace_traceme),
 	LSM_HOOK_INIT(unix_stream_connect, hook_unix_stream_connect),
 	LSM_HOOK_INIT(unix_may_send, hook_unix_may_send),
 	LSM_HOOK_INIT(task_kill, hook_task_kill),
+	LSM_HOOK_INIT(file_send_sigiotask, hook_file_send_sigiotask),
 };
 
 __init void landlock_add_task_hooks(void)
-- 
2.34.1

On Thu, Aug 15, 2024 at 12:29:21PM -0600, Tahera Fahimi wrote:
> This patch adds two new hooks "hook_file_set_fowner" and
> "hook_file_free_security" to set and release a pointer to the
> domain of the file owner. This pointer "fown_domain" in
> "landlock_file_security" will be used in "file_send_sigiotask"
> to check if the process can send a signal.
> 
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> ---
>  security/landlock/fs.c   | 18 ++++++++++++++++++
>  security/landlock/fs.h   |  6 ++++++
>  security/landlock/task.c | 27 +++++++++++++++++++++++++++
>  3 files changed, 51 insertions(+)

Please squash this patch with the previous one, both are enforcing the
signal scoping restriction with LANDLOCK_SCOPED_SIGNAL.

You'll also need to update the scoped_test.c file with
LANDLOCK_SCOPED_SIGNAL (in this same squashed patch).

On Thu, Aug 15, 2024 at 12:29:21PM -0600, Tahera Fahimi wrote:
> This patch adds two new hooks "hook_file_set_fowner" and
> "hook_file_free_security" to set and release a pointer to the
> domain of the file owner. This pointer "fown_domain" in
> "landlock_file_security" will be used in "file_send_sigiotask"
> to check if the process can send a signal.

We need to make sure this file_send_sigiotask hook is useful and can be
triggered by user space code.  Currently, all tests pass without this
patch.

On Thu, Aug 15, 2024 at 8:29 PM Tahera Fahimi <fahimitahera@gmail.com> wrote:
> This patch adds two new hooks "hook_file_set_fowner" and
> "hook_file_free_security" to set and release a pointer to the
> domain of the file owner. This pointer "fown_domain" in
> "landlock_file_security" will be used in "file_send_sigiotask"
> to check if the process can send a signal.
>
> Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> ---
>  security/landlock/fs.c   | 18 ++++++++++++++++++
>  security/landlock/fs.h   |  6 ++++++
>  security/landlock/task.c | 27 +++++++++++++++++++++++++++
>  3 files changed, 51 insertions(+)
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 7877a64cc6b8..d05f0e9c5e54 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1636,6 +1636,21 @@ static int hook_file_ioctl_compat(struct file *file, unsigned int cmd,
>         return -EACCES;
>  }
>
> +static void hook_file_set_fowner(struct file *file)
> +{
> +       write_lock_irq(&file->f_owner.lock);

Before updating landlock_file(file)->fown_domain, this hook must also
drop a reference on the old domain - maybe by just calling
landlock_put_ruleset_deferred(landlock_file(file)->fown_domain) here.

> +       landlock_file(file)->fown_domain = landlock_get_current_domain();
> +       landlock_get_ruleset(landlock_file(file)->fown_domain);
> +       write_unlock_irq(&file->f_owner.lock);
> +}
> +
> +static void hook_file_free_security(struct file *file)
> +{
> +       write_lock_irq(&file->f_owner.lock);
> +       landlock_put_ruleset(landlock_file(file)->fown_domain);
> +       write_unlock_irq(&file->f_owner.lock);
> +}
> +
>  static struct security_hook_list landlock_hooks[] __ro_after_init = {
>         LSM_HOOK_INIT(inode_free_security, hook_inode_free_security),
>
> @@ -1660,6 +1675,9 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
>         LSM_HOOK_INIT(file_truncate, hook_file_truncate),
>         LSM_HOOK_INIT(file_ioctl, hook_file_ioctl),
>         LSM_HOOK_INIT(file_ioctl_compat, hook_file_ioctl_compat),
> +
> +       LSM_HOOK_INIT(file_set_fowner, hook_file_set_fowner),
> +       LSM_HOOK_INIT(file_free_security, hook_file_free_security),
>  };
>
>  __init void landlock_add_fs_hooks(void)
> diff --git a/security/landlock/fs.h b/security/landlock/fs.h
> index 488e4813680a..6054563295d8 100644
> --- a/security/landlock/fs.h
> +++ b/security/landlock/fs.h
> @@ -52,6 +52,12 @@ struct landlock_file_security {
>          * needed to authorize later operations on the open file.
>          */
>         access_mask_t allowed_access;
> +       /**
> +        * @fown_domain: A pointer to a &landlock_ruleset of the process own
> +        * the file. This ruleset is protected by fowner_struct.lock same as
> +        * pid, uid, euid fields in fown_struct.
> +        */
> +       struct landlock_ruleset *fown_domain;
>  };
>
>  /**
> diff --git a/security/landlock/task.c b/security/landlock/task.c
> index 9de96a5005c4..568292dbfe7d 100644
> --- a/security/landlock/task.c
> +++ b/security/landlock/task.c
> @@ -18,6 +18,7 @@
>
>  #include "common.h"
>  #include "cred.h"
> +#include "fs.h"
>  #include "ruleset.h"
>  #include "setup.h"
>  #include "task.h"
> @@ -261,12 +262,38 @@ static int hook_task_kill(struct task_struct *const p,
>         return 0;
>  }
>
> +static int hook_file_send_sigiotask(struct task_struct *tsk,
> +                                   struct fown_struct *fown, int signum)
> +{
> +       struct file *file;
> +       bool is_scoped;
> +       const struct landlock_ruleset *dom, *target_dom;
> +
> +       /* struct fown_struct is never outside the context of a struct file */
> +       file = container_of(fown, struct file, f_owner);
> +
> +       read_lock_irq(&file->f_owner.lock);
> +       dom = landlock_file(file)->fown_domain;
> +       read_unlock_irq(&file->f_owner.lock);

At this point, the ->fown_domain pointer could concurrently change,
and (once you apply my suggestion above) the old ->fown_domain could
therefore be freed concurrently. One way to avoid that would be to use
landlock_get_ruleset() to grab a reference before calling
read_unlock_irq(), and drop that reference with
landlock_put_ruleset_deferred() before exiting from this function.

> +       if (!dom)
> +               return 0;
> +
> +       rcu_read_lock();
> +       target_dom = landlock_get_task_domain(tsk);
> +       is_scoped = domain_is_scoped(dom, target_dom, LANDLOCK_SCOPED_SIGNAL);
> +       rcu_read_unlock();
> +       if (is_scoped)
> +               return -EPERM;
> +       return 0;
> +}
> +
>  static struct security_hook_list landlock_hooks[] __ro_after_init = {
>         LSM_HOOK_INIT(ptrace_access_check, hook_ptrace_access_check),
>         LSM_HOOK_INIT(ptrace_traceme, hook_ptrace_traceme),
>         LSM_HOOK_INIT(unix_stream_connect, hook_unix_stream_connect),
>         LSM_HOOK_INIT(unix_may_send, hook_unix_may_send),
>         LSM_HOOK_INIT(task_kill, hook_task_kill),
> +       LSM_HOOK_INIT(file_send_sigiotask, hook_file_send_sigiotask),
>  };
>
>  __init void landlock_add_task_hooks(void)
> --
> 2.34.1
>

On Thu, Aug 15, 2024 at 10:25:15PM +0200, Jann Horn wrote:
> On Thu, Aug 15, 2024 at 8:29 PM Tahera Fahimi <fahimitahera@gmail.com> wrote:
> > This patch adds two new hooks "hook_file_set_fowner" and
> > "hook_file_free_security" to set and release a pointer to the
> > domain of the file owner. This pointer "fown_domain" in
> > "landlock_file_security" will be used in "file_send_sigiotask"
> > to check if the process can send a signal.
> >
> > Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> > ---
> >  security/landlock/fs.c   | 18 ++++++++++++++++++
> >  security/landlock/fs.h   |  6 ++++++
> >  security/landlock/task.c | 27 +++++++++++++++++++++++++++
> >  3 files changed, 51 insertions(+)
> >
> > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> > index 7877a64cc6b8..d05f0e9c5e54 100644
> > --- a/security/landlock/fs.c
> > +++ b/security/landlock/fs.c
> > @@ -1636,6 +1636,21 @@ static int hook_file_ioctl_compat(struct file *file, unsigned int cmd,
> >         return -EACCES;
> >  }
> >
> > +static void hook_file_set_fowner(struct file *file)
> > +{
> > +       write_lock_irq(&file->f_owner.lock);
> 
> Before updating landlock_file(file)->fown_domain, this hook must also
> drop a reference on the old domain - maybe by just calling
> landlock_put_ruleset_deferred(landlock_file(file)->fown_domain) here.
Hi Jann,

Thanks for the feedback :)
It totally make sense.
> > +       landlock_file(file)->fown_domain = landlock_get_current_domain();
> > +       landlock_get_ruleset(landlock_file(file)->fown_domain);
> > +       write_unlock_irq(&file->f_owner.lock);
> > +}
> > +
> > +static void hook_file_free_security(struct file *file)
> > +{
> > +       write_lock_irq(&file->f_owner.lock);
> > +       landlock_put_ruleset(landlock_file(file)->fown_domain);
I was thinking of if we can replace this landlock_put_ruleset with
landlock_put_ruleset_deferred. In this case, it would be better use of
handling the lock?

> > +       write_unlock_irq(&file->f_owner.lock);
> > +}
> > +
> >  static struct security_hook_list landlock_hooks[] __ro_after_init = {
> >         LSM_HOOK_INIT(inode_free_security, hook_inode_free_security),
> >
> > @@ -1660,6 +1675,9 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
> >         LSM_HOOK_INIT(file_truncate, hook_file_truncate),
> >         LSM_HOOK_INIT(file_ioctl, hook_file_ioctl),
> >         LSM_HOOK_INIT(file_ioctl_compat, hook_file_ioctl_compat),
> > +
> > +       LSM_HOOK_INIT(file_set_fowner, hook_file_set_fowner),
> > +       LSM_HOOK_INIT(file_free_security, hook_file_free_security),
> >  };
> >
> >  __init void landlock_add_fs_hooks(void)
> > diff --git a/security/landlock/fs.h b/security/landlock/fs.h
> > index 488e4813680a..6054563295d8 100644
> > --- a/security/landlock/fs.h
> > +++ b/security/landlock/fs.h
> > @@ -52,6 +52,12 @@ struct landlock_file_security {
> >          * needed to authorize later operations on the open file.
> >          */
> >         access_mask_t allowed_access;
> > +       /**
> > +        * @fown_domain: A pointer to a &landlock_ruleset of the process own
> > +        * the file. This ruleset is protected by fowner_struct.lock same as
> > +        * pid, uid, euid fields in fown_struct.
> > +        */
> > +       struct landlock_ruleset *fown_domain;
> >  };
> >
> >  /**
> > diff --git a/security/landlock/task.c b/security/landlock/task.c
> > index 9de96a5005c4..568292dbfe7d 100644
> > --- a/security/landlock/task.c
> > +++ b/security/landlock/task.c
> > @@ -18,6 +18,7 @@
> >
> >  #include "common.h"
> >  #include "cred.h"
> > +#include "fs.h"
> >  #include "ruleset.h"
> >  #include "setup.h"
> >  #include "task.h"
> > @@ -261,12 +262,38 @@ static int hook_task_kill(struct task_struct *const p,
> >         return 0;
> >  }
> >
> > +static int hook_file_send_sigiotask(struct task_struct *tsk,
> > +                                   struct fown_struct *fown, int signum)
> > +{
> > +       struct file *file;
> > +       bool is_scoped;
> > +       const struct landlock_ruleset *dom, *target_dom;
> > +
> > +       /* struct fown_struct is never outside the context of a struct file */
> > +       file = container_of(fown, struct file, f_owner);
> > +
> > +       read_lock_irq(&file->f_owner.lock);
> > +       dom = landlock_file(file)->fown_domain;
> > +       read_unlock_irq(&file->f_owner.lock);
> 
> At this point, the ->fown_domain pointer could concurrently change,
> and (once you apply my suggestion above) the old ->fown_domain could
> therefore be freed concurrently. One way to avoid that would be to use
> landlock_get_ruleset() to grab a reference before calling
> read_unlock_irq(), and drop that reference with
> landlock_put_ruleset_deferred() before exiting from this function.
Correct, I applied the changes. 
> > +       if (!dom)
> > +               return 0;
> > +
> > +       rcu_read_lock();
> > +       target_dom = landlock_get_task_domain(tsk);
> > +       is_scoped = domain_is_scoped(dom, target_dom, LANDLOCK_SCOPED_SIGNAL);
> > +       rcu_read_unlock();
> > +       if (is_scoped)
> > +               return -EPERM;
> > +       return 0;
> > +}
> > +
> >  static struct security_hook_list landlock_hooks[] __ro_after_init = {
> >         LSM_HOOK_INIT(ptrace_access_check, hook_ptrace_access_check),
> >         LSM_HOOK_INIT(ptrace_traceme, hook_ptrace_traceme),
> >         LSM_HOOK_INIT(unix_stream_connect, hook_unix_stream_connect),
> >         LSM_HOOK_INIT(unix_may_send, hook_unix_may_send),
> >         LSM_HOOK_INIT(task_kill, hook_task_kill),
> > +       LSM_HOOK_INIT(file_send_sigiotask, hook_file_send_sigiotask),
> >  };
> >
> >  __init void landlock_add_task_hooks(void)
> > --
> > 2.34.1
> >

On Thu, Aug 15, 2024 at 11:28 PM Tahera Fahimi <fahimitahera@gmail.com> wrote:
>
> On Thu, Aug 15, 2024 at 10:25:15PM +0200, Jann Horn wrote:
> > On Thu, Aug 15, 2024 at 8:29 PM Tahera Fahimi <fahimitahera@gmail.com> wrote:
> > > This patch adds two new hooks "hook_file_set_fowner" and
> > > "hook_file_free_security" to set and release a pointer to the
> > > domain of the file owner. This pointer "fown_domain" in
> > > "landlock_file_security" will be used in "file_send_sigiotask"
> > > to check if the process can send a signal.
> > >
> > > Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> > > ---
> > >  security/landlock/fs.c   | 18 ++++++++++++++++++
> > >  security/landlock/fs.h   |  6 ++++++
> > >  security/landlock/task.c | 27 +++++++++++++++++++++++++++
> > >  3 files changed, 51 insertions(+)
> > >
> > > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> > > index 7877a64cc6b8..d05f0e9c5e54 100644
> > > --- a/security/landlock/fs.c
> > > +++ b/security/landlock/fs.c
> > > @@ -1636,6 +1636,21 @@ static int hook_file_ioctl_compat(struct file *file, unsigned int cmd,
> > >         return -EACCES;
> > >  }
> > >
> > > +static void hook_file_set_fowner(struct file *file)
> > > +{
> > > +       write_lock_irq(&file->f_owner.lock);
> >
> > Before updating landlock_file(file)->fown_domain, this hook must also
> > drop a reference on the old domain - maybe by just calling
> > landlock_put_ruleset_deferred(landlock_file(file)->fown_domain) here.
> Hi Jann,
>
> Thanks for the feedback :)
> It totally make sense.
> > > +       landlock_file(file)->fown_domain = landlock_get_current_domain();
> > > +       landlock_get_ruleset(landlock_file(file)->fown_domain);
> > > +       write_unlock_irq(&file->f_owner.lock);
> > > +}
> > > +
> > > +static void hook_file_free_security(struct file *file)
> > > +{
> > > +       write_lock_irq(&file->f_owner.lock);
> > > +       landlock_put_ruleset(landlock_file(file)->fown_domain);
> I was thinking of if we can replace this landlock_put_ruleset with
> landlock_put_ruleset_deferred. In this case, it would be better use of
> handling the lock?

I don't think you have to take the "file->f_owner.lock" in this hook -
the file has already been torn down pretty far, nothing is going to be
able to trigger the file_set_fowner hook anymore.

But either way, you're right that we can't just use
landlock_put_ruleset() here because landlock_put_ruleset() can sleep
and the file_free_security hook can be invoked from non-sleepable
context. (This only happens when fput() directly calls file_free(),
and I think that only happens with ->fown_domain==NULL, so technically
it would also be fine to do something like "if (domain)
landlock_put_ruleset(domain);".)
If you test your current code in a kernel that was built with
CONFIG_DEBUG_ATOMIC_SLEEP=y, this will probably print an warning
message in the kernel log (dmesg). You're right that using
landlock_put_ruleset_deferred() instead would fix that.

I think the right solution here is probably just to do:

static void hook_file_free_security(struct file *file)
{
  landlock_put_ruleset_deferred(landlock_file(file)->fown_domain);
}

Alternatively it would also work to do this - this code is probably a
bit more efficient but also a little less clear:

static void hook_file_free_security(struct file *file)
{
  /* don't trigger might_sleep() for tearing down unopened file */
  if (landlock_file(file)->fown_domain)
    landlock_put_ruleset(landlock_file(file)->fown_domain);
}

>
> > > +       write_unlock_irq(&file->f_owner.lock);
> > > +}

On Fri, Aug 16, 2024 at 12:10:44AM +0200, Jann Horn wrote:
> On Thu, Aug 15, 2024 at 11:28 PM Tahera Fahimi <fahimitahera@gmail.com> wrote:
> >
> > On Thu, Aug 15, 2024 at 10:25:15PM +0200, Jann Horn wrote:
> > > On Thu, Aug 15, 2024 at 8:29 PM Tahera Fahimi <fahimitahera@gmail.com> wrote:
> > > > This patch adds two new hooks "hook_file_set_fowner" and
> > > > "hook_file_free_security" to set and release a pointer to the
> > > > domain of the file owner. This pointer "fown_domain" in
> > > > "landlock_file_security" will be used in "file_send_sigiotask"
> > > > to check if the process can send a signal.
> > > >
> > > > Signed-off-by: Tahera Fahimi <fahimitahera@gmail.com>
> > > > ---
> > > >  security/landlock/fs.c   | 18 ++++++++++++++++++
> > > >  security/landlock/fs.h   |  6 ++++++
> > > >  security/landlock/task.c | 27 +++++++++++++++++++++++++++
> > > >  3 files changed, 51 insertions(+)
> > > >
> > > > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> > > > index 7877a64cc6b8..d05f0e9c5e54 100644
> > > > --- a/security/landlock/fs.c
> > > > +++ b/security/landlock/fs.c
> > > > @@ -1636,6 +1636,21 @@ static int hook_file_ioctl_compat(struct file *file, unsigned int cmd,
> > > >         return -EACCES;
> > > >  }
> > > >
> > > > +static void hook_file_set_fowner(struct file *file)
> > > > +{
> > > > +       write_lock_irq(&file->f_owner.lock);
> > >
> > > Before updating landlock_file(file)->fown_domain, this hook must also
> > > drop a reference on the old domain - maybe by just calling
> > > landlock_put_ruleset_deferred(landlock_file(file)->fown_domain) here.
> > Hi Jann,
> >
> > Thanks for the feedback :)
> > It totally make sense.
> > > > +       landlock_file(file)->fown_domain = landlock_get_current_domain();
> > > > +       landlock_get_ruleset(landlock_file(file)->fown_domain);
> > > > +       write_unlock_irq(&file->f_owner.lock);
> > > > +}
> > > > +
> > > > +static void hook_file_free_security(struct file *file)
> > > > +{
> > > > +       write_lock_irq(&file->f_owner.lock);
> > > > +       landlock_put_ruleset(landlock_file(file)->fown_domain);
> > I was thinking of if we can replace this landlock_put_ruleset with
> > landlock_put_ruleset_deferred. In this case, it would be better use of
> > handling the lock?
> 
> I don't think you have to take the "file->f_owner.lock" in this hook -
> the file has already been torn down pretty far, nothing is going to be
> able to trigger the file_set_fowner hook anymore.
That's right. Thanks.

> But either way, you're right that we can't just use
> landlock_put_ruleset() here because landlock_put_ruleset() can sleep
> and the file_free_security hook can be invoked from non-sleepable
> context. (This only happens when fput() directly calls file_free(),
> and I think that only happens with ->fown_domain==NULL, so technically
> it would also be fine to do something like "if (domain)
> landlock_put_ruleset(domain);".)
> If you test your current code in a kernel that was built with
> CONFIG_DEBUG_ATOMIC_SLEEP=y, this will probably print an warning
> message in the kernel log (dmesg). You're right that using
> landlock_put_ruleset_deferred() instead would fix that.
> 
> I think the right solution here is probably just to do:
> 
> static void hook_file_free_security(struct file *file)
> {
>   landlock_put_ruleset_deferred(landlock_file(file)->fown_domain);
> }
I think I will stick to this one since it is easier to understand. 
> Alternatively it would also work to do this - this code is probably a
> bit more efficient but also a little less clear:
> 
> static void hook_file_free_security(struct file *file)
> {
>   /* don't trigger might_sleep() for tearing down unopened file */
>   if (landlock_file(file)->fown_domain)
>     landlock_put_ruleset(landlock_file(file)->fown_domain);
> }
> 
> >
> > > > +       write_unlock_irq(&file->f_owner.lock);
> > > > +}