Skip to content

Commit 0837e49

Browse files
dhowellsJames Morris
authored andcommitted
KEYS: Differentiate uses of rcu_dereference_key() and user_key_payload()
rcu_dereference_key() and user_key_payload() are currently being used in two different, incompatible ways: (1) As a wrapper to rcu_dereference() - when only the RCU read lock used to protect the key. (2) As a wrapper to rcu_dereference_protected() - when the key semaphor is used to protect the key and the may be being modified. Fix this by splitting both of the key wrappers to produce: (1) RCU accessors for keys when caller has the key semaphore locked: dereference_key_locked() user_key_payload_locked() (2) RCU accessors for keys when caller holds the RCU read lock: dereference_key_rcu() user_key_payload_rcu() This should fix following warning in the NFS idmapper =============================== [ INFO: suspicious RCU usage. ] 4.10.0 #1 Tainted: G W ------------------------------- ./include/keys/user-type.h:53 suspicious rcu_dereference_protected() usage! other info that might help us debug this: rcu_scheduler_active = 2, debug_locks = 0 1 lock held by mount.nfs/5987: #0: (rcu_read_lock){......}, at: [<d000000002527abc>] nfs_idmap_get_key+0x15c/0x420 [nfsv4] stack backtrace: CPU: 1 PID: 5987 Comm: mount.nfs Tainted: G W 4.10.0 #1 Call Trace: dump_stack+0xe8/0x154 (unreliable) lockdep_rcu_suspicious+0x140/0x190 nfs_idmap_get_key+0x380/0x420 [nfsv4] nfs_map_name_to_uid+0x2a0/0x3b0 [nfsv4] decode_getfattr_attrs+0xfac/0x16b0 [nfsv4] decode_getfattr_generic.constprop.106+0xbc/0x150 [nfsv4] nfs4_xdr_dec_lookup_root+0xac/0xb0 [nfsv4] rpcauth_unwrap_resp+0xe8/0x140 [sunrpc] call_decode+0x29c/0x910 [sunrpc] __rpc_execute+0x140/0x8f0 [sunrpc] rpc_run_task+0x170/0x200 [sunrpc] nfs4_call_sync_sequence+0x68/0xa0 [nfsv4] _nfs4_lookup_root.isra.44+0xd0/0xf0 [nfsv4] nfs4_lookup_root+0xe0/0x350 [nfsv4] nfs4_lookup_root_sec+0x70/0xa0 [nfsv4] nfs4_find_root_sec+0xc4/0x100 [nfsv4] nfs4_proc_get_rootfh+0x5c/0xf0 [nfsv4] nfs4_get_rootfh+0x6c/0x190 [nfsv4] nfs4_server_common_setup+0xc4/0x260 [nfsv4] nfs4_create_server+0x278/0x3c0 [nfsv4] nfs4_remote_mount+0x50/0xb0 [nfsv4] mount_fs+0x74/0x210 vfs_kern_mount+0x78/0x220 nfs_do_root_mount+0xb0/0x140 [nfsv4] nfs4_try_mount+0x60/0x100 [nfsv4] nfs_fs_mount+0x5ec/0xda0 [nfs] mount_fs+0x74/0x210 vfs_kern_mount+0x78/0x220 do_mount+0x254/0xf70 SyS_mount+0x94/0x100 system_call+0x38/0xe0 Reported-by: Jan Stancek <jstancek@redhat.com> Signed-off-by: David Howells <dhowells@redhat.com> Tested-by: Jan Stancek <jstancek@redhat.com> Signed-off-by: James Morris <james.l.morris@oracle.com>
1 parent 6053dc9 commit 0837e49

File tree

15 files changed

+43
-22
lines changed

15 files changed

+43
-22
lines changed

Documentation/security/keys.txt

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1151,8 +1151,21 @@ access the data:
11511151
usage. This is called key->payload.rcu_data0. The following accessors
11521152
wrap the RCU calls to this element:
11531153

1154-
rcu_assign_keypointer(struct key *key, void *data);
1155-
void *rcu_dereference_key(struct key *key);
1154+
(a) Set or change the first payload pointer:
1155+
1156+
rcu_assign_keypointer(struct key *key, void *data);
1157+
1158+
(b) Read the first payload pointer with the key semaphore held:
1159+
1160+
[const] void *dereference_key_locked([const] struct key *key);
1161+
1162+
Note that the return value will inherit its constness from the key
1163+
parameter. Static analysis will give an error if it things the lock
1164+
isn't held.
1165+
1166+
(c) Read the first payload pointer with the RCU read lock held:
1167+
1168+
const void *dereference_key_rcu(const struct key *key);
11561169

