Skip to content

Commit fa4ccdc

Browse files
dhowellsgregkh
authored andcommitted
afs: Fix dynamic lookup to fail on cell lookup failure
[ Upstream commit 330e2c5 ] When a process tries to access an entry in /afs, normally what happens is that an automount dentry is created by ->lookup() and then triggered, which jumps through the ->d_automount() op. Currently, afs_dynroot_lookup() does not do cell DNS lookup, leaving that to afs_d_automount() to perform - however, it is possible to use access() or stat() on the automount point, which will always return successfully, have briefly created an afs_cell record if one did not already exist. This means that something like: test -d "/afs/.west" && echo Directory exists will print "Directory exists" even though no such cell is configured. This breaks the "west" python module available on PIP as it expects this access to fail. Now, it could be possible to make afs_dynroot_lookup() perform the DNS[*] lookup, but that would make "ls --color /afs" do this for each cell in /afs that is listed but not yet probed. kafs-client, probably wrongly, preloads the entire cell database and all the known cells are then listed in /afs - and doing ls /afs would be very, very slow, especially if any cell supplied addresses but was wholly inaccessible. [*] When I say "DNS", actually read getaddrinfo(), which could use any one of a host of mechanisms. Could also use static configuration. To fix this, make the following changes: (1) Create an enum to specify the origination point of a call to afs_lookup_cell() and pass this value into that function in place of the "excl" parameter (which can be derived from it). There are six points of origination: - Cell preload through /proc/net/afs/cells - Root cell config through /proc/net/afs/rootcell - Lookup in dynamic root - Automount trigger - Direct mount with mount() syscall - Alias check where YFS tells us the cell name is different (2) Add an extra state into the afs_cell state machine to indicate a cell that's been initialised, but not yet looked up. This is separate from one that can be considered active and has been looked up at least once. (3) Make afs_lookup_cell() vary its behaviour more, depending on where it was called from: If called from preload or root cell config, DNS lookup will not happen until we definitely want to use the cell (dynroot mount, automount, direct mount or alias check). The cell will appear in /afs but stat() won't trigger DNS lookup. If the cell already exists, dynroot will not wait for the DNS lookup to complete. If the cell did not already exist, dynroot will wait. If called from automount, direct mount or alias check, it will wait for the DNS lookup to complete. (4) Make afs_lookup_cell() return an error if lookup failed in one way or another. We try to return -ENOENT if the DNS says the cell does not exist and -EDESTADDRREQ if we couldn't access the DNS. Reported-by: Markus Suvanto <markus.suvanto@gmail.com> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220685 Signed-off-by: David Howells <dhowells@redhat.com> Link: https://patch.msgid.link/1784747.1761158912@warthog.procyon.org.uk Fixes: 1d0b929 ("afs: Change dynroot to create contents on demand") Tested-by: Markus Suvanto <markus.suvanto@gmail.com> cc: Marc Dionne <marc.dionne@auristor.com> cc: linux-afs@lists.infradead.org Signed-off-by: Christian Brauner <brauner@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent f28c9fc commit fa4ccdc

7 files changed

Lines changed: 86 additions & 18 deletions

File tree

fs/afs/cell.c

