linux-kernel - [PATCH] Fix common_audit_data type for smack_inode_unlink() and smack_inode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 11 Mar 2013 15:50:36 +0400
From:	Igor Zhbanov <i.zhbanov@...sung.com>
To:	linux-kernel@...r.kernel.org
Cc:	Kyungmin Park <kyungmin.park@...sung.com>,
	Igor Zhbanov <i.zhbanov@...sung.com>
Subject: [PATCH] Fix common_audit_data type for smack_inode_unlink() and
 smack_inode_rmdir()

This patch fixes kernel Oops because of wrong common_audit_data type
in smack_inode_unlink() and smack_inode_rmdir().

When SMACK security module is enabled and SMACK logging is on (/smack/logging
is not zero) and you try to delete the file which
1) you cannot delete due to SMACK rules and logging of failures is on
or
2) you can delete and logging of success is on,

you will see following:

	Unable to handle kernel NULL pointer dereference at virtual address 000002d7

	[<...>] (strlen+0x0/0x28)
	[<...>] (audit_log_untrustedstring+0x14/0x28)
	[<...>] (common_lsm_audit+0x108/0x6ac)
	[<...>] (smack_log+0xc4/0xe4)
	[<...>] (smk_curacc+0x80/0x10c)
	[<...>] (smack_inode_unlink+0x74/0x80)
	[<...>] (security_inode_unlink+0x2c/0x30)
	[<...>] (vfs_unlink+0x7c/0x100)
	[<...>] (do_unlinkat+0x144/0x16c)

The function smack_inode_unlink() (and smack_inode_rmdir()) need
to log two structures of different types. First of all it does:

	smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_DENTRY);
	smk_ad_setfield_u_fs_path_dentry(&ad, dentry);

This will set common audit data type to LSM_AUDIT_DATA_DENTRY
and store dentry for auditing (by function smk_curacc(), which in turn calls
dump_common_audit_data(), which is actually uses provided data and logs it).

	/*
	 * You need write access to the thing you're unlinking
	 */
	rc = smk_curacc(smk_of_inode(ip), MAY_WRITE, &ad);
	if (rc == 0) {
		/*
		 * You also need write access to the containing directory
		 */

Then this function wants to log anoter data:

		smk_ad_setfield_u_fs_path_dentry(&ad, NULL);
		smk_ad_setfield_u_fs_inode(&ad, dir);

The function sets inode field, but don't change common_audit_data type.

		rc = smk_curacc(smk_of_inode(dir), MAY_WRITE, &ad);
	}

So the dump_common_audit() function incorrectly interprets inode structure
as dentry, and Oops will happen.

This patch reinitializes common_audit_data structures with correct type.
Also I removed unneeded
	smk_ad_setfield_u_fs_path_dentry(&ad, NULL);
initialization, because both dentry and inode pointers are stored
in the same union.

Signed-off-by: Igor Zhbanov <i.zhbanov@...sung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@...sung.com>
---
 security/smack/smack_lsm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index fa64740..d52c780 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -654,7 +654,7 @@ static int smack_inode_unlink(struct inode *dir, struct dentry *dentry)
 		/*
 		 * You also need write access to the containing directory
 		 */
-		smk_ad_setfield_u_fs_path_dentry(&ad, NULL);
+		smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_INODE);
 		smk_ad_setfield_u_fs_inode(&ad, dir);
 		rc = smk_curacc(smk_of_inode(dir), MAY_WRITE, &ad);
 	}
@@ -685,7 +685,7 @@ static int smack_inode_rmdir(struct inode *dir, struct dentry *dentry)
 		/*
 		 * You also need write access to the containing directory
 		 */
-		smk_ad_setfield_u_fs_path_dentry(&ad, NULL);
+		smk_ad_init(&ad, __func__, LSM_AUDIT_DATA_INODE);
 		smk_ad_setfield_u_fs_inode(&ad, dir);
 		rc = smk_curacc(smk_of_inode(dir), MAY_WRITE, &ad);
 	}
-- 
1.7.5.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/