linux-kernel - Re: [PATCH v2 07/20] randstruct: Whitelist big

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170528081249.GD22193@infradead.org>
Date:   Sun, 28 May 2017 01:12:49 -0700
From:   Christoph Hellwig <hch@...radead.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     kernel-hardening@...ts.openwall.com,
        David Howells <dhowells@...hat.com>,
        Laura Abbott <labbott@...hat.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 07/20] randstruct: Whitelist big_key path struct
 overloading

What about the untested patch below to just fix the issue?

---
>From e9eb519c854d2f3d16a4def492577a883246e290 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@....de>
Date: Sun, 28 May 2017 11:03:34 +0300
Subject: security/keys: don't cast union key_payload

Instead store the individual pointers in struct path.

Signed-off-by: Christoph Hellwig <hch@....de>
---
 security/keys/big_key.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index 835c1ab30d01..06f2cd07dbd7 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -26,8 +26,8 @@
  */
 enum {
 	big_key_data,
-	big_key_path,
-	big_key_path_2nd_part,
+	big_key_path_mnt,
+	big_key_path_dentry,
 	big_key_len,
 };
 
@@ -118,12 +118,16 @@ static int big_key_crypt(enum big_key_op op, u8 *data, size_t datalen, u8 *key)
 	return ret;
 }
 
+#define PATH_FROM_PAYLOAD(p) {				\
+	.mnt	= (p)->data[big_key_path_mnt],		\
+	.dentry	= (p)->data[big_key_path_dentry],	\
+}
+
 /*
  * Preparse a big key
  */
 int big_key_preparse(struct key_preparsed_payload *prep)
 {
-	struct path *path = (struct path *)&prep->payload.data[big_key_path];
 	struct file *file;
 	u8 *enckey;
 	u8 *data = NULL;
@@ -190,9 +194,10 @@ int big_key_preparse(struct key_preparsed_payload *prep)
 		/* Pin the mount and dentry to the key so that we can open it again
 		 * later
 		 */
+		path_get(&file->f_path);
 		prep->payload.data[big_key_data] = enckey;
-		*path = file->f_path;
-		path_get(path);
+		prep->payload.data[big_key_path_mnt] = file->f_path.mnt;
+		prep->payload.data[big_key_path_dentry] = file->f_path.dentry;
 		fput(file);
 		kfree(data);
 	} else {
@@ -222,9 +227,9 @@ int big_key_preparse(struct key_preparsed_payload *prep)
 void big_key_free_preparse(struct key_preparsed_payload *prep)
 {
 	if (prep->datalen > BIG_KEY_FILE_THRESHOLD) {
-		struct path *path = (struct path *)&prep->payload.data[big_key_path];
+		struct path path = PATH_FROM_PAYLOAD(&prep->payload);
 
-		path_put(path);
+		path_put(&path);
 	}
 	kfree(prep->payload.data[big_key_data]);
 }
@@ -235,13 +240,13 @@ void big_key_free_preparse(struct key_preparsed_payload *prep)
  */
 void big_key_revoke(struct key *key)
 {
-	struct path *path = (struct path *)&key->payload.data[big_key_path];
+	struct path path = PATH_FROM_PAYLOAD(&key->payload);
 
 	/* clear the quota */
 	key_payload_reserve(key, 0);
 	if (key_is_instantiated(key) &&
 	    (size_t)key->payload.data[big_key_len] > BIG_KEY_FILE_THRESHOLD)
-		vfs_truncate(path, 0);
+		vfs_truncate(&path, 0);
 }
 
 /*
@@ -252,11 +257,11 @@ void big_key_destroy(struct key *key)
 	size_t datalen = (size_t)key->payload.data[big_key_len];
 
 	if (datalen > BIG_KEY_FILE_THRESHOLD) {
-		struct path *path = (struct path *)&key->payload.data[big_key_path];
+		struct path path = PATH_FROM_PAYLOAD(&key->payload);
 
-		path_put(path);
-		path->mnt = NULL;
-		path->dentry = NULL;
+		path_put(&path);
+		key->payload.data[big_key_path_mnt] = NULL;
+		key->payload.data[big_key_path_dentry] = NULL;
 	}
 	kfree(key->payload.data[big_key_data]);
 	key->payload.data[big_key_data] = NULL;
@@ -290,7 +295,7 @@ long big_key_read(const struct key *key, char __user *buffer, size_t buflen)
 		return datalen;
 
 	if (datalen > BIG_KEY_FILE_THRESHOLD) {
-		struct path *path = (struct path *)&key->payload.data[big_key_path];
+		struct path path = PATH_FROM_PAYLOAD(&key->payload);
 		struct file *file;
 		u8 *data;
 		u8 *enckey = (u8 *)key->payload.data[big_key_data];
@@ -300,7 +305,7 @@ long big_key_read(const struct key *key, char __user *buffer, size_t buflen)
 		if (!data)
 			return -ENOMEM;
 
-		file = dentry_open(path, O_RDONLY, current_cred());
+		file = dentry_open(&path, O_RDONLY, current_cred());
 		if (IS_ERR(file)) {
 			ret = PTR_ERR(file);
 			goto error;
-- 
2.11.0