Linux kernel -stable discussions
 help / color / mirror / Atom feed
* [PATCH v1 1/2] lsm: Preserve full ioctl commands in audit records
       [not found] <20260910144837.381855-1-mic@digikod.net>
@ 2026-09-10 14:48 ` Mickaël Salaün
  2026-09-14 20:41   ` Paul Moore
  0 siblings, 1 reply; 2+ messages in thread
From: Mickaël Salaün @ 2026-09-10 14:48 UTC (permalink / raw)
  To: Paul Moore
  Cc: Mickaël Salaün, Günther Noack, Jeff Vander Stoep,
	Nick Kralevich, Ondrej Mosnacek, Serge E . Hallyn,
	Stephen Smalley, audit, kernel-team, linux-kernel,
	linux-security-module, selinux, stable

Ioctl commands are unsigned int values, with their encoded direction and
size in the upper 16 bits.  However, struct lsm_ioctlop_audit keeps only
a u16, and the common audit formatter uses %hx.  This logs 0xc00ffeee as
0xfeee and makes distinct commands with matching low 16 bits
indistinguishable.

Change cmd to unsigned int and use %x.  Pass the full command through
ioctl_has_perm() as well.  Its driver and xperm values remain u8 values
derived from the same low 16 bits, so SELinux enforcement does not
change.  Commands that fit in 16 bits keep the same audit text.

Cc: Jeff Vander Stoep <jeffv@google.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: stable@vger.kernel.org
Fixes: 671a2781ff01 ("security: add ioctl specific auditing to lsm_audit")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 include/linux/lsm_audit.h | 2 +-
 security/lsm_audit.c      | 2 +-
 security/selinux/hooks.c  | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
index 584db296e43b..fc68ca94f05a 100644
--- a/include/linux/lsm_audit.h
+++ b/include/linux/lsm_audit.h
@@ -44,7 +44,7 @@ struct lsm_network_audit {
 
 struct lsm_ioctlop_audit {
 	struct path path;
-	u16 cmd;
+	unsigned int cmd;
 };
 
 struct lsm_ibpkey_audit {
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index 737f5a263a8f..a7891a75d0aa 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -231,7 +231,7 @@ void audit_log_lsm_data(struct audit_buffer *ab,
 			audit_log_format(ab, " ino=%llu", inode->i_ino);
 		}
 
-		audit_log_format(ab, " ioctlcmd=0x%hx", a->u.op->cmd);
+		audit_log_format(ab, " ioctlcmd=0x%x", a->u.op->cmd);
 		break;
 	}
 	case LSM_AUDIT_DATA_DENTRY: {
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 035aaf113d1d..e3eaa3141d91 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3859,7 +3859,7 @@ static int selinux_backing_file_alloc(struct file *backing_file,
  * operation to an inode.
  */
 static int ioctl_has_perm(const struct cred *cred, struct file *file,
-		u32 requested, u16 cmd)
+		u32 requested, unsigned int cmd)
 {
 	struct common_audit_data ad;
 	struct file_security_struct *fsec = selinux_file(file);
@@ -3930,14 +3930,14 @@ static int selinux_file_ioctl(struct file *file, unsigned int cmd,
 	case FIOCLEX:
 	case FIONCLEX:
 		if (!selinux_policycap_ioctl_skip_cloexec())
-			error = ioctl_has_perm(cred, file, FILE__IOCTL, (u16) cmd);
+			error = ioctl_has_perm(cred, file, FILE__IOCTL, cmd);
 		break;
 
 	/* default case assumes that the command will go
 	 * to the file's ioctl() function.
 	 */
 	default:
-		error = ioctl_has_perm(cred, file, FILE__IOCTL, (u16) cmd);
+		error = ioctl_has_perm(cred, file, FILE__IOCTL, cmd);
 	}
 	return error;
 }
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v1 1/2] lsm: Preserve full ioctl commands in audit records
  2026-09-10 14:48 ` [PATCH v1 1/2] lsm: Preserve full ioctl commands in audit records Mickaël Salaün
@ 2026-09-14 20:41   ` Paul Moore
  0 siblings, 0 replies; 2+ messages in thread
From: Paul Moore @ 2026-09-14 20:41 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Mickaël Salaün, Günther Noack, Jeff Vander Stoep,
	Nick Kralevich, Ondrej Mosnacek, Serge E . Hallyn,
	Stephen Smalley, audit, kernel-team, linux-kernel,
	linux-security-module, selinux, stable

On Sep 10, 2026 =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic@digikod.net> wrote:
> 
> Ioctl commands are unsigned int values, with their encoded direction and
> size in the upper 16 bits.  However, struct lsm_ioctlop_audit keeps only
> a u16, and the common audit formatter uses %hx.  This logs 0xc00ffeee as
> 0xfeee and makes distinct commands with matching low 16 bits
> indistinguishable.
> 
> Change cmd to unsigned int and use %x.  Pass the full command through
> ioctl_has_perm() as well.  Its driver and xperm values remain u8 values
> derived from the same low 16 bits, so SELinux enforcement does not
> change.  Commands that fit in 16 bits keep the same audit text.
> 
> Cc: Jeff Vander Stoep <jeffv@google.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: stable@vger.kernel.org
> Fixes: 671a2781ff01 ("security: add ioctl specific auditing to lsm_audit")
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  include/linux/lsm_audit.h | 2 +-
>  security/lsm_audit.c      | 2 +-
>  security/selinux/hooks.c  | 6 +++---
>  3 files changed, 5 insertions(+), 5 deletions(-)

This looks good, thanks Mickaël.

However, I am going to drop the stable tag and merge this via lsm/dev
instead of lsm/stable-7.3 simply because we're changing the audit field
(going from a 16-bit to 32-bit hex string) and I don't want that to catch
anyone by surprise in a stable kernel release.  I will leave the 'Fixes:'
tag for reference in case there is a downstream that wants to pick this
up as a fix.

--
paul-moore.com

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-14 20:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260910144837.381855-1-mic@digikod.net>
2026-09-10 14:48 ` [PATCH v1 1/2] lsm: Preserve full ioctl commands in audit records Mickaël Salaün
2026-09-14 20:41   ` Paul Moore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox