Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
MIME-Version: 1.0
References: <20230928130147.564503-1-mszeredi@redhat.com> <20230928130147.564503-5-mszeredi@redhat.com>
 <CAHC9VhQD9r+Qf5Vz1XmxUdJJJO7HNTKdo8Ux=n+xkxr=JGFMrw@mail.gmail.com>
 <CAJfpegsPbDgaz46x4Rr9ZgCpF9rohVHsvuWtQ5LNAdiYU_D4Ww@mail.gmail.com>
 <a25f2736-1837-f4ca-b401-85db24f46452@themaw.net> <CAJfpegv78njkWdaShTskKXoGOpKAndvYYJwq7CLibiu+xmLCvg@mail.gmail.com>
In-Reply-To: <CAJfpegv78njkWdaShTskKXoGOpKAndvYYJwq7CLibiu+xmLCvg@mail.gmail.com>
From:   Paul Moore <paul@paul-moore.com>
Date:   Thu, 5 Oct 2023 22:56:08 -0400
Message-ID: <CAHC9VhTwnjhfmkT5Rzt+SBf-8hyw4PYkbuPYnm6XLoyY7VAUiw@mail.gmail.com>
Subject: Re: [PATCH v3 4/4] add listmount(2) syscall
To:     Miklos Szeredi <miklos@szeredi.hu>
Cc:     Ian Kent <raven@themaw.net>, Miklos Szeredi <mszeredi@redhat.com>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-api@vger.kernel.org, linux-man@vger.kernel.org,
        linux-security-module@vger.kernel.org, Karel Zak <kzak@redhat.com>,
        David Howells <dhowells@redhat.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Christian Brauner <christian@brauner.io>,
        Amir Goldstein <amir73il@gmail.com>,
        Matthew House <mattlloydhouse@gmail.com>,
        Florian Weimer <fweimer@redhat.com>,
        Arnd Bergmann <arnd@arndb.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Oct 5, 2023 at 11:47=E2=80=AFAM Miklos Szeredi <miklos@szeredi.hu> =
wrote:
> On Thu, 5 Oct 2023 at 06:23, Ian Kent <raven@themaw.net> wrote:
> > The proc interfaces essentially use <mount namespace>->list to provide
> >
> > the mounts that can be seen so it's filtered by mount namespace of the
> >
> > task that's doing the open().
> >
> >
> > See fs/namespace.c:mnt_list_next() and just below the m_start(), m_next=
(),
>
> /proc/$PID/mountinfo will list the mount namespace of $PID.  Whether
> current task has permission to do so is decided at open time.
>
> listmount() will list the children of the given mount ID.  The mount
> ID is looked up in the task's mount namespace, so this cannot be used
> to list mounts of other namespaces.  It's a more limited interface.
>
> I sort of understand the reasoning behind calling into a security hook
> on entry to statmount() and listmount().  And BTW I also think that if
> statmount() and listmount() is limited in this way, then the same
> limitation should be applied to the proc interfaces.  But that needs
> to be done real carefully because it might cause regressions.  OTOH if
> it's only done on the new interfaces, then what is the point, since
> the old interfaces will be available indefinitely?

LSMs that are designed to enforce access controls on procfs interfaces
typically leverage the fact that the procfs interfaces are file based
and the normal file I/O access controls can be used.  In some cases,
e.g. /proc/self/attr, there may also be additional access controls
implemented via a dedicated set of LSM hooks.

> Also I cannot see the point in hiding some mount ID's from the list.
> It seems to me that the list is just an array of numbers that in
> itself doesn't carry any information.

I think it really comes down to the significance of the mount ID, and
I can't say I know enough of the details here to be entirely
comfortable taking a hard stance on this.  Can you help me understand
the mount ID concept a bit better?

While I'm reasonably confident that we want a security_sb_statfs()
control point in statmount(), it may turn out that we don't want/need
a call in the listmount() case.  Perhaps your original patch was
correct in the sense that we only want a single security_sb_statfs()
call for the root (implying that the child mount IDs are attributes of
the root/parent mount)?  Maybe it's something else entirely?

--=20
paul-moore.com