| Message ID | 20241109110856.222842-4-mic@digikod.net (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Refactor Landlock access mask management | expand |
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@google.com> > Cc: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com> > Cc: Tahera Fahimi <fahimitahera@gmail.com> > Signed-off-by: Mickaël Salaün <mic@digikod.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.) > + > 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@google.com> Looks good! —Günther
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@google.com> > > Cc: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com> > > Cc: Tahera Fahimi <fahimitahera@gmail.com> > > Signed-off-by: Mickaël Salaün <mic@digikod.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@google.com> > > Looks good! Thanks! > > —Günther >
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, +}; + 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)
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@google.com> Cc: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com> Cc: Tahera Fahimi <fahimitahera@gmail.com> Signed-off-by: Mickaël Salaün <mic@digikod.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(-)