If security_file_ioctl or security_file_ioctl_compat return ENOFILEOPS, the IOCTL logic in fs/ioctl.c will permit the given IOCTL command, but only as long as the IOCTL command is implemented directly in fs/ioctl.c and does not use the f_ops->unhandled_ioctl or f_ops->compat_ioctl operations, which are defined by the given file. The possible return values for security_file_ioctl and security_file_ioctl_compat are now: * 0 - to permit the IOCTL * ENOFILEOPS - to permit the IOCTL, but forbid it if it needs to fall back to the file implementation. * any other error - to forbid the IOCTL and return that error This is an alternative to the previously discussed approaches [1] and [2], and implements the proposal from [3]. Cc: Christian Brauner <brauner@xxxxxxxxxx> Cc: Paul Moore <paul@xxxxxxxxxxxxxx> Cc: Mickaël Salaün <mic@xxxxxxxxxxx> Cc: linux-fsdevel@xxxxxxxxxxxxxxx Link: https://lore.kernel.org/r/20240309075320.160128-2-gnoack@xxxxxxxxxx [1] Link: https://lore.kernel.org/r/20240322151002.3653639-2-gnoack@xxxxxxxxxx/ [2] Link: https://lore.kernel.org/r/32b1164e-9d5f-40c0-9a4e-001b2c9b822f@xxxxxxxxxxxxxxxx/ [3] Suggested-by: Arnd Bergmann <arnd@xxxxxxxx> Signed-off-by: Günther Noack <gnoack@xxxxxxxxxx> --- fs/ioctl.c | 25 ++++++++++++++++++++----- include/linux/security.h | 6 ++++++ security/security.c | 10 ++++++++-- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/fs/ioctl.c b/fs/ioctl.c index 76cf22ac97d7..8244354ad04d 100644 --- a/fs/ioctl.c +++ b/fs/ioctl.c @@ -828,7 +828,7 @@ static int do_vfs_ioctl(struct file *filp, unsigned int fd, case FIONREAD: if (!S_ISREG(inode->i_mode)) - return vfs_ioctl(filp, cmd, arg); + return -ENOIOCTLCMD; return put_user(i_size_read(inode) - filp->f_pos, (int __user *)argp); @@ -858,17 +858,24 @@ SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd, unsigned long, arg) { struct fd f = fdget(fd); int error; + bool use_file_ops = true; if (!f.file) return -EBADF; error = security_file_ioctl(f.file, cmd, arg); - if (error) + if (error == -ENOFILEOPS) + use_file_ops = false; + else if (error) goto out; error = do_vfs_ioctl(f.file, fd, cmd, arg); - if (error == -ENOIOCTLCMD) - error = vfs_ioctl(f.file, cmd, arg); + if (error == -ENOIOCTLCMD) { + if (use_file_ops) + error = vfs_ioctl(f.file, cmd, arg); + else + error = -EACCES; + } out: fdput(f); @@ -916,12 +923,15 @@ COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd, { struct fd f = fdget(fd); int error; + bool use_file_ops = true; if (!f.file) return -EBADF; error = security_file_ioctl_compat(f.file, cmd, arg); - if (error) + if (error == -ENOFILEOPS) + use_file_ops = false; + else if (error) goto out; switch (cmd) { @@ -967,6 +977,11 @@ COMPAT_SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd, if (error != -ENOIOCTLCMD) break; + if (!use_file_ops) { + error = -EACCES; + break; + } + if (f.file->f_op->compat_ioctl) error = f.file->f_op->compat_ioctl(f.file, cmd, arg); if (error == -ENOIOCTLCMD) diff --git a/include/linux/security.h b/include/linux/security.h index d0eb20f90b26..b769dc888d07 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -248,6 +248,12 @@ static const char * const kernel_load_data_str[] = { __kernel_read_file_id(__data_id_stringify) }; +/* + * Returned by security_file_ioctl and security_file_ioctl_compat to indicate + * that the IOCTL request may not be dispatched to the file's f_ops IOCTL impl. + */ +#define ENOFILEOPS 532 + static inline const char *kernel_load_data_id_str(enum kernel_load_data_id id) { if ((unsigned)id >= LOADING_MAX_ID) diff --git a/security/security.c b/security/security.c index 7035ee35a393..000c54a1e541 100644 --- a/security/security.c +++ b/security/security.c @@ -2719,7 +2719,10 @@ void security_file_free(struct file *file) * value. When @arg represents a user space pointer, it should never be used * by the security module. * - * Return: Returns 0 if permission is granted. + * Return: Returns 0 if permission is granted. Returns -ENOFILEOPS if + * permission is granted for IOCTL commands that do not get handled by + * f_ops->unlocked_ioctl(). Returns another negative error code is + * permission is denied. */ int security_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -2736,7 +2739,10 @@ EXPORT_SYMBOL_GPL(security_file_ioctl); * Compat version of security_file_ioctl() that correctly handles 32-bit * processes running on 64-bit kernels. * - * Return: Returns 0 if permission is granted. + * Return: Returns 0 if permission is granted. Returns -ENOFILEOPS if permission + * is granted for IOCTL commands that do not get handled by + * f_ops->compat_ioctl(). Returns another negative error code is + * permission is denied. */ int security_file_ioctl_compat(struct file *file, unsigned int cmd, unsigned long arg) -- 2.44.0.396.g6e790dbe36-goog