11571170

11581171
===================

drivers/md/dm-crypt.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1536,7 +1536,7 @@ static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string
15361536

15371537
down_read(&key->sem);
15381538

1539-
ukp = user_key_payload(key);
1539+
ukp = user_key_payload_locked(key);
15401540
if (!ukp) {
15411541
up_read(&key->sem);
15421542
key_put(key);

fs/cifs/connect.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2455,7 +2455,7 @@ cifs_set_cifscreds(struct smb_vol *vol, struct cifs_ses *ses)
24552455
}
24562456

24572457
down_read(&key->sem);
2458-
upayload = user_key_payload(key);
2458+
upayload = user_key_payload_locked(key);
24592459
if (IS_ERR_OR_NULL(upayload)) {
24602460
rc = upayload ? PTR_ERR(upayload) : -EINVAL;
24612461
goto out_key_put;

fs/crypto/keyinfo.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
103103
goto out;
104104
}
105105
down_read(&keyring_key->sem);
106-
ukp = user_key_payload(keyring_key);
106+
ukp = user_key_payload_locked(keyring_key);
107107
if (ukp->datalen != sizeof(struct fscrypt_key)) {
108108
res = -EINVAL;
109109
up_read(&keyring_key->sem);

fs/ecryptfs/ecryptfs_kernel.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ ecryptfs_get_key_payload_data(struct key *key)
117117

118118
auth_tok = ecryptfs_get_encrypted_key_payload_data(key);
119119
if (!auth_tok)
120-
return (struct ecryptfs_auth_tok *)user_key_payload(key)->data;
120+
return (struct ecryptfs_auth_tok *)user_key_payload_locked(key)->data;
121121
else
122122
return auth_tok;
123123
}

fs/fscache/object-list.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ static void fscache_objlist_config(struct fscache_objlist_data *data)
329329
config = 0;
330330
rcu_read_lock();
331331

332-
confkey = user_key_payload(key);
332+
confkey = user_key_payload_rcu(key);
333333
buf = confkey->data;
334334

335335
for (len = confkey->datalen - 1; len >= 0; len--) {

fs/nfs/nfs4idmap.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ static ssize_t nfs_idmap_get_key(const char *name, size_t namelen,
316316
if (ret < 0)
317317
goto out_up;
318318

319-
payload = user_key_payload(rkey);
319+
payload = user_key_payload_rcu(rkey);
320320
if (IS_ERR_OR_NULL(payload)) {
321321
ret = PTR_ERR(payload);
322322
goto out_up;

include/keys/user-type.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,14 @@ extern void user_describe(const struct key *user, struct seq_file *m);
4848
extern long user_read(const struct key *key,
4949
char __user *buffer, size_t buflen);
5050

51-
static inline const struct user_key_payload *user_key_payload(const struct key *key)
51+
static inline const struct user_key_payload *user_key_payload_rcu(const struct key *key)
5252
{
53-
return (struct user_key_payload *)rcu_dereference_key(key);
53+
return (struct user_key_payload *)dereference_key_rcu(key);
54+
}
55+
56+
static inline struct user_key_payload *user_key_payload_locked(const struct key *key)
57+
{
58+
return (struct user_key_payload *)dereference_key_locked((struct key *)key);
5459
}
5560

5661
#endif /* CONFIG_KEYS */

include/linux/key.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,10 @@ static inline bool key_is_instantiated(const struct key *key)
354354
!test_bit(KEY_FLAG_NEGATIVE, &key->flags);
355355
}
356356

357-
#define rcu_dereference_key(KEY) \
357+
#define dereference_key_rcu(KEY) \
358+
(rcu_dereference((KEY)->payload.rcu_data0))
359+
360+
#define dereference_key_locked(KEY) \
358361
(rcu_dereference_protected((KEY)->payload.rcu_data0, \
359362
rwsem_is_locked(&((struct key *)(KEY))->sem)))
360363

lib/digsig.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ static int digsig_verify_rsa(struct key *key,
8585
struct pubkey_hdr *pkh;
8686

8787
down_read(&key->sem);
88-
ukp = user_key_payload(key);
88+
ukp = user_key_payload_locked(key);
8989

9090
if (ukp->datalen < sizeof(*pkh))
9191
goto err1;

0 commit comments

Comments
 (0)