Skip to content

Commit d27c712

Browse files
dhowellsbrauner
authored andcommitted
afs: Fix delayed allocation of a cell's anonymous key
The allocation of a cell's anonymous key is done in a background thread along with other cell setup such as doing a DNS upcall. In the reported bug, this is triggered by afs_parse_source() parsing the device name given to mount() and calling afs_lookup_cell() with the name of the cell. The normal key lookup then tries to use the key description on the anonymous authentication key as the reference for request_key() - but it may not yet be set and so an oops can happen. This has been made more likely to happen by the fix for dynamic lookup failure. Fix this by firstly allocating a reference name and attaching it to the afs_cell record when the record is created. It can share the memory allocation with the cell name (unfortunately it can't just overlap the cell name by prepending it with "afs@" as the cell name already has a '.' prepended for other purposes). This reference name is then passed to request_key(). Secondly, the anon key is now allocated on demand at the point a key is requested in afs_request_key() if it is not already allocated. A mutex is used to prevent multiple allocation for a cell. Thirdly, make afs_request_key_rcu() return NULL if the anonymous key isn't yet allocated (if we need it) and then the caller can return -ECHILD to drop out of RCU-mode and afs_request_key() can be called. Note that the anonymous key is kind of necessary to make the key lookup cache work as that doesn't currently cache a negative lookup, but it's probably worth some investigation to see if NULL can be used instead. Fixes: 330e2c5 ("afs: Fix dynamic lookup to fail on cell lookup failure") Reported-by: syzbot+41c68824eefb67cdf00c@syzkaller.appspotmail.com Signed-off-by: David Howells <dhowells@redhat.com> Link: https://patch.msgid.link/800328.1764325145@warthog.procyon.org.uk cc: Marc Dionne <marc.dionne@auristor.com> cc: linux-afs@lists.infradead.org cc: linux-fsdevel@vger.kernel.org Signed-off-by: Christian Brauner <brauner@kernel.org>
1 parent e9c7008 commit d27c712

3 files changed

Lines changed: 49 additions & 43 deletions

File tree

fs/afs/cell.c

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,9 @@ static struct afs_cell *afs_alloc_cell(struct afs_net *net,
140140
return ERR_PTR(-ENOMEM);
141141
}
142142

