[RFC PATCH] fs: Add vfs_masks_device_ioctl*() helpers

[Mickaël Salaün]

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 at 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.
> 
> This won't just protect from calling into drivers that are lacking
> a CAP_SYS_ADMIN check, but also from those that end up being
> harmful regardless of the ioctl command code passed into them
> because of stupid driver bugs.

Indeed.

"safe" is definitely not the right word, it is too broad, relative to
use cases and threat models.  There is no "safe" IOCTL.

Let's replace "safe IOCTL" with "IOCTL always allowed in a Landlock
sandbox".

Our assumptions are (in the context of Landlock):

1. There are IOCTLs tied to file types (e.g. block device with
   major/minor) that can easily be identified from user space (e.g. with
   the path name and file's metadata).  /dev/* files make sense for user
   space and they scope to a specific use case (with relative
   privileges).  This category of IOCTLs is implemented in standalone
   device drivers (for most of them).

2. Most user space processes should not be denied access to IOCTLs that
   are managed by the VFS layer or the underlying filesystem
   implementations.  For instance, the do_vfs_ioctl()'s ones (e.g.
   FIOCLEX, FIONREAD) should always be allowed because they may be
   required to legitimately use files, and for performance and security
   reasons (e.g. fs-crypt, fsverity implemented at the filesystem layer).
   Moreover, these IOCTLs should already check the read/write permission
   (on the related FD), which is not the case for most block/char device
   IOCTL.

3. IOCTLs to pipes and sockets are out of scope.  They should always be
   allowed for now because they don't directly expose files' data but
   IPCs instead, and we are focusing on FS access rights for now.

We want to add a new LANDLOCK_ACCESS_FS_IOCTL_DEV right that could match
on char/block device's specific IOCTLs, but it would not have any impact
on other IOCTLs which would then always be allowed (if the sandboxed
process is allowed to open the file).

Because IOCTLs are implemented in layers and all IOCTLs commands live in
the same 32-bit namespace, we need a way to identify the layer
implemented by block and character devices.  The new LSM hook proposal
enables us to cleanly and efficiently identify the char/block device
IOCTL layer with an additional check on the file type.