afs: Fix dynamic lookup to fail on cell lookup failure

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: 1d0b929fc070 ("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>

authored by

David Howells and committed by

Christian Brauner 7 months ago 330e2c51 2c2b67af

+86 -18

7 changed files

expand all

afs

cell.c

dynroot.c

internal.h

mntpt.c

proc.c

super.c

vl_alias.c

+66 -12

fs/afs/cell.c

··· 229 229 * @name: The name of the cell. 230 230 * @namesz: The strlen of the cell name. 231 231 * @vllist: A colon/comma separated list of numeric IP addresses or NULL. 232 - * @excl: T if an error should be given if the cell name already exists. 232 + * @reason: The reason we're doing the lookup 233 233 * @trace: The reason to be logged if the lookup is successful. 234 234 * 235 235 * Look up a cell record by name and query the DNS for VL server addresses if ··· 239 239 */ 240 240 struct afs_cell *afs_lookup_cell(struct afs_net *net, 241 241 const char *name, unsigned int namesz, 242 - const char *vllist, bool excl, 242 + const char *vllist, 243 + enum afs_lookup_cell_for reason, 243 244 enum afs_cell_trace trace) 244 245 { 245 246 struct afs_cell *cell, *candidate, *cursor; ··· 248 247 enum afs_cell_state state; 249 248 int ret, n; 250 249 251 - _enter("%s,%s", name, vllist); 250 + _enter("%s,%s,%u", name, vllist, reason); 252 251 253 - if (!excl) { 252 + if (reason != AFS_LOOKUP_CELL_PRELOAD) { 254 253 cell = afs_find_cell(net, name, namesz, trace); 255 - 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; 256 260 goto wait_for_cell; 261 + } 257 262 } 258 263 259 264 /* Assume we're probably going to create a cell and preallocate and ··· 305 298 rb_insert_color(&cell->net_node, &net->cells); 306 299 up_write(&net->cells_lock); 307 300 308 - afs_queue_cell(cell, afs_cell_trace_queue_new); 301 + lookup_cell: 302 + if (reason != AFS_LOOKUP_CELL_PRELOAD && 303 + reason != AFS_LOOKUP_CELL_ROOTCELL) { 304 + set_bit(AFS_CELL_FL_DO_LOOKUP, &cell->flags); 305 + afs_queue_cell(cell, afs_cell_trace_queue_new); 306 + } 309 307 310 308 wait_for_cell: 311 - _debug("wait_for_cell"); 312 309 state = smp_load_acquire(&cell->state); /* vs error */ 313 - if (state != AFS_CELL_ACTIVE && 314 - state != AFS_CELL_DEAD) { 310 + switch (state) { 311 + case AFS_CELL_ACTIVE: 312 + case AFS_CELL_DEAD: 313 + break; 314 + case AFS_CELL_UNLOOKED: 315 + default: 316 + if (reason == AFS_LOOKUP_CELL_PRELOAD || 317 + reason == AFS_LOOKUP_CELL_ROOTCELL) 318 + break; 319 + _debug("wait_for_cell"); 315 320 afs_see_cell(cell, afs_cell_trace_wait); 316 321 wait_var_event(&cell->state, 317 322 ({ 318 323 state = smp_load_acquire(&cell->state); /* vs error */ 319 324 state == AFS_CELL_ACTIVE || state == AFS_CELL_DEAD; 320 325 })); 326 + _debug("waited_for_cell %d %d", cell->state, cell->error); 321 327 } 322 328 329 + no_wait: 323 330 /* Check the state obtained from the wait check. */ 331 + state = smp_load_acquire(&cell->state); /* vs error */ 324 332 if (state == AFS_CELL_DEAD) { 325 333 ret = cell->error; 326 334 goto error; 335 + } 336 + if (state == AFS_CELL_ACTIVE) { 337 + switch (cell->dns_status) { 338 + case DNS_LOOKUP_NOT_DONE: 339 + if (cell->dns_source == DNS_RECORD_FROM_CONFIG) { 340 + ret = 0; 341 + break; 342 + } 343 + fallthrough; 344 + default: 345 + ret = -EIO; 346 + goto error; 347 + case DNS_LOOKUP_GOOD: 348 + case DNS_LOOKUP_GOOD_WITH_BAD: 349 + ret = 0; 350 + break; 351 + case DNS_LOOKUP_GOT_NOT_FOUND: 352 + ret = -ENOENT; 353 + goto error; 354 + case DNS_LOOKUP_BAD: 355 + ret = -EREMOTEIO; 356 + goto error; 357 + case DNS_LOOKUP_GOT_LOCAL_FAILURE: 358 + case DNS_LOOKUP_GOT_TEMP_FAILURE: 359 + case DNS_LOOKUP_GOT_NS_FAILURE: 360 + ret = -EDESTADDRREQ; 361 + goto error; 362 + } 327 363 } 328 364 329 365 _leave(" = %p [cell]", cell); ··· 375 325 cell_already_exists: 376 326 _debug("cell exists"); 377 327 cell = cursor; 378 - if (excl) { 328 + if (reason == AFS_LOOKUP_CELL_PRELOAD) { 379 329 ret = -EEXIST; 380 330 } else { 381 331 afs_use_cell(cursor, trace); ··· 434 384 return -EINVAL; 435 385 436 386 /* allocate a cell record for the root/workstation cell */ 437 - new_root = afs_lookup_cell(net, rootcell, len, vllist, false, 387 + new_root = afs_lookup_cell(net, rootcell, len, vllist, 388 + AFS_LOOKUP_CELL_ROOTCELL, 438 389 afs_cell_trace_use_lookup_ws); 439 390 if (IS_ERR(new_root)) { 440 391 _leave(" = %ld", PTR_ERR(new_root)); ··· 828 777 switch (cell->state) { 829 778 case AFS_CELL_SETTING_UP: 830 779 goto set_up_cell; 780 + case AFS_CELL_UNLOOKED: 831 781 case AFS_CELL_ACTIVE: 832 782 goto cell_is_active; 833 783 case AFS_CELL_REMOVING: ··· 849 797 goto remove_cell; 850 798 } 851 799 852 - afs_set_cell_state(cell, AFS_CELL_ACTIVE); 800 + afs_set_cell_state(cell, AFS_CELL_UNLOOKED); 853 801 854 802 cell_is_active: 855 803 if (afs_has_cell_expired(cell, &next_manage)) ··· 859 807 ret = afs_update_cell(cell); 860 808 if (ret < 0) 861 809 cell->error = ret; 810 + if (cell->state == AFS_CELL_UNLOOKED) 811 + afs_set_cell_state(cell, AFS_CELL_ACTIVE); 862 812 } 863 813 864 814 if (next_manage < TIME64_MAX && cell->net->live) {

+2 -1

fs/afs/dynroot.c

··· 108 108 dotted = true; 109 109 } 110 110 111 - cell = afs_lookup_cell(net, name, len, NULL, false, 111 + cell = afs_lookup_cell(net, name, len, NULL, 112 + AFS_LOOKUP_CELL_DYNROOT, 112 113 afs_cell_trace_use_lookup_dynroot); 113 114 if (IS_ERR(cell)) { 114 115 ret = PTR_ERR(cell);

+11 -1

fs/afs/internal.h

··· 343 343 344 344 enum afs_cell_state { 345 345 AFS_CELL_SETTING_UP, 346 + AFS_CELL_UNLOOKED, 346 347 AFS_CELL_ACTIVE, 347 348 AFS_CELL_REMOVING, 348 349 AFS_CELL_DEAD, ··· 1050 1049 extern int afs_cell_init(struct afs_net *, const char *); 1051 1050 extern struct afs_cell *afs_find_cell(struct afs_net *, const char *, unsigned, 1052 1051 enum afs_cell_trace); 1052 + enum afs_lookup_cell_for { 1053 + AFS_LOOKUP_CELL_DYNROOT, 1054 + AFS_LOOKUP_CELL_MOUNTPOINT, 1055 + AFS_LOOKUP_CELL_DIRECT_MOUNT, 1056 + AFS_LOOKUP_CELL_PRELOAD, 1057 + AFS_LOOKUP_CELL_ROOTCELL, 1058 + AFS_LOOKUP_CELL_ALIAS_CHECK, 1059 + }; 1053 1060 struct afs_cell *afs_lookup_cell(struct afs_net *net, 1054 1061 const char *name, unsigned int namesz, 1055 - const char *vllist, bool excl, 1062 + const char *vllist, 1063 + enum afs_lookup_cell_for reason, 1056 1064 enum afs_cell_trace trace); 1057 1065 extern struct afs_cell *afs_use_cell(struct afs_cell *, enum afs_cell_trace); 1058 1066 void afs_unuse_cell(struct afs_cell *cell, enum afs_cell_trace reason);

+2 -1

fs/afs/mntpt.c

··· 107 107 if (size > AFS_MAXCELLNAME) 108 108 return -ENAMETOOLONG; 109 109 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, 111 112 afs_cell_trace_use_lookup_mntpt); 112 113 if (IS_ERR(cell)) { 113 114 pr_err("kAFS: unable to lookup cell '%pd'\n", mntpt);

+2 -1

fs/afs/proc.c

··· 122 122 if (strcmp(buf, "add") == 0) { 123 123 struct afs_cell *cell; 124 124 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, 126 127 afs_cell_trace_use_lookup_add); 127 128 if (IS_ERR(cell)) { 128 129 ret = PTR_ERR(cell);

+1 -1

fs/afs/super.c

··· 290 290 /* lookup the cell record */ 291 291 if (cellname) { 292 292 cell = afs_lookup_cell(ctx->net, cellname, cellnamesz, 293 - NULL, false, 293 + NULL, AFS_LOOKUP_CELL_DIRECT_MOUNT, 294 294 afs_cell_trace_use_lookup_mount); 295 295 if (IS_ERR(cell)) { 296 296 pr_err("kAFS: unable to lookup cell '%*.*s'\n",

+2 -1

fs/afs/vl_alias.c

··· 269 269 if (!name_len || name_len > AFS_MAXCELLNAME) 270 270 master = ERR_PTR(-EOPNOTSUPP); 271 271 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, 273 274 afs_cell_trace_use_lookup_canonical); 274 275 kfree(cell_name); 275 276 if (IS_ERR(master))

Configure Feed

Configure Feed