[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20241109.oiyaa0Woh0Ee@digikod.net>
Date: Sat, 9 Nov 2024 19:47:35 +0100
From: Mickaël Salaün <mic@...ikod.net>
To: Günther Noack <gnoack@...gle.com>
Cc: Mikhail Ivanov <ivanov.mikhail1@...wei-partners.com>,
Konstantin Meskhidze <konstantin.meskhidze@...wei.com>, Paul Moore <paul@...l-moore.com>,
Tahera Fahimi <fahimitahera@...il.com>, linux-kernel@...r.kernel.org,
linux-security-module@...r.kernel.org
Subject: Re: [PATCH v4 3/3] landlock: Optimize scope enforcement
On Sat, Nov 09, 2024 at 07:30:28PM +0100, Günther Noack wrote:
> On Sat, Nov 09, 2024 at 12:08:56PM +0100, Mickaël Salaün wrote:
> > Do not walk through the domain hierarchy when the required scope is not
> > supported by this domain. This is the same approach as for filesystem
> > and network restrictions.
> >
> > Cc: Günther Noack <gnoack@...gle.com>
> > Cc: Mikhail Ivanov <ivanov.mikhail1@...wei-partners.com>
> > Cc: Tahera Fahimi <fahimitahera@...il.com>
> > Signed-off-by: Mickaël Salaün <mic@...ikod.net>
> > Link: https://lore.kernel.org/r/20241109110856.222842-4-mic@digikod.net
> > ---
> >
> > Changes since v2:
> > * Make the unix_scope variable global to the file and remove
> > previous get_current_unix_scope_domain().
> > ---
> > security/landlock/task.c | 18 +++++++++++++++---
> > 1 file changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/security/landlock/task.c b/security/landlock/task.c
> > index 4acbd7c40eee..dc7dab78392e 100644
> > --- a/security/landlock/task.c
> > +++ b/security/landlock/task.c
> > @@ -204,12 +204,17 @@ static bool is_abstract_socket(struct sock *const sock)
> > return false;
> > }
> >
> > +static const struct access_masks unix_scope = {
> > + .scope = LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET,
> > +};
>
> Optional nit: You could probably inline these two struct access_masks values
> into the respective functions where they are used. (But it's just a minor code
> style matter IMHO; both ways are fine.)
That was the case in a previous version, but because we'll need to have
it global for a next patch series, I already put it there to avoid
useless code change:
https://lore.kernel.org/all/20241022161009.982584-14-mic@digikod.net/
>
> > +
> > static int hook_unix_stream_connect(struct sock *const sock,
> > struct sock *const other,
> > struct sock *const newsk)
> > {
> > const struct landlock_ruleset *const dom =
> > - landlock_get_current_domain();
> > + landlock_get_applicable_domain(landlock_get_current_domain(),
> > + unix_scope);
> >
> > /* Quick return for non-landlocked tasks. */
> > if (!dom)
> > @@ -225,7 +230,8 @@ static int hook_unix_may_send(struct socket *const sock,
> > struct socket *const other)
> > {
> > const struct landlock_ruleset *const dom =
> > - landlock_get_current_domain();
> > + landlock_get_applicable_domain(landlock_get_current_domain(),
> > + unix_scope);
> >
> > if (!dom)
> > return 0;
> > @@ -243,6 +249,10 @@ static int hook_unix_may_send(struct socket *const sock,
> > return 0;
> > }
> >
> > +static const struct access_masks signal_scope = {
> > + .scope = LANDLOCK_SCOPE_SIGNAL,
> > +};
> > +
> > static int hook_task_kill(struct task_struct *const p,
> > struct kernel_siginfo *const info, const int sig,
> > const struct cred *const cred)
> > @@ -256,6 +266,7 @@ static int hook_task_kill(struct task_struct *const p,
> > } else {
> > dom = landlock_get_current_domain();
> > }
> > + dom = landlock_get_applicable_domain(dom, signal_scope);
> >
> > /* Quick return for non-landlocked tasks. */
> > if (!dom)
> > @@ -279,7 +290,8 @@ static int hook_file_send_sigiotask(struct task_struct *tsk,
> >
> > /* Lock already held by send_sigio() and send_sigurg(). */
> > lockdep_assert_held(&fown->lock);
> > - dom = landlock_file(fown->file)->fown_domain;
> > + dom = landlock_get_applicable_domain(
> > + landlock_file(fown->file)->fown_domain, signal_scope);
> >
> > /* Quick return for unowned socket. */
> > if (!dom)
> > --
> > 2.47.0
> >
>
> Reviewed-by: Günther Noack <gnoack@...gle.com>
>
> Looks good!
Thanks!
>
> —Günther
>
Powered by blists - more mailing lists