afs: Avoid non-dir ENOENT errors in afs_lookup 37/13537/6
authorAndrew Deason <adeason@sinenomine.net>
Mon, 25 Mar 2019 21:33:39 +0000 (16:33 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Tue, 16 Jul 2019 16:01:29 +0000 (12:01 -0400)
Historically, there have been many subsystems in libafs that can
generate ENOENT errors for a variety of reasons. In addition to the
expected case where we lookup a name that doesn't exist, other
scenarios have caused ENOENT error codes to be generated, such as:
internal inconsistencies, I/O errors, or even abort codes from the
network.

When one of these scenarios cause an ENOENT error code in one of those
situations during afs_lookup() when the target name does actually
exist, it can be confusing to a user, or even result in incorrect
application behavior. On Linux in particular, ENOENT results from a
lookup are cached in negative dcache entries, and so can cause future
lookups for the same name to yield ENOENT errors.

Various commits have tried to avoid this abuse of the ENOENT error
code, such as 2aa4cb04 (afs: Stop abusing ENOENT). But we cannot
prevent receiving ENOENT abort codes from the network, and mistakes in
the future may cause more scenarios incorrectly yielding ENOENTs.

However, in afs_lookup, we do know that legitimate ENOENT errors can
only occur in one situation: when we have a valid directory blob, and the
afs_dir_Lookup() operation itself returns an ENOENT error for the
target name. For all other areas of afs_lookup(), we know that an
ENOENT error is not legitimate, since we may not be sure if the target
name exists or not.

So to proactively avoid incorrect ENOENT results, prevent afs_lookup
from returning ENOENT, except in the specific code path where
afs_dir_Lookup is called.

Change-Id: I1c91600fd38b1179f02fa6eadea631b6eb8edb6d
Reviewed-on: https://gerrit.openafs.org/13537
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

src/afs/VNOPS/afs_vnop_lookup.c

index 8146957..2ca0969 100644 (file)
@@ -1388,6 +1388,11 @@ afs_lookup(OSI_VC_DECL(adp), char *aname, struct vcache **avcp, afs_ucred_t *acr
     int dynrootRetry = 1;
     struct afs_fakestat_state fakestate;
     int tryEvalOnly = 0;
+
+    /* Don't allow ENOENT errors, except for a specific code path where
+     * 'enoent_prohibited' is cleared below. */
+    int enoent_prohibited = 1;
+
     OSI_VC_CONVERT(adp);
 
     AFS_STATCNT(afs_lookup);
@@ -1727,8 +1732,10 @@ afs_lookup(OSI_VC_DECL(adp), char *aname, struct vcache **avcp, afs_ucred_t *acr
                   ICL_TYPE_INT32, code);
 
        if (code) {
-           if (code != ENOENT) {
-               /*printf("LOOKUP dirLookupOff -> %d\n", code);*/
+           if (code == ENOENT) {
+               /* The target name really doesn't exist (according to
+                * afs_dir_LookupOffset, anyway). */
+               enoent_prohibited = 0;
            }
            goto done;
        }
@@ -1948,6 +1955,16 @@ afs_lookup(OSI_VC_DECL(adp), char *aname, struct vcache **avcp, afs_ucred_t *acr
         */
        *avcp = NULL;
     }
+    if (code == ENOENT && enoent_prohibited) {
+       /*
+        * We got an ENOENT error, but we didn't get it while looking up the
+        * dir entry in the relevant dir blob. That means we likely hit some
+        * other internal error; don't allow us to return ENOENT in this case,
+        * since some platforms cache ENOENT errors, and the target path name
+        * may actually exist.
+        */
+       code = EIO;
+    }
 
     afs_PutFakeStat(&fakestate);
     afs_DestroyReq(treq);