143-
cell->name = kmalloc(1 + namelen + 1, GFP_KERNEL);
143+
/* Allocate the cell name and the key name in one go. */
144+
cell->name = kmalloc(1 + namelen + 1 +
145+
4 + namelen + 1, GFP_KERNEL);
144146
if (!cell->name) {
145147
kfree(cell);
146148
return ERR_PTR(-ENOMEM);
@@ -151,7 +153,11 @@ static struct afs_cell *afs_alloc_cell(struct afs_net *net,
151153
cell->name_len = namelen;
152154
for (i = 0; i < namelen; i++)
153155
cell->name[i] = tolower(name[i]);
154-
cell->name[i] = 0;
156+
cell->name[i++] = 0;
157+
158+
cell->key_desc = cell->name + i;
159+
memcpy(cell->key_desc, "afs@", 4);
160+
memcpy(cell->key_desc + 4, cell->name, cell->name_len + 1);
155161

156162
cell->net = net;
157163
refcount_set(&cell->ref, 1);
@@ -710,33 +716,6 @@ void afs_set_cell_timer(struct afs_cell *cell, unsigned int delay_secs)
710716
timer_reduce(&cell->management_timer, jiffies + delay_secs * HZ);
711717
}
712718

713-
/*
714-
* Allocate a key to use as a placeholder for anonymous user security.
715-
*/
716-
static int afs_alloc_anon_key(struct afs_cell *cell)
717-
{
718-
struct key *key;
719-
char keyname[4 + AFS_MAXCELLNAME + 1], *cp, *dp;
720-
721-
/* Create a key to represent an anonymous user. */
722-
memcpy(keyname, "afs@", 4);
723-
dp = keyname + 4;
724-
cp = cell->name;
725-
do {
726-
*dp++ = tolower(*cp);
727-
} while (*cp++);
728-
729-
key = rxrpc_get_null_key(keyname);
730-
if (IS_ERR(key))
731-
return PTR_ERR(key);
732-
733-
cell->anonymous_key = key;
734-
735-
_debug("anon key %p{%x}",
736-
cell->anonymous_key, key_serial(cell->anonymous_key));
737-
return 0;
738-
}
739-
740719
/*
741720
* Activate a cell.
742721
*/
@@ -746,12 +725,6 @@ static int afs_activate_cell(struct afs_net *net, struct afs_cell *cell)
746725
struct afs_cell *pcell;
747726
int ret;
748727

749-
if (!cell->anonymous_key) {
750-
ret = afs_alloc_anon_key(cell);
751-
if (ret < 0)
752-
return ret;
753-
}
754-
755728
ret = afs_proc_cell_setup(cell);
756729
if (ret < 0)
757730
return ret;

fs/afs/internal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,7 @@ struct afs_cell {
413413

414414
u8 name_len; /* Length of name */
415415
char *name; /* Cell name, case-flattened and NUL-padded */
416+
char *key_desc; /* Authentication key description */
416417
};
417418

418419
/*

fs/afs/security.c

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,25 +16,56 @@
1616

1717
static DEFINE_HASHTABLE(afs_permits_cache, 10);
1818
static DEFINE_SPINLOCK(afs_permits_lock);
19+
static DEFINE_MUTEX(afs_key_lock);
20+
21+
/*
22+
* Allocate a key to use as a placeholder for anonymous user security.
23+
*/
24+
static int afs_alloc_anon_key(struct afs_cell *cell)
25+
{
26+
struct key *key;
27+
28+
mutex_lock(&afs_key_lock);
29+
if (!cell->anonymous_key) {
30+
key = rxrpc_get_null_key(cell->key_desc);
31+
if (!IS_ERR(key))
32+
cell->anonymous_key = key;
33+
}
34+
mutex_unlock(&afs_key_lock);
35+
36+
if (IS_ERR(key))
37+
return PTR_ERR(key);
38+
39+
_debug("anon key %p{%x}",
40+
cell->anonymous_key, key_serial(cell->anonymous_key));
41+
return 0;
42+
}
1943

2044
/*
2145
* get a key
2246
*/
2347
struct key *afs_request_key(struct afs_cell *cell)
2448
{
2549
struct key *key;
50+
int ret;
2651

27-
_enter("{%x}", key_serial(cell->anonymous_key));
52+
_enter("{%s}", cell->key_desc);
2853

29-
_debug("key %s", cell->anonymous_key->description);
30-
key = request_key_net(&key_type_rxrpc, cell->anonymous_key->description,
54+
_debug("key %s", cell->key_desc);
55+
key = request_key_net(&key_type_rxrpc, cell->key_desc,
3156
cell->net->net, NULL);
3257
if (IS_ERR(key)) {
3358
if (PTR_ERR(key) != -ENOKEY) {
3459
_leave(" = %ld", PTR_ERR(key));
3560
return key;
3661
}
3762

63+
if (!cell->anonymous_key) {
64+
ret = afs_alloc_anon_key(cell);
65+
if (ret < 0)
66+
return ERR_PTR(ret);
67+
}
68+
3869
/* act as anonymous user */
3970
_leave(" = {%x} [anon]", key_serial(cell->anonymous_key));
4071
return key_get(cell->anonymous_key);
@@ -52,11 +83,10 @@ struct key *afs_request_key_rcu(struct afs_cell *cell)
5283
{
5384
struct key *key;
5485

55-
_enter("{%x}", key_serial(cell->anonymous_key));
86+
_enter("{%s}", cell->key_desc);
5687

57-
_debug("key %s", cell->anonymous_key->description);
58-
key = request_key_net_rcu(&key_type_rxrpc,
59-
cell->anonymous_key->description,
88+
_debug("key %s", cell->key_desc);
89+
key = request_key_net_rcu(&key_type_rxrpc, cell->key_desc,
6090
cell->net->net);
6191
if (IS_ERR(key)) {
6292
if (PTR_ERR(key) != -ENOKEY) {
@@ -65,6 +95,8 @@ struct key *afs_request_key_rcu(struct afs_cell *cell)
6595
}
6696

6797
/* act as anonymous user */
98+
if (!cell->anonymous_key)
99+
return NULL; /* Need to allocate */
68100
_leave(" = {%x} [anon]", key_serial(cell->anonymous_key));
69101
return key_get(cell->anonymous_key);
70102
} else {
@@ -408,7 +440,7 @@ int afs_permission(struct mnt_idmap *idmap, struct inode *inode,
408440

409441
if (mask & MAY_NOT_BLOCK) {
410442
key = afs_request_key_rcu(vnode->volume->cell);
411-
if (IS_ERR(key))
443+
if (IS_ERR_OR_NULL(key))
412444
return -ECHILD;
413445

414446
ret = -ECHILD;

0 commit comments

Comments
 (0)