Skip to content

Commit

Permalink
big_keys: Use struct for internal payload
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Cc: Eric Biggers <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Jarkko Sakkinen <[email protected]>
Cc: James Morris <[email protected]>
Cc: "Serge E. Hallyn" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Kees Cook <[email protected]>
  • Loading branch information
@kees
kees committed May 16, 2022
1 parent 61f60ba commit c1298a3
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 39 deletions.
2 changes: 0 additions & 2 deletions scripts/gcc-plugins/randomize_layout_plugin.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 *" },
{ }
};

Expand Down
73 changes: 36 additions & 37 deletions security/keys/big_key.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -55,21 +56,23 @@ 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;
size_t datalen = prep->datalen;
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
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
Expand All @@ -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);
}

/*
Expand All @@ -162,31 +164,28 @@ 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);
}

/*
* dispose of the data dangling from the corpse of a big_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;
}

/*
Expand All @@ -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");
}

/*
Expand All @@ -227,24 +226,24 @@ 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;

buf = kvmalloc(enclen, GFP_KERNEL);
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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit c1298a3

Please sign in to comment.