[PATCH v9 1/8] landlock: Add IOCTL access right

[Günther Noack]

Hello Arnd!

On Fri, Feb 09, 2024 at 06:06:05PM +0100, Günther Noack wrote:
> Introduces the LANDLOCK_ACCESS_FS_IOCTL access right
> and increments the Landlock ABI version to 5.
>
> [...]
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 73997e63734f..84efea3f7c0f 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> +/**
> + * get_required_ioctl_access(): Determine required IOCTL access rights.
> + *
> + * @cmd: The IOCTL command that is supposed to be run.
> + *
> + * Any new IOCTL commands that are implemented in fs/ioctl.c's do_vfs_ioctl()
> + * should be considered for inclusion here.
> + *
> + * Returns: The access rights that must be granted on an opened file in order to
> + * use the given @cmd.
> + */
> +static __attribute_const__ access_mask_t
> +get_required_ioctl_access(const unsigned int cmd)
> +{
> +	switch (cmd) {

  [...]

> +	case FIONREAD:

Hello Arnd!  Christian Brauner suggested at FOSDEM that you would be a
good person to reach out to regarding this question -- we would
appreciate if you could have a short look! :)

Context: This patch set lets processes restrict the use of IOCTLs with
the Landlock LSM.  To make the use of this feature more practical, we
are relaxing the rules for some common and harmless IOCTL commands,
which are directly implemented in fs/ioctl.c.

The IOCTL command in question is FIONREAD: fs/ioctl.c implements
FIONREAD directly for S_ISREG files, but it does call the FIONREAD
implementation in the VFS layer for other file types.

The question we are asking ourselves is:

* Can we let processes safely use FIONREAD for all files which get
  opened for the purpose of reading, or do we run the risk of
  accidentally exposing surprising IOCTL implementations which have
  completely different purposes?

  Is it safe to assume that the VFS layer FIONREAD implementations are
  actually implementing FIONREAD semantics?

* I know there have been accidental collisions of IOCTL command
  numbers in the past -- Hypothetically, if this were to happen in one
  of the VFS implementations of FIONREAD, would that be considered a
  bug that would need to get fixed in that implementation?


> +	case FIOQSIZE:
> +	case FIGETBSZ:
> +		/*
> +		 * FIONREAD returns the number of bytes available for reading.
> +		 * FIONREAD returns the number of immediately readable bytes for
> +		 * a file.

(Oops, repetitive doc, probably mis-merged.  Fixing it in next version.)


Thanks!
—Günther

-- 
Sent using Mutt 🐕 Woof Woof