Lines changed: 66 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ static struct afs_cell *afs_alloc_cell(struct afs_net *net,
228228
* @name: The name of the cell.
229229
* @namesz: The strlen of the cell name.
230230
* @vllist: A colon/comma separated list of numeric IP addresses or NULL.
231-
* @excl: T if an error should be given if the cell name already exists.
231+
* @reason: The reason we're doing the lookup
232232
* @trace: The reason to be logged if the lookup is successful.
233233
*
234234
* Look up a cell record by name and query the DNS for VL server addresses if
@@ -238,20 +238,27 @@ static struct afs_cell *afs_alloc_cell(struct afs_net *net,
238238
*/
239239
struct afs_cell *afs_lookup_cell(struct afs_net *net,
240240
const char *name, unsigned int namesz,
241-
const char *vllist, bool excl,
241+
const char *vllist,
242+
enum afs_lookup_cell_for reason,
242243
enum afs_cell_trace trace)
243244
{
244245
struct afs_cell *cell, *candidate, *cursor;
245246
struct rb_node *parent, **pp;
246247
enum afs_cell_state state;
247248
int ret, n;
248249

249-
_enter("%s,%s", name, vllist);
250+
_enter("%s,%s,%u", name, vllist, reason);
250251

251-
if (!excl) {
252+
if (reason != AFS_LOOKUP_CELL_PRELOAD) {
252253
cell = afs_find_cell(net, name, namesz, trace);
253-
if (!IS_ERR(cell))
254+
if (!IS_ERR(cell)) {
255+
if (reason == AFS_LOOKUP_CELL_DYNROOT)
256+
goto no_wait;
257+
if (cell->state == AFS_CELL_SETTING_UP ||
258+
cell->state == AFS_CELL_UNLOOKED)
259+
goto lookup_cell;
254260
goto wait_for_cell;
261+
}
255262
}
256263

257264
/* Assume we're probably going to create a cell and preallocate and
@@ -297,34 +304,77 @@ struct afs_cell *afs_lookup_cell(struct afs_net *net,
297304
rb_insert_color(&cell->net_node, &net->cells);
298305
up_write(&net->cells_lock);
299306

300-
afs_queue_cell(cell, afs_cell_trace_queue_new);
307+
lookup_cell:
308+
if (reason != AFS_LOOKUP_CELL_PRELOAD &&
309+
reason != AFS_LOOKUP_CELL_ROOTCELL) {
310+
set_bit(AFS_CELL_FL_DO_LOOKUP, &cell->flags);
311+
afs_queue_cell(cell, afs_cell_trace_queue_new);
312+
}
301313

302314
wait_for_cell:
303-
_debug("wait_for_cell");
304315
state = smp_load_acquire(&cell->state); /* vs error */
305-
if (state != AFS_CELL_ACTIVE &&
306-
state != AFS_CELL_DEAD) {
316+
switch (state) {
317+
case AFS_CELL_ACTIVE:
318+
case AFS_CELL_DEAD:
319+
break;
320+
case AFS_CELL_UNLOOKED:
321+
default:
322+
if (reason == AFS_LOOKUP_CELL_PRELOAD ||
323+
reason == AFS_LOOKUP_CELL_ROOTCELL)
324+
break;
325+
_debug("wait_for_cell");
307326
afs_see_cell(cell, afs_cell_trace_wait);
308327
wait_var_event(&cell->state,
309328
({
310329
state = smp_load_acquire(&cell->state); /* vs error */
311330
state == AFS_CELL_ACTIVE || state == AFS_CELL_DEAD;
312331
}));
332+
_debug("waited_for_cell %d %d", cell->state, cell->error);
313333
}
314334

335+
no_wait:
315336
/* Check the state obtained from the wait check. */
337+
state = smp_load_acquire(&cell->state); /* vs error */
316338
if (state == AFS_CELL_DEAD) {
317339
ret = cell->error;
318340
goto error;
319341
}
342+
if (state == AFS_CELL_ACTIVE) {
343+
switch (cell->dns_status) {
344+
case DNS_LOOKUP_NOT_DONE:
345+
if (cell->dns_source == DNS_RECORD_FROM_CONFIG) {
346+
ret = 0;
347+
break;
348+
}
349+
fallthrough;
350+
default:
351+
ret = -EIO;
352+
goto error;
353+
case DNS_LOOKUP_GOOD:
354+
case DNS_LOOKUP_GOOD_WITH_BAD:
355+
ret = 0;
356+
break;
357+
case DNS_LOOKUP_GOT_NOT_FOUND:
358+
ret = -ENOENT;
359+
goto error;
360+
case DNS_LOOKUP_BAD:
361+
ret = -EREMOTEIO;
362+
goto error;
363+
case DNS_LOOKUP_GOT_LOCAL_FAILURE:
364+
case DNS_LOOKUP_GOT_TEMP_FAILURE:
365+
case DNS_LOOKUP_GOT_NS_FAILURE:
366+
ret = -EDESTADDRREQ;
367+
goto error;
368+
}
369+
}
320370

321371
_leave(" = %p [cell]", cell);
322372
return cell;
323373

324374
cell_already_exists:
325375
_debug("cell exists");
326376
cell = cursor;
327-
if (excl) {
377+
if (reason == AFS_LOOKUP_CELL_PRELOAD) {
328378
ret = -EEXIST;
329379
} else {
330380
afs_use_cell(cursor, trace);
@@ -383,7 +433,8 @@ int afs_cell_init(struct afs_net *net, const char *rootcell)
383433
return -EINVAL;
384434

385435
/* allocate a cell record for the root/workstation cell */
386-
new_root = afs_lookup_cell(net, rootcell, len, vllist, false,
436+
new_root = afs_lookup_cell(net, rootcell, len, vllist,
437+
AFS_LOOKUP_CELL_ROOTCELL,
387438
afs_cell_trace_use_lookup_ws);
388439
if (IS_ERR(new_root)) {
389440
_leave(" = %ld", PTR_ERR(new_root));
@@ -778,6 +829,7 @@ static bool afs_manage_cell(struct afs_cell *cell)
778829
switch (cell->state) {
779830
case AFS_CELL_SETTING_UP:
780831
goto set_up_cell;
832+
case AFS_CELL_UNLOOKED:
781833
case AFS_CELL_ACTIVE:
782834
goto cell_is_active;
783835
case AFS_CELL_REMOVING:
@@ -798,7 +850,7 @@ static bool afs_manage_cell(struct afs_cell *cell)
798850
goto remove_cell;
799851
}
800852

801-
afs_set_cell_state(cell, AFS_CELL_ACTIVE);
853+
afs_set_cell_state(cell, AFS_CELL_UNLOOKED);
802854

803855
cell_is_active:
804856
if (afs_has_cell_expired(cell, &next_manage))
@@ -808,6 +860,8 @@ static bool afs_manage_cell(struct afs_cell *cell)
808860
ret = afs_update_cell(cell);
809861
if (ret < 0)
810862
cell->error = ret;
863+
if (cell->state == AFS_CELL_UNLOOKED)
864+
afs_set_cell_state(cell, AFS_CELL_ACTIVE);
811865
}
812866

813867
if (next_manage < TIME64_MAX && cell->net->live) {

fs/afs/dynroot.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@ static struct dentry *afs_dynroot_lookup_cell(struct inode *dir, struct dentry *
108108
dotted = true;
109109
}
110110

111-
cell = afs_lookup_cell(net, name, len, NULL, false,
111+
cell = afs_lookup_cell(net, name, len, NULL,
112+
AFS_LOOKUP_CELL_DYNROOT,
112113
afs_cell_trace_use_lookup_dynroot);
113114
if (IS_ERR(cell)) {
114115
ret = PTR_ERR(cell);

fs/afs/internal.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,7 @@ extern const char afs_init_sysname[];
353353

354354
enum afs_cell_state {
355355
AFS_CELL_SETTING_UP,
356+
AFS_CELL_UNLOOKED,
356357
AFS_CELL_ACTIVE,
357358
AFS_CELL_REMOVING,
358359
AFS_CELL_DEAD,
@@ -1034,9 +1035,18 @@ static inline bool afs_cb_is_broken(unsigned int cb_break,
10341035
extern int afs_cell_init(struct afs_net *, const char *);
10351036
extern struct afs_cell *afs_find_cell(struct afs_net *, const char *, unsigned,
10361037
enum afs_cell_trace);
1038+
enum afs_lookup_cell_for {
1039+
AFS_LOOKUP_CELL_DYNROOT,
1040+
AFS_LOOKUP_CELL_MOUNTPOINT,
1041+
AFS_LOOKUP_CELL_DIRECT_MOUNT,
1042+
AFS_LOOKUP_CELL_PRELOAD,
1043+
AFS_LOOKUP_CELL_ROOTCELL,
1044+
AFS_LOOKUP_CELL_ALIAS_CHECK,
1045+
};
10371046
struct afs_cell *afs_lookup_cell(struct afs_net *net,
10381047
const char *name, unsigned int namesz,
1039-
const char *vllist, bool excl,
1048+
const char *vllist,
1049+
enum afs_lookup_cell_for reason,
10401050
enum afs_cell_trace trace);
10411051
extern struct afs_cell *afs_use_cell(struct afs_cell *, enum afs_cell_trace);
10421052
void afs_unuse_cell(struct afs_cell *cell, enum afs_cell_trace reason);

fs/afs/mntpt.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ static int afs_mntpt_set_params(struct fs_context *fc, struct dentry *mntpt)
107107
if (size > AFS_MAXCELLNAME)
108108
return -ENAMETOOLONG;
109109

110-
cell = afs_lookup_cell(ctx->net, p, size, NULL, false,
110+
cell = afs_lookup_cell(ctx->net, p, size, NULL,
111+
AFS_LOOKUP_CELL_MOUNTPOINT,
111112
afs_cell_trace_use_lookup_mntpt);
112113
if (IS_ERR(cell)) {
113114
pr_err("kAFS: unable to lookup cell '%pd'\n", mntpt);

fs/afs/proc.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,8 @@ static int afs_proc_cells_write(struct file *file, char *buf, size_t size)
122122
if (strcmp(buf, "add") == 0) {
123123
struct afs_cell *cell;
124124

125-
cell = afs_lookup_cell(net, name, strlen(name), args, true,
125+
cell = afs_lookup_cell(net, name, strlen(name), args,
126+
AFS_LOOKUP_CELL_PRELOAD,
126127
afs_cell_trace_use_lookup_add);
127128
if (IS_ERR(cell)) {
128129
ret = PTR_ERR(cell);

fs/afs/super.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ static int afs_parse_source(struct fs_context *fc, struct fs_parameter *param)
290290
/* lookup the cell record */
291291
if (cellname) {
292292
cell = afs_lookup_cell(ctx->net, cellname, cellnamesz,
293-
NULL, false,
293+
NULL, AFS_LOOKUP_CELL_DIRECT_MOUNT,
294294
afs_cell_trace_use_lookup_mount);
295295
if (IS_ERR(cell)) {
296296
pr_err("kAFS: unable to lookup cell '%*.*s'\n",

fs/afs/vl_alias.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,8 @@ static int yfs_check_canonical_cell_name(struct afs_cell *cell, struct key *key)
269269
if (!name_len || name_len > AFS_MAXCELLNAME)
270270
master = ERR_PTR(-EOPNOTSUPP);
271271
else
272-
master = afs_lookup_cell(cell->net, cell_name, name_len, NULL, false,
272+
master = afs_lookup_cell(cell->net, cell_name, name_len, NULL,
273+
AFS_LOOKUP_CELL_ALIAS_CHECK,
273274
afs_cell_trace_use_lookup_canonical);
274275
kfree(cell_name);
275276
if (IS_ERR(master))

0 commit comments

Comments
 (0)