Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230412205316.7114-1-junxiao.bi@oracle.com> <878reqs0xg.fsf@linux.ibm.com>
In-Reply-To: <878reqs0xg.fsf@linux.ibm.com>
From:   Paul Moore <paul@paul-moore.com>
Date:   Mon, 17 Apr 2023 17:56:42 -0400
Message-ID: <CAHC9VhS3LpJ_x7ZfdV83KY3U49XFGMLejz7rsiEH19rzyUfD-w@mail.gmail.com>
Subject: Re: [PATCH V2] debugfs: allow access relay files in lockdown mode
To:     Nathan Lynch <nathanl@linux.ibm.com>
Cc:     Junxiao Bi <junxiao.bi@oracle.com>, linux-kernel@vger.kernel.org,
        linux-security-module@vger.kernel.org, jmorris@namei.org,
        serge@hallyn.com, axboe@kernel.dk, konrad.wilk@oracle.com,
        joe.jin@oracle.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Apr 17, 2023 at 4:39=E2=80=AFPM Nathan Lynch <nathanl@linux.ibm.com=
> wrote:
> Junxiao Bi <junxiao.bi@oracle.com> writes:
> > Relay files are used by kernel to transfer information to userspace, th=
ese
> > files have permission 0400, but mmap is supported, so they are blocked =
by
> > lockdown. But since kernel just generates the contents of those files w=
hile
> > not reading it, it is saft to access relay files in lockdown mode.
> >
> > With this, blktrace can work well in lockdown mode.
>
> Assuming that all relay users do not expose the kinds of information
> that confidentiality mode tries to restrict, this change seems OK to
> me. I think that assumption applies to blktrace; apart from that, there
> is a handful of drivers that use relay files (I searched for
> relay_open() call sites, maybe there is a better way).

At the very least I see an Intel graphics driver and some network
drivers, but like you, that was a quick search and I'm probably
missing something.  At the very least someone needs to go audit those
users/drivers to ensure this is safe to merge.

However, regardless of what that code audit may turn up, I'm a little
concerned that it would be all too easy to add a new relay interface
user which isn't safe.  The check in debugfs_locked_down() is far too
removed from the code which is using the relay interface for it to be
likely noticed in a future case where an unsafe user is added.  This
looks like a vulnerability waiting to happen.

--=20
paul-moore.com