[<prev] [next>] [day] [month] [year] [list]
Message-Id: <20220510235817.3627784-1-keescook@chromium.org>
Date: Tue, 10 May 2022 16:58:17 -0700
From: Kees Cook <keescook@...omium.org>
To: David Howells <dhowells@...hat.com>
Cc: Kees Cook <keescook@...omium.org>,
Eric Biggers <ebiggers@...nel.org>,
Christoph Hellwig <hch@...radead.org>,
Jarkko Sakkinen <jarkko@...nel.org>,
James Morris <jmorris@...ei.org>,
"Serge E. Hallyn" <serge@...lyn.com>,
linux-hardening@...r.kernel.org, keyrings@...r.kernel.org,
linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH v2] big_keys: Use struct for internal payload
The randstruct GCC plugin gets upset when it sees struct path (which is
randomized) being assigned from a "void *" (which it cannot type-check).
There's no need for these casts, as the entire internal payload use is
following a normal struct layout. Convert the enum-based void * offset
dereferencing to the new big_key_payload struct. No meaningful machine
code changes result after this change, and source readability is improved.
Drop the randstruct exception now that there is no "confusing" cross-type
assignment.
Cc: David Howells <dhowells@...hat.com>
Cc: Eric Biggers <ebiggers@...nel.org>
Cc: Christoph Hellwig <hch@...radead.org>
Cc: Jarkko Sakkinen <jarkko@...nel.org>
Cc: James Morris <jmorris@...ei.org>
Cc: "Serge E. Hallyn" <serge@...lyn.com>
Cc: linux-hardening@...r.kernel.org
Cc: keyrings@...r.kernel.org
Cc: linux-security-module@...r.kernel.org
Signed-off-by: Kees Cook <keescook@...omium.org>
---
scripts/gcc-plugins/randomize_layout_plugin.c | 2 -
security/keys/big_key.c | 73 +++++++++----------
2 files changed, 36 insertions(+), 39 deletions(-)
diff --git a/scripts/gcc-plugins/randomize_layout_plugin.c b/scripts/gcc-plugins/randomize_layout_plugin.c
index 19214e573137..5836a7fc7532 100644
--- a/scripts/gcc-plugins/randomize_layout_plugin.c
+++ b/scripts/gcc-plugins/randomize_layout_plugin.c
@@ -50,8 +50,6 @@ static const struct whitelist_entry whitelist[] = {
{ "drivers/net/ethernet/sun/niu.c", "page", "address_space" },
/* unix_skb_parms via UNIXCB() buffer */
{ "net/unix/af_unix.c", "unix_skb_parms", "char" },
- /* big_key payload.data struct splashing */
- { "security/keys/big_key.c", "path", "void *" },
{ }
};
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index d17e5f09eeb8..c3367622c683 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -20,12 +20,13 @@
/*
* Layout of key payload words.
*/
-enum {
- big_key_data,
- big_key_path,
- big_key_path_2nd_part,
- big_key_len,
+struct big_key_payload {
+ u8 *data;
+ struct path path;
+ size_t length;
};
+#define to_big_key_payload(payload) \
+ (struct big_key_payload *)((payload).data)
/*
* If the data is under this limit, there's no point creating a shm file to
@@ -55,7 +56,7 @@ struct key_type key_type_big_key = {
*/
int big_key_preparse(struct key_preparsed_payload *prep)
{
- struct path *path = (struct path *)&prep->payload.data[big_key_path];
+ struct big_key_payload *payload = to_big_key_payload(prep->payload);
struct file *file;
u8 *buf, *enckey;
ssize_t written;
@@ -63,13 +64,15 @@ int big_key_preparse(struct key_preparsed_payload *prep)
size_t enclen = datalen + CHACHA20POLY1305_AUTHTAG_SIZE;
int ret;
+ BUILD_BUG_ON(sizeof(*payload) != sizeof(prep->payload.data));
+
if (datalen <= 0 || datalen > 1024 * 1024 || !prep->data)
return -EINVAL;
/* Set an arbitrary quota */
prep->quotalen = 16;
- prep->payload.data[big_key_len] = (void *)(unsigned long)datalen;
+ payload->length = datalen;
if (datalen > BIG_KEY_FILE_THRESHOLD) {
/* Create a shmem file to store the data in. This will permit the data
@@ -117,9 +120,9 @@ 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
*/
- prep->payload.data[big_key_data] = enckey;
- *path = file->f_path;
- path_get(path);
+ payload->data = enckey;
+ payload->path = file->f_path;
+ path_get(&payload->path);
fput(file);
kvfree_sensitive(buf, enclen);
} else {
@@ -129,7 +132,7 @@ int big_key_preparse(struct key_preparsed_payload *prep)
if (!data)
return -ENOMEM;
- prep->payload.data[big_key_data] = data;
+ payload->data = data;
memcpy(data, prep->data, prep->datalen);
}
return 0;
@@ -148,12 +151,11 @@ 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 big_key_payload *payload = to_big_key_payload(prep->payload);
- path_put(path);
- }
- kfree_sensitive(prep->payload.data[big_key_data]);
+ if (prep->datalen > BIG_KEY_FILE_THRESHOLD)
+ path_put(&payload->path);
+ kfree_sensitive(payload->data);
}
/*
@@ -162,13 +164,12 @@ 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 big_key_payload *payload = to_big_key_payload(key->payload);
/* clear the quota */
key_payload_reserve(key, 0);
- if (key_is_positive(key) &&
- (size_t)key->payload.data[big_key_len] > BIG_KEY_FILE_THRESHOLD)
- vfs_truncate(path, 0);
+ if (key_is_positive(key) && payload->length > BIG_KEY_FILE_THRESHOLD)
+ vfs_truncate(&payload->path, 0);
}
/*
@@ -176,17 +177,15 @@ void big_key_revoke(struct key *key)
*/
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 big_key_payload *payload = to_big_key_payload(key->payload);
- path_put(path);
- path->mnt = NULL;
- path->dentry = NULL;
+ if (payload->length > BIG_KEY_FILE_THRESHOLD) {
+ path_put(&payload->path);
+ payload->path.mnt = NULL;
+ payload->path.dentry = NULL;
}
- kfree_sensitive(key->payload.data[big_key_data]);
- key->payload.data[big_key_data] = NULL;
+ kfree_sensitive(payload->data);
+ payload->data = NULL;
}
/*
@@ -211,14 +210,14 @@ int big_key_update(struct key *key, struct key_preparsed_payload *prep)
*/
void big_key_describe(const struct key *key, struct seq_file *m)
{
- size_t datalen = (size_t)key->payload.data[big_key_len];
+ struct big_key_payload *payload = to_big_key_payload(key->payload);
seq_puts(m, key->description);
if (key_is_positive(key))
seq_printf(m, ": %zu [%s]",
- datalen,
- datalen > BIG_KEY_FILE_THRESHOLD ? "file" : "buff");
+ payload->length,
+ payload->length > BIG_KEY_FILE_THRESHOLD ? "file" : "buff");
}
/*
@@ -227,16 +226,16 @@ void big_key_describe(const struct key *key, struct seq_file *m)
*/
long big_key_read(const struct key *key, char *buffer, size_t buflen)
{
- size_t datalen = (size_t)key->payload.data[big_key_len];
+ struct big_key_payload *payload = to_big_key_payload(key->payload);
+ size_t datalen = payload->length;
long ret;
if (!buffer || buflen < datalen)
return datalen;
if (datalen > BIG_KEY_FILE_THRESHOLD) {
- struct path *path = (struct path *)&key->payload.data[big_key_path];
struct file *file;
- u8 *buf, *enckey = (u8 *)key->payload.data[big_key_data];
+ u8 *buf, *enckey = payload->data;
size_t enclen = datalen + CHACHA20POLY1305_AUTHTAG_SIZE;
loff_t pos = 0;
@@ -244,7 +243,7 @@ long big_key_read(const struct key *key, char *buffer, size_t buflen)
if (!buf)
return -ENOMEM;
- file = dentry_open(path, O_RDONLY, current_cred());
+ file = dentry_open(&payload->path, O_RDONLY, current_cred());
if (IS_ERR(file)) {
ret = PTR_ERR(file);
goto error;
@@ -274,7 +273,7 @@ long big_key_read(const struct key *key, char *buffer, size_t buflen)
kvfree_sensitive(buf, enclen);
} else {
ret = datalen;
- memcpy(buffer, key->payload.data[big_key_data], datalen);
+ memcpy(buffer, payload->data, datalen);
}
return ret;
--
2.32.0
Powered by blists - more mailing lists