From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2A0847C092; Wed, 21 Feb 2024 13:25:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708521918; cv=none; b=CH85VTJ/Kw/NzGsL7qMyIdkLwuJ9pq4sAQXeTb4bnx47g6oDtcEvoU7bA3wY7sGmevHiWEdIAyZrT9KoeV3RlL+549rAzssAgeUk6bGnVhsmmYggEI3P5MzRR6rAoefV+2KDsF7mknIS2UwJ2MoJQoKb4JHcli+90hS+SBDc57s= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708521918; c=relaxed/simple; bh=nxD2zliihvBhWQ1DEGgPui6kILRlYe3Vj6qkGn2S4d8=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=W3xT/nI3KNVxYwi2HIJ89wAAfpu/LtP+xZGsqxMX7xs7V+YGEpLzKbYrJ1Q4WUSF6Aai+EFcYSXuBKQsxPW5Vc1rFdon60hNIPW/+9Rzs4X8Q5zftPuRkGDzKUVum46wLFK06S5qMZL5oxKMJXkne/Eq9ud9Cc6YeI7LVsvnbs0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b=PIi33jBO; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="PIi33jBO" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9977CC433C7; Wed, 21 Feb 2024 13:25:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1708521918; bh=nxD2zliihvBhWQ1DEGgPui6kILRlYe3Vj6qkGn2S4d8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=PIi33jBOXXJmr2lqvVhDKOxrIsYm+NwPvzbO/yKTr7wB1OPgm9jVSAGFUHBA4RuwW 04Ts9tceRH7p2ST0MnbzHV75IShZnFmC1uNkO5WOFjKZB6AxOckc/n1w4CVf8n43cg efH3XaBMVkW81KtLHWjkyS5dkCLDLVZ0fyYOkyjA= From: Greg Kroah-Hartman To: stable@vger.kernel.org Cc: Greg Kroah-Hartman , patches@lists.linux.dev, Alfred Piccioni , Stephen Smalley , Paul Moore Subject: [PATCH 5.15 032/476] lsm: new security_file_ioctl_compat() hook Date: Wed, 21 Feb 2024 14:01:23 +0100 Message-ID: <20240221130009.133010177@linuxfoundation.org> X-Mailer: git-send-email 2.43.2 In-Reply-To: <20240221130007.738356493@linuxfoundation.org> References: <20240221130007.738356493@linuxfoundation.org> User-Agent: quilt/0.67 X-stable: review X-Patchwork-Hint: ignore Precedence: bulk X-Mailing-List: patches@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 5.15-stable review patch. If anyone has any objections, please let me know. ------------------ From: Alfred Piccioni commit f1bb47a31dff6d4b34fb14e99850860ee74bb003 upstream. Some ioctl commands do not require ioctl permission, but are routed to other permissions such as FILE_GETATTR or FILE_SETATTR. This routing is done by comparing the ioctl cmd to a set of 64-bit flags (FS_IOC_*). However, if a 32-bit process is running on a 64-bit kernel, it emits 32-bit flags (FS_IOC32_*) for certain ioctl operations. These flags are being checked erroneously, which leads to these ioctl operations being routed to the ioctl permission, rather than the correct file permissions. This was also noted in a RED-PEN finding from a while back - "/* RED-PEN how should LSM module know it's handling 32bit? */". This patch introduces a new hook, security_file_ioctl_compat(), that is called from the compat ioctl syscall. All current LSMs have been changed to support this hook. Reviewing the three places where we are currently using security_file_ioctl(), it appears that only SELinux needs a dedicated compat change; TOMOYO and SMACK appear to be functional without any change. Cc: stable@vger.kernel.org Fixes: 0b24dcb7f2f7 ("Revert "selinux: simplify ioctl checking"") Signed-off-by: Alfred Piccioni Reviewed-by: Stephen Smalley [PM: subject tweak, line length fixes, and alignment corrections] Signed-off-by: Paul Moore Signed-off-by: Greg Kroah-Hartman --- fs/ioctl.c | 3 +-- include/linux/lsm_hook_defs.h | 2 ++ include/linux/security.h | 9 +++++++++ security/security.c | 18 ++++++++++++++++++ security/selinux/hooks.c | 28 ++++++++++++++++++++++++++++ security/smack/smack_lsm.c | 1 + security/tomoyo/tomoyo.c | 1 + 7 files changed, 60 insertions(+), 2 deletions(-) --- a/fs/ioctl.c +++ b/fs/ioctl.c @@ -920,8 +920,7 @@ COMPAT_SYSCALL_DEFINE3(ioctl, unsigned i if (!f.file) return -EBADF; - /* RED-PEN how should LSM module know it's handling 32bit? */ - error = security_file_ioctl(f.file, cmd, arg); + error = security_file_ioctl_compat(f.file, cmd, arg); if (error) goto out; --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -165,6 +165,8 @@ LSM_HOOK(int, 0, file_alloc_security, st LSM_HOOK(void, LSM_RET_VOID, file_free_security, struct file *file) LSM_HOOK(int, 0, file_ioctl, struct file *file, unsigned int cmd, unsigned long arg) +LSM_HOOK(int, 0, file_ioctl_compat, struct file *file, unsigned int cmd, + unsigned long arg) LSM_HOOK(int, 0, mmap_addr, unsigned long addr) LSM_HOOK(int, 0, mmap_file, struct file *file, unsigned long reqprot, unsigned long prot, unsigned long flags) --- a/include/linux/security.h +++ b/include/linux/security.h @@ -382,6 +382,8 @@ int security_file_permission(struct file int security_file_alloc(struct file *file); void security_file_free(struct file *file); int security_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg); +int security_file_ioctl_compat(struct file *file, unsigned int cmd, + unsigned long arg); int security_mmap_file(struct file *file, unsigned long prot, unsigned long flags); int security_mmap_addr(unsigned long addr); @@ -962,6 +964,13 @@ static inline int security_file_ioctl(st { return 0; } + +static inline int security_file_ioctl_compat(struct file *file, + unsigned int cmd, + unsigned long arg) +{ + return 0; +} static inline int security_mmap_file(struct file *file, unsigned long prot, unsigned long flags) --- a/security/security.c +++ b/security/security.c @@ -1556,6 +1556,24 @@ int security_file_ioctl(struct file *fil } EXPORT_SYMBOL_GPL(security_file_ioctl); +/** + * security_file_ioctl_compat() - Check if an ioctl is allowed in compat mode + * @file: associated file + * @cmd: ioctl cmd + * @arg: ioctl arguments + * + * Compat version of security_file_ioctl() that correctly handles 32-bit + * processes running on 64-bit kernels. + * + * Return: Returns 0 if permission is granted. + */ +int security_file_ioctl_compat(struct file *file, unsigned int cmd, + unsigned long arg) +{ + return call_int_hook(file_ioctl_compat, 0, file, cmd, arg); +} +EXPORT_SYMBOL_GPL(security_file_ioctl_compat); + static inline unsigned long mmap_prot(struct file *file, unsigned long prot) { /* --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3810,6 +3810,33 @@ static int selinux_file_ioctl(struct fil return error; } +static int selinux_file_ioctl_compat(struct file *file, unsigned int cmd, + unsigned long arg) +{ + /* + * If we are in a 64-bit kernel running 32-bit userspace, we need to + * make sure we don't compare 32-bit flags to 64-bit flags. + */ + switch (cmd) { + case FS_IOC32_GETFLAGS: + cmd = FS_IOC_GETFLAGS; + break; + case FS_IOC32_SETFLAGS: + cmd = FS_IOC_SETFLAGS; + break; + case FS_IOC32_GETVERSION: + cmd = FS_IOC_GETVERSION; + break; + case FS_IOC32_SETVERSION: + cmd = FS_IOC_SETVERSION; + break; + default: + break; + } + + return selinux_file_ioctl(file, cmd, arg); +} + static int default_noexec __ro_after_init; static int file_map_prot_check(struct file *file, unsigned long prot, int shared) @@ -7208,6 +7235,7 @@ static struct security_hook_list selinux LSM_HOOK_INIT(file_permission, selinux_file_permission), LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security), LSM_HOOK_INIT(file_ioctl, selinux_file_ioctl), + LSM_HOOK_INIT(file_ioctl_compat, selinux_file_ioctl_compat), LSM_HOOK_INIT(mmap_file, selinux_mmap_file), LSM_HOOK_INIT(mmap_addr, selinux_mmap_addr), LSM_HOOK_INIT(file_mprotect, selinux_file_mprotect), --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4767,6 +4767,7 @@ static struct security_hook_list smack_h LSM_HOOK_INIT(file_alloc_security, smack_file_alloc_security), LSM_HOOK_INIT(file_ioctl, smack_file_ioctl), + LSM_HOOK_INIT(file_ioctl_compat, smack_file_ioctl), LSM_HOOK_INIT(file_lock, smack_file_lock), LSM_HOOK_INIT(file_fcntl, smack_file_fcntl), LSM_HOOK_INIT(mmap_file, smack_mmap_file), --- a/security/tomoyo/tomoyo.c +++ b/security/tomoyo/tomoyo.c @@ -546,6 +546,7 @@ static struct security_hook_list tomoyo_ LSM_HOOK_INIT(path_rename, tomoyo_path_rename), LSM_HOOK_INIT(inode_getattr, tomoyo_inode_getattr), LSM_HOOK_INIT(file_ioctl, tomoyo_file_ioctl), + LSM_HOOK_INIT(file_ioctl_compat, tomoyo_file_ioctl), LSM_HOOK_INIT(path_chmod, tomoyo_path_chmod), LSM_HOOK_INIT(path_chown, tomoyo_path_chown), LSM_HOOK_INIT(path_chroot, tomoyo_path_chroot),