Skip to contentSkip to conversation

audit: treat s_id as an untrusted string

PATCH In review Kees Cook 2 participants 2 messages Jan 7, 2012 – Jan 8, 2012

Subsystems (3)
Security (LSM) Integrity measurement architecture (IMA) Extended Verification Module (EVM)
lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH 1/1 audit: treat s_id as an untrusted string

The use of s_id should go through the untrusted string path, just to be extra careful.

Signed-off-by Kees Cook <[email protected]>

---
applies on top of http://lkml.org/lkml/2012/1/6/308

---
 security/integrity/ima/ima_audit.c |    8 +++++---
 security/lsm_audit.c               |   23 +++++++++++++----------
 2 files changed, 18 insertions(+), 13 deletions(-)
security/integrity/ima/ima_audit.c +5 -3
index c5c5a72..2ad942f 100644
@@ -56,9 +56,11 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,
 		audit_log_format(ab, " name=");
 		audit_log_untrustedstring(ab, fname);
 	}
-	if (inode)
-		audit_log_format(ab, " dev=%s ino=%lu",
-				 inode->i_sb->s_id, inode->i_ino);
+	if (inode) {
+		audit_log_format(ab, " dev=");
+		audit_log_untrustedstring(ab, inode->i_sb->s_id);
+		audit_log_format(ab, " ino=%lu", inode->i_ino);
+	}
 	audit_log_format(ab, " res=%d", !result ? 0 : 1);
 	audit_log_end(ab);
 }
security/lsm_audit.c +13 -10
index 5f3b532..293b8c4 100644
@@ -235,10 +235,11 @@ static void dump_common_audit_data(struct audit_buffer *ab,
 		audit_log_d_path(ab, " path=", &a->u.path);
 
 		inode = a->u.path.dentry->d_inode;
-		if (inode)
-			audit_log_format(ab, " dev=%s ino=%lu",
-					inode->i_sb->s_id,
-					inode->i_ino);
+		if (inode) {
+			audit_log_format(ab, " dev=");
+			audit_log_untrustedstring(ab, inode->i_sb->s_id);
+			audit_log_format(ab, " ino=%lu", inode->i_ino);
+		}
 		break;
 	}
 	case LSM_AUDIT_DATA_DENTRY: {

@@ -248,10 +249,11 @@ static void dump_common_audit_data(struct audit_buffer *ab,
 		audit_log_untrustedstring(ab, a->u.dentry->d_name.name);
 
 		inode = a->u.dentry->d_inode;
-		if (inode)
-			audit_log_format(ab, " dev=%s ino=%lu",
-					inode->i_sb->s_id,
-					inode->i_ino);
+		if (inode) {
+			audit_log_format(ab, " dev=");
+			audit_log_untrustedstring(ab, inode->i_sb->s_id);
+			audit_log_format(ab, " ino=%lu", inode->i_ino);
+		}
 		break;
 	}
 	case LSM_AUDIT_DATA_INODE: {

@@ -266,8 +268,9 @@ static void dump_common_audit_data(struct audit_buffer *ab,
 					 dentry->d_name.name);
 			dput(dentry);
 		}
-		audit_log_format(ab, " dev=%s ino=%lu", inode->i_sb->s_id,
-				 inode->i_ino);
+		audit_log_format(ab, " dev=");
+		audit_log_untrustedstring(ab, inode->i_sb->s_id);
+		audit_log_format(ab, " ino=%lu", inode->i_ino);
 		break;
 	}
 	case LSM_AUDIT_DATA_TASK:
Signature · 6 lines
1.7.4.1


-- 
Kees Cook
ChromeOS Security
Acked-by on v1 1/1
#1 lore raw
Quoted 4 lines · Kees Cook

The use of s_id should go through the untrusted string path, just to be extra careful.

Signed-off-by Kees Cook <[email protected]>
Acked-by Mimi Zohar <[email protected]>

thanks,
Mimi

Quoted 78 lines

---
applies on top of http://lkml.org/lkml/2012/1/6/308

---
 security/integrity/ima/ima_audit.c |    8 +++++---
 security/lsm_audit.c               |   23 +++++++++++++----------
 2 files changed, 18 insertions(+), 13 deletions(-)
security/integrity/ima/ima_audit.c +5 -3
index c5c5a72..2ad942f 100644
@@ -56,9 +56,11 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,
 		audit_log_format(ab, " name=");
 		audit_log_untrustedstring(ab, fname);
 	}
-	if (inode)
-		audit_log_format(ab, " dev=%s ino=%lu",
-				 inode->i_sb->s_id, inode->i_ino);
+	if (inode) {
+		audit_log_format(ab, " dev=");
+		audit_log_untrustedstring(ab, inode->i_sb->s_id);
+		audit_log_format(ab, " ino=%lu", inode->i_ino);
+	}
 	audit_log_format(ab, " res=%d", !result ? 0 : 1);
 	audit_log_end(ab);
 }
security/lsm_audit.c +13 -10
index 5f3b532..293b8c4 100644
@@ -235,10 +235,11 @@ static void dump_common_audit_data(struct audit_buffer *ab,
 		audit_log_d_path(ab, " path=", &a->u.path);
 
 		inode = a->u.path.dentry->d_inode;
-		if (inode)
-			audit_log_format(ab, " dev=%s ino=%lu",
-					inode->i_sb->s_id,
-					inode->i_ino);
+		if (inode) {
+			audit_log_format(ab, " dev=");
+			audit_log_untrustedstring(ab, inode->i_sb->s_id);
+			audit_log_format(ab, " ino=%lu", inode->i_ino);
+		}
 		break;
 	}
 	case LSM_AUDIT_DATA_DENTRY: {

@@ -248,10 +249,11 @@ static void dump_common_audit_data(struct audit_buffer *ab,
 		audit_log_untrustedstring(ab, a->u.dentry->d_name.name);
 
 		inode = a->u.dentry->d_inode;
-		if (inode)
-			audit_log_format(ab, " dev=%s ino=%lu",
-					inode->i_sb->s_id,
-					inode->i_ino);
+		if (inode) {
+			audit_log_format(ab, " dev=");
+			audit_log_untrustedstring(ab, inode->i_sb->s_id);
+			audit_log_format(ab, " ino=%lu", inode->i_ino);
+		}
 		break;
 	}
 	case LSM_AUDIT_DATA_INODE: {

@@ -266,8 +268,9 @@ static void dump_common_audit_data(struct audit_buffer *ab,
 					 dentry->d_name.name);
 			dput(dentry);
 		}
-		audit_log_format(ab, " dev=%s ino=%lu", inode->i_sb->s_id,
-				 inode->i_ino);
+		audit_log_format(ab, " dev=");
+		audit_log_untrustedstring(ab, inode->i_sb->s_id);
+		audit_log_format(ab, " ino=%lu", inode->i_ino);
 		break;
 	}
 	case LSM_AUDIT_DATA_TASK:
Signature · 1 line
1.7.4.1