On Mon, Dec 18, 2023 at 12:11 PM Casey Schaufler casey@schaufler-ca.com wrote:
On 12/18/2023 6:16 AM, Alfred Piccioni wrote:
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 syscal. 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.
Fixes: 0b24dcb7f2f7 ("Revert "selinux: simplify ioctl checking"") Signed-off-by: Alfred Piccioni alpic@google.com Cc: stable@vger.kernel.org
This *really* needs to go the the LSM email list: linux-security-module@vger.kernel.org
Yep, pointed that out a little earlier in this thread.
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index ac962c4cb44b..626aa8cf930d 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -171,6 +171,8 @@ LSM_HOOK(int, 0, file_alloc_security, struct file *file) 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)
Please add a flags parameter to file_ioctl() rather than a new hook.
Paul told him the opposite earlier.