From: Dave Chinner <david@fromorbit.com>
To: "Günther Noack" <gnoack@google.com>
Cc: "Arnd Bergmann" <arnd@arndb.de>,
"Paul Moore" <paul@paul-moore.com>,
"Mickaël Salaün" <mic@digikod.net>,
"Christian Brauner" <brauner@kernel.org>,
"Allen Webb" <allenwebb@google.com>,
"Dmitry Torokhov" <dtor@google.com>,
"Jeff Xu" <jeffxu@google.com>,
"Jorge Lucangeli Obes" <jorgelo@chromium.org>,
"Konstantin Meskhidze" <konstantin.meskhidze@huawei.com>,
"Matt Bobrowski" <repnop@google.com>,
linux-fsdevel@vger.kernel.org,
linux-security-module@vger.kernel.org
Subject: Re: [RFC PATCH] fs: Add vfs_masks_device_ioctl*() helpers
Date: Mon, 11 Mar 2024 12:03:13 +1100 [thread overview]
Message-ID: <Ze5YUUUQqaZsPjql@dread.disaster.area> (raw)
In-Reply-To: <ZervrVoHfZzAYZy4@google.com>
On Fri, Mar 08, 2024 at 12:03:01PM +0100, Günther Noack wrote:
> On Fri, Mar 08, 2024 at 08:02:13AM +0100, Arnd Bergmann wrote:
> > On Fri, Mar 8, 2024, at 00:09, Dave Chinner wrote:
> > > On Thu, Mar 07, 2024 at 03:40:44PM -0500, Paul Moore wrote:
> > >> On Thu, Mar 7, 2024 at 7:57 AM Günther Noack <gnoack@google.com> wrote:
> > >> I need some more convincing as to why we need to introduce these new
> > >> hooks, or even the vfs_masked_device_ioctl() classifier as originally
> > >> proposed at the top of this thread. I believe I understand why
> > >> Landlock wants this, but I worry that we all might have different
> > >> definitions of a "safe" ioctl list, and encoding a definition into the
> > >> LSM hooks seems like a bad idea to me.
> > >
> > > I have no idea what a "safe" ioctl means here. Subsystems already
> > > restrict ioctls that can do damage if misused to CAP_SYS_ADMIN, so
> > > "safe" clearly means something different here.
> >
> > That was my problem with the first version as well, but I think
> > drawing the line between "implemented in fs/ioctl.c" and
> > "implemented in a random device driver fops->unlock_ioctl()"
> > seems like a more helpful definition.
>
> Yes, sorry for the confusion - that is exactly what I meant to say with "safe".:
>
> Those are the IOCTL commands implemented in fs/ioctl.c which do not go through
> f_ops->unlocked_ioctl (or the compat equivalent).
Which means all the ioctls we wrequire for to manage filesystems are
going to be considered "unsafe" and barred, yes?
That means you'll break basic commands like 'xfs_info' that tell you
the configuration of the filesystem. It will prevent things like
online growing and shrinking, online defrag, fstrim, online
scrubbing and repair, etc will not worki anymore. It will break
backup utilities like xfsdump, and break -all- the device management
of btrfs and bcachefs filesystems.
Further, all the setup and management of -VFS functionality- like
fsverity and fscrypt is actually done at the filesystem level (i.e
through ->unlocked_ioctl, no do_vfs_ioctl()) so those are all going
to get broken as well despite them being "vfs features".
Hence from a filesystem perspective, this is a fundamentally
unworkable definition of "safe".
> We want to give people a way with Landlock so that they can restrict the use of
> device-driver implemented IOCTLs, but where they can keep using the bulk of
> more harmless IOCTLs in fs/ioctl.c.
Hah! There's plenty of "harm" that can be done through those ioctls.
It's the entry point for things like filesystem freeze/thaw, FIEMAP
(returns physical data location information), file cloning,
deduplication and per-inode feature manipulation. Lots of this stuff
is under CAP_SYS_ADMIN because they aren't safe for to be exposed to
general users...
So, yeah, I don't think this definition of "safe" is actually useful
in any way. It's arbitrary, and will require both widespread
whitelisting of ioctls to maintain a useful working system and
widespread blacklisting to create a secure system....
-Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2024-03-11 1:03 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-09 17:06 [PATCH v9 0/8] Landlock: IOCTL support Günther Noack
2024-02-09 17:06 ` [PATCH v9 1/8] landlock: Add IOCTL access right Günther Noack
2024-02-10 11:06 ` Günther Noack
2024-02-10 11:49 ` Arnd Bergmann
2024-02-12 11:09 ` Christian Brauner
2024-02-12 22:10 ` Günther Noack
2024-02-10 11:18 ` Günther Noack
2024-02-16 14:11 ` Mickaël Salaün
2024-02-16 15:51 ` Mickaël Salaün
2024-02-18 8:34 ` Günther Noack
2024-02-19 21:44 ` Günther Noack
2024-02-16 17:19 ` Mickaël Salaün
2024-02-19 18:34 ` Mickaël Salaün
2024-02-19 18:35 ` [RFC PATCH] fs: Add vfs_masks_device_ioctl*() helpers Mickaël Salaün
2024-03-01 13:42 ` Mickaël Salaün
2024-03-01 16:24 ` Arnd Bergmann
2024-03-01 18:35 ` Mickaël Salaün
2024-03-05 18:13 ` Günther Noack
2024-03-06 13:47 ` Mickaël Salaün
2024-03-06 15:18 ` Arnd Bergmann
2024-03-07 12:15 ` Christian Brauner
2024-03-07 12:21 ` Arnd Bergmann
2024-03-07 12:57 ` Günther Noack
2024-03-07 20:40 ` Paul Moore
2024-03-07 23:09 ` Dave Chinner
2024-03-07 23:35 ` Paul Moore
2024-03-08 7:02 ` Arnd Bergmann
2024-03-08 9:29 ` Mickaël Salaün
2024-03-08 19:22 ` Paul Moore
2024-03-08 20:12 ` Mickaël Salaün
2024-03-08 22:04 ` Casey Schaufler
2024-03-08 22:25 ` Paul Moore
2024-03-09 8:14 ` Günther Noack
2024-03-09 17:41 ` Casey Schaufler
2024-03-11 19:04 ` Paul Moore
2024-03-08 11:03 ` Günther Noack
2024-03-11 1:03 ` Dave Chinner [this message]
2024-03-11 9:01 ` Günther Noack
2024-03-11 22:12 ` Dave Chinner
2024-03-12 10:58 ` Mickaël Salaün
2024-02-28 12:57 ` [PATCH v9 1/8] landlock: Add IOCTL access right Günther Noack
2024-03-01 12:59 ` Mickaël Salaün
2024-03-01 13:38 ` Mickaël Salaün
2024-02-09 17:06 ` [PATCH v9 2/8] selftests/landlock: Test IOCTL support Günther Noack
2024-02-09 17:06 ` [PATCH v9 3/8] selftests/landlock: Test IOCTL with memfds Günther Noack
2024-02-09 17:06 ` [PATCH v9 4/8] selftests/landlock: Test ioctl(2) and ftruncate(2) with open(O_PATH) Günther Noack
2024-02-09 17:06 ` [PATCH v9 5/8] selftests/landlock: Test IOCTLs on named pipes Günther Noack
2024-02-09 17:06 ` [PATCH v9 6/8] selftests/landlock: Check IOCTL restrictions for named UNIX domain sockets Günther Noack
2024-02-09 17:06 ` [PATCH v9 7/8] samples/landlock: Add support for LANDLOCK_ACCESS_FS_IOCTL Günther Noack
2024-02-09 17:06 ` [PATCH v9 8/8] landlock: Document IOCTL support Günther Noack
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Ze5YUUUQqaZsPjql@dread.disaster.area \
--to=david@fromorbit.com \
--cc=allenwebb@google.com \
--cc=arnd@arndb.de \
--cc=brauner@kernel.org \
--cc=dtor@google.com \
--cc=gnoack@google.com \
--cc=jeffxu@google.com \
--cc=jorgelo@chromium.org \
--cc=konstantin.meskhidze@huawei.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mic@digikod.net \
--cc=paul@paul-moore.com \
--cc=repnop@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).