Skip to content

Commit 822ad64

Browse files
dhowellsJames Morris
dhowells
authored and
James Morris
committed
keys: Fix dependency loop between construction record and auth key
In the request_key() upcall mechanism there's a dependency loop by which if a key type driver overrides the ->request_key hook and the userspace side manages to lose the authorisation key, the auth key and the internal construction record (struct key_construction) can keep each other pinned. Fix this by the following changes: (1) Killing off the construction record and using the auth key instead. (2) Including the operation name in the auth key payload and making the payload available outside of security/keys/. (3) The ->request_key hook is given the authkey instead of the cons record and operation name. Changes (2) and (3) allow the auth key to naturally be cleaned up if the keyring it is in is destroyed or cleared or the auth key is unlinked. Fixes: 7ee02a316600 ("keys: Fix dependency loop between construction record and auth key") Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: James Morris <james.morris@microsoft.com>
1 parent bb2ba2d commit 822ad64

File tree

8 files changed

+100
-92
lines changed

8 files changed

+100
-92
lines changed

‎fs/nfs/nfs4idmap.c

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
#include <linux/keyctl.h>
4545
#include <linux/key-type.h>
4646
#include <keys/user-type.h>
47+
#include <keys/request_key_auth-type.h>
4748
#include <linux/module.h>
4849

4950
#include "internal.h"
@@ -59,7 +60,7 @@ static struct key_type key_type_id_resolver_legacy;
5960
struct idmap_legacy_upcalldata {
6061
struct rpc_pipe_msg pipe_msg;
6162
struct idmap_msg idmap_msg;
62-
struct key_construction *key_cons;
63+
struct key *authkey;
6364
struct idmap *idmap;
6465
};
6566

@@ -384,7 +385,7 @@ static const match_table_t nfs_idmap_tokens = {
384385
{ Opt_find_err, NULL }
385386
};
386387

387-
static int nfs_idmap_legacy_upcall(struct key_construction *, const char *, void *);
388+
static int nfs_idmap_legacy_upcall(struct key *, void *);
388389
static ssize_t idmap_pipe_downcall(struct file *, const char __user *,
389390
size_t);
390391
static void idmap_release_pipe(struct inode *);
@@ -549,11 +550,12 @@ nfs_idmap_prepare_pipe_upcall(struct idmap *idmap,
549550
static void
550551
nfs_idmap_complete_pipe_upcall_locked(struct idmap *idmap, int ret)
551552
{
552-
struct key_construction *cons = idmap->idmap_upcall_data->key_cons;
553+
struct key *authkey = idmap->idmap_upcall_data->authkey;
553554

554555
kfree(idmap->idmap_upcall_data);
555556
idmap->idmap_upcall_data = NULL;
556-
complete_request_key(cons, ret);
557+
complete_request_key(authkey, ret);
558+
key_put(authkey);
557559
}
558560

559561
static void
@@ -563,15 +565,14 @@ nfs_idmap_abort_pipe_upcall(struct idmap *idmap, int ret)
563565
nfs_idmap_complete_pipe_upcall_locked(idmap, ret);
564566
}
565567

566-
static int nfs_idmap_legacy_upcall(struct key_construction *cons,
567-
const char *op,
568-
void *aux)
568+
static int nfs_idmap_legacy_upcall(struct key *authkey, void *aux)
569569
{
570570
struct idmap_legacy_upcalldata *data;
571+
struct request_key_auth *rka = get_request_key_auth(authkey);
571572
struct rpc_pipe_msg *msg;
572573
struct idmap_msg *im;
573574
struct idmap *idmap = (struct idmap *)aux;
574-
struct key *key = cons->key;
575+
struct key *key = rka->target_key;
575576
int ret = -ENOKEY;
576577

577578
if (!aux)
@@ -586,7 +587,7 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
586587
msg = &data->pipe_msg;
587588
im = &data->idmap_msg;
588589
data->idmap = idmap;
589-
data->key_cons = cons;
590+
data->authkey = key_get(authkey);
590591

591592
ret = nfs_idmap_prepare_message(key->description, idmap, im, msg);
592593
if (ret < 0)
@@ -604,7 +605,7 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
604605
out2:
605606
kfree(data);
606607
out1:
607-
complete_request_key(cons, ret);
608+
complete_request_key(authkey, ret);
608609
return ret;
609610
}
610611

