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
