Re: [PATCH v3 1/3] [security] Add new hook to compare new mount to an existing mount

[Paul Moore <paul@paul-moore.com>]

On Fri, Feb 19, 2021 at 5:25 PM Olga Kornievskaia
<olga.kornievskaia@gmail.com> wrote:
>
> From: Olga Kornievskaia <kolga@netapp.com>
>
> Add a new hook that takes an existing super block and a new mount
> with new options and determines if new options confict with an
> existing mount or not.
>
> A filesystem can use this new hook to determine if it can share
> the an existing superblock with a new superblock for the new mount.
>
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  include/linux/lsm_hook_defs.h |  1 +
>  include/linux/lsm_hooks.h     |  6 ++++
>  include/linux/security.h      |  8 +++++
>  security/security.c           |  7 +++++
>  security/selinux/hooks.c      | 56 +++++++++++++++++++++++++++++++++++
>  5 files changed, 78 insertions(+)

...

> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index a19adef1f088..d76aaecfdf0f 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -142,6 +142,12 @@
>   *     @orig the original mount data copied from userspace.
>   *     @copy copied data which will be passed to the security module.
>   *     Returns 0 if the copy was successful.
> + * @sb_mnt_opts_compat:
> + *     Determine if the existing mount options are compatible with the new
> + *     mount options being used.

Full disclosure: I'm a big fan of good documentation, regardless of if
it lives in comments or a separate dedicated resource.  Looking at the
comment above, and the SELinux implementation of this hook below, it
appears that the comment is a bit vague; specifically the use of
"compatible".  Based on the SELinux implementation, "compatible" would
seem to equal, do you envision that to be the case for every
LSM/security-model?  If the answer is yes, then let's say that (and
possibly rename the hook to "sb_mnt_opts_equal").  If the answer is
no, then I think we need to do a better job explaining what
compatibility really means; put yourself in the shoes of someone
writing a LSM, what would they need to know to write an implementation
for this hook?

> + *     @sb superblock being compared
> + *     @mnt_opts new mount options
> + *     Return 0 if options are compatible.

-- 
paul moore
www.paul-moore.com