@@ -651,9 +652,10 @@ static int nfs_idmap_read_and_verify_message(struct idmap_msg *im,
651652
static ssize_t
652653
idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
653654
{
655+
struct request_key_auth *rka;
654656
struct rpc_inode *rpci = RPC_I(file_inode(filp));
655657
struct idmap *idmap = (struct idmap *)rpci->private;
656-
struct key_construction *cons;
658+
struct key *authkey;
657659
struct idmap_msg im;
658660
size_t namelen_in;
659661
int ret = -ENOKEY;
@@ -665,7 +667,8 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
665667
if (idmap->idmap_upcall_data == NULL)
666668
goto out_noupcall;
667669

668-
cons = idmap->idmap_upcall_data->key_cons;
670+
authkey = idmap->idmap_upcall_data->authkey;
671+
rka = get_request_key_auth(authkey);
669672

670673
if (mlen != sizeof(im)) {
671674
ret = -ENOSPC;
@@ -690,9 +693,9 @@ idmap_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
690693

691694
ret = nfs_idmap_read_and_verify_message(&im,
692695
&idmap->idmap_upcall_data->idmap_msg,
693-
cons->key, cons->authkey);
696+
rka->target_key, authkey);
694697
if (ret >= 0) {
695-
key_set_timeout(cons->key, nfs_idmap_cache_timeout);
698+
key_set_timeout(rka->target_key, nfs_idmap_cache_timeout);
696699
ret = mlen;
697700
}
698701

‎include/keys/request_key_auth-type.h

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/* request_key authorisation token key type
2+
*
3+
* Copyright (C) 2005 Red Hat, Inc. All Rights Reserved.
4+
* Written by David Howells (dhowells@redhat.com)
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU General Public Licence
8+
* as published by the Free Software Foundation; either version
9+
* 2 of the Licence, or (at your option) any later version.
10+
*/
11+
12+
#ifndef _KEYS_REQUEST_KEY_AUTH_TYPE_H
13+
#define _KEYS_REQUEST_KEY_AUTH_TYPE_H
14+
15+
#include <linux/key.h>
16+
17+
/*
18+
* Authorisation record for request_key().
19+
*/
20+
struct request_key_auth {
21+
struct key *target_key;
22+
struct key *dest_keyring;
23+
const struct cred *cred;
24+
void *callout_info;
25+
size_t callout_len;
26+
pid_t pid;
27+
char op[8];
28+
} __randomize_layout;
29+
30+
static inline struct request_key_auth *get_request_key_auth(const struct key *key)
31+
{
32+
return key->payload.data[0];
33+
}
34+
35+
36+
#endif /* _KEYS_REQUEST_KEY_AUTH_TYPE_H */

‎include/linux/key-type.h

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,6 @@
2020
struct kernel_pkey_query;
2121
struct kernel_pkey_params;
2222

23-
/*
24-
* key under-construction record
25-
* - passed to the request_key actor if supplied
26-
*/
27-
struct key_construction {
28-
struct key *key; /* key being constructed */
29-
struct key *authkey;/* authorisation for key being constructed */
30-
};
31-
3223
/*
3324
* Pre-parsed payload, used by key add, update and instantiate.
3425
*
@@ -50,8 +41,7 @@ struct key_preparsed_payload {
5041
time64_t expiry; /* Expiry time of key */
5142
} __randomize_layout;
5243

53-
typedef int (*request_key_actor_t)(struct key_construction *key,
54-
const char *op, void *aux);
44+
typedef int (*request_key_actor_t)(struct key *auth_key, void *aux);
5545

5646
/*
5747
* Preparsed matching criterion.
@@ -181,20 +171,20 @@ extern int key_instantiate_and_link(struct key *key,
181171
const void *data,
182172
size_t datalen,
183173
struct key *keyring,
184-
struct key *instkey);
174+
struct key *authkey);
185175
extern int key_reject_and_link(struct key *key,
186176
unsigned timeout,
187177
unsigned error,
188178
struct key *keyring,
189-
struct key *instkey);
190-
extern void complete_request_key(struct key_construction *cons, int error);
179+
struct key *authkey);
180+
extern void complete_request_key(struct key *authkey, int error);
191181

192182
static inline int key_negate_and_link(struct key *key,
193183
unsigned timeout,
194184
struct key *keyring,
195-
struct key *instkey)
185+
struct key *authkey)
196186
{
197-
return key_reject_and_link(key, timeout, ENOKEY, keyring, instkey);
187+
return key_reject_and_link(key, timeout, ENOKEY, keyring, authkey);
198188
}
199189

200190
extern int generic_key_instantiate(struct key *key, struct key_preparsed_payload *prep);

‎security/keys/internal.h

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -186,20 +186,9 @@ static inline int key_permission(const key_ref_t key_ref, unsigned perm)
186186
return key_task_permission(key_ref, current_cred(), perm);
187187
}
188188

189-
/*
190-
* Authorisation record for request_key().
191-
*/
192-
struct request_key_auth {
193-
struct key *target_key;
194-
struct key *dest_keyring;
195-
const struct cred *cred;
196-
void *callout_info;
197-
size_t callout_len;
198-
pid_t pid;
199-
} __randomize_layout;
200-
201189
extern struct key_type key_type_request_key_auth;
202190
extern struct key *request_key_auth_new(struct key *target,
191+
const char *op,
203192
const void *callout_info,
204193
size_t callout_len,
205194
struct key *dest_keyring);

‎security/keys/keyctl.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <linux/security.h>
2626
#include <linux/uio.h>
2727
#include <linux/uaccess.h>
28+
#include <keys/request_key_auth-type.h>
2829
#include "internal.h"
2930

3031
#define KEY_MAX_DESC_SIZE 4096

‎security/keys/process_keys.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include <linux/security.h>
2020
#include <linux/user_namespace.h>
2121
#include <linux/uaccess.h>
22+
#include <keys/request_key_auth-type.h>
2223
#include "internal.h"
2324

2425
/* Session keyring create vs join semaphore */

‎security/keys/request_key.c

Lines changed: 29 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -18,31 +18,30 @@
1818
#include <linux/keyctl.h>
1919
#include <linux/slab.h>
2020
#include "internal.h"
21+
#include <keys/request_key_auth-type.h>
2122

2223
#define key_negative_timeout 60 /* default timeout on a negative key's existence */
2324

2425
/**
2526
* complete_request_key - Complete the construction of a key.
26-
* @cons: The key construction record.
27+
* @auth_key: The authorisation key.
2728
* @error: The success or failute of the construction.
2829
*
2930
* Complete the attempt to construct a key. The key will be negated
3031
* if an error is indicated. The authorisation key will be revoked
3132
* unconditionally.
3233
*/
33-
void complete_request_key(struct key_construction *cons, int error)
34+
void complete_request_key(struct key *authkey, int error)
3435
{
35-
kenter("{%d,%d},%d", cons->key->serial, cons->authkey->serial, error);
36+
struct request_key_auth *rka = get_request_key_auth(authkey);
37+
struct key *key = rka->target_key;
38+
39+
kenter("%d{%d},%d", authkey->serial, key->serial, error);
3640

3741
if (error < 0)
38-
key_negate_and_link(cons->key, key_negative_timeout, NULL,
39-
cons->authkey);
42+
key_negate_and_link(key, key_negative_timeout, NULL, authkey);
4043
else
41-
key_revoke(cons->authkey);
42-
43-
key_put(cons->key);
44-
key_put(cons->authkey);
45-
kfree(cons);
44+
key_revoke(authkey);
4645
}
4746
EXPORT_SYMBOL(complete_request_key);
4847

@@ -91,21 +90,19 @@ static int call_usermodehelper_keys(const char *path, char **argv, char **envp,
9190
* Request userspace finish the construction of a key
9291
* - execute "/sbin/request-key <op> <key> <uid> <gid> <keyring> <keyring> <keyring>"
9392
*/
94-
static int call_sbin_request_key(struct key_construction *cons,
95-
const char *op,
96-
void *aux)
93+
static int call_sbin_request_key(struct key *authkey, void *aux)
9794
{
9895
static char const request_key[] = "/sbin/request-key";
96+
struct request_key_auth *rka = get_request_key_auth(authkey);
9997
const struct cred *cred = current_cred();
10098
key_serial_t prkey, sskey;
101-
struct key *key = cons->key, *authkey = cons->authkey, *keyring,
102-
*session;
99+
struct key *key = rka->target_key, *keyring, *session;
103100
char *argv[9], *envp[3], uid_str[12], gid_str[12];
104101
char key_str[12], keyring_str[3][12];
105102
char desc[20];
106103
int ret, i;
107104

108-
kenter("{%d},{%d},%s", key->serial, authkey->serial, op);
105+
kenter("{%d},{%d},%s", key->serial, authkey->serial, rka->op);
109106

110107
ret = install_user_keyrings();
111108
if (ret < 0)
@@ -163,7 +160,7 @@ static int call_sbin_request_key(struct key_construction *cons,
163160
/* set up the argument list */
164161
i = 0;
165162
argv[i++] = (char *)request_key;
166-
argv[i++] = (char *) op;
163+
argv[i++] = (char *)rka->op;
167164
argv[i++] = key_str;
168165
argv[i++] = uid_str;
169166
argv[i++] = gid_str;
@@ -191,7 +188,7 @@ static int call_sbin_request_key(struct key_construction *cons,
191188
key_put(keyring);
192189

193190
error_alloc:
194-
complete_request_key(cons, ret);
191+
complete_request_key(authkey, ret);
195192
kleave(" = %d", ret);
196193
return ret;
197194
}
@@ -205,42 +202,31 @@ static int construct_key(struct key *key, const void *callout_info,
205202
size_t callout_len, void *aux,
206203
struct key *dest_keyring)
207204
{
208-
struct key_construction *cons;
209205
request_key_actor_t actor;
210206
struct key *authkey;
211207
int ret;
212208

213209
kenter("%d,%p,%zu,%p", key->serial, callout_info, callout_len, aux);
214210

215-
cons = kmalloc(sizeof(*cons), GFP_KERNEL);
216-
if (!cons)
217-
return -ENOMEM;
218-
219211
/* allocate an authorisation key */
220-
authkey = request_key_auth_new(key, callout_info, callout_len,
212+
authkey = request_key_auth_new(key, "create", callout_info, callout_len,
221213
dest_keyring);
222-
if (IS_ERR(authkey)) {
223-
kfree(cons);
224-
ret = PTR_ERR(authkey);
225-
authkey = NULL;
226-
} else {
227-
cons->authkey = key_get(authkey);
228-
cons->key = key_get(key);
214+
if (IS_ERR(authkey))
215+
return PTR_ERR(authkey);
229216

230-
/* make the call */
231-
actor = call_sbin_request_key;
232-
if (key->type->request_key)
233-
actor = key->type->request_key;
217+
/* Make the call */
218+
actor = call_sbin_request_key;
219+
if (key->type->request_key)
220+
actor = key->type->request_key;
234221

235-
ret = actor(cons, "create", aux);
222+
ret = actor(authkey, aux);
236223

237-
/* check that the actor called complete_request_key() prior to
238-
* returning an error */
239-
WARN_ON(ret < 0 &&
240-
!test_bit(KEY_FLAG_REVOKED, &authkey->flags));
241-
key_put(authkey);
242-
}
224+
/* check that the actor called complete_request_key() prior to
225+
* returning an error */
226+
WARN_ON(ret < 0 &&
227+
!test_bit(KEY_FLAG_REVOKED, &authkey->flags));
243228

229+
key_put(authkey);
244230
kleave(" = %d", ret);
245231
return ret;
246232
}
@@ -275,7 +261,7 @@ static int construct_get_dest_keyring(struct key **_dest_keyring)
275261
if (cred->request_key_auth) {
276262
authkey = cred->request_key_auth;
277263
down_read(&authkey->sem);
278-
rka = authkey->payload.data[0];
264+
rka = get_request_key_auth(authkey);
279265
if (!test_bit(KEY_FLAG_REVOKED,
280266
&authkey->flags))
281267
dest_keyring =

0 commit comments

Comments
 (0)