Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210219222233.20748-1-olga.kornievskaia@gmail.com> <CAHC9VhRKLBNNfUE0FMgGJBR5eBQ+Et=oK1rcErUU_i62AGhfsQ@mail.gmail.com>
In-Reply-To: <CAHC9VhRKLBNNfUE0FMgGJBR5eBQ+Et=oK1rcErUU_i62AGhfsQ@mail.gmail.com>
From:   Olga Kornievskaia <olga.kornievskaia@gmail.com>
Date:   Thu, 25 Feb 2021 13:03:06 -0500
Message-ID: <CAN-5tyGuV-gs0KzVbKSj42ZMx553zy9wOfVb1SoHoE-WCoN1_w@mail.gmail.com>
Subject: Re: [PATCH v3 1/3] [security] Add new hook to compare new mount to an
 existing mount
To:     Paul Moore <paul@paul-moore.com>
Cc:     Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna.schumaker@netapp.com>,
        linux-nfs <linux-nfs@vger.kernel.org>,
        Linux Security Module list 
        <linux-security-module@vger.kernel.org>,
        SElinux list <selinux@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Feb 25, 2021 at 12:53 PM Paul Moore <paul@paul-moore.com> wrote:
>
> 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?

That's is tough to do as it is vague. All I was doing was fixing a
bug. Selinux didn't allow a new mount because it had a different
security context. What that translates to for the new hook, is up to
the LSM module whether it would need the options to be exactly the
same or if they can be slightly different but yet compatible this is
really up to the LSM.

Do you care to suggest wording to use? It is hard to find words that
somebody else is looking for but one is unable to provide them.

>
> > + *     @sb superblock being compared
> > + *     @mnt_opts new mount options
> > + *     Return 0 if options are compatible.
>
> --
> paul moore
> www.paul-moore.com