dir: Honor non-ENOENT lookup errors 31/13431/4
authorAndrew Deason <adeason@sinenomine.net>
Thu, 17 Jan 2019 06:04:36 +0000 (00:04 -0600)
committerBenjamin Kaduk <kaduk@mit.edu>
Sat, 6 Apr 2019 17:26:09 +0000 (13:26 -0400)
Currently, several places in src/dir/dir.c assume that any error from
a lower-level function (e.g. FindItem) means that the item we're
looking for does not exist in that directory. But if we encountered
some other error, that may not be the case; the directory blob may be
corrupt, we may have encountered some I/O error, etc.

To detect cases like this, return the actual error code from FindItem
&c, instead of always reporting ENOENT. For the code paths that are
actually specifically looking for if the target exists (in
afs_dir_Create), change our checks to specifically check for ENOENT,
and return any other error.

Do the same thing for a few similar callers in viced/afsfileprocs.c,
as well.

FIXES 134904

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

src/dir/dir.c
src/viced/afsfileprocs.c

index 8f388dc..8aa336d 100644 (file)
@@ -99,13 +99,18 @@ afs_dir_Create(dir_file_t dir, char *entry, void *voidfid)
     struct DirBuffer entrybuf, prevbuf, headerbuf;
     struct DirEntry *ep;
     struct DirHeader *dhp;
+    int code;
 
     /* check name quality */
     if (*entry == 0)
        return EINVAL;
 
     /* First check if file already exists. */
-    if (FindItem(dir, entry, &prevbuf, &entrybuf) == 0) {
+    code = FindItem(dir, entry, &prevbuf, &entrybuf);
+    if (code && code != ENOENT) {
+        return code;
+    }
+    if (code == 0) {
        DRelease(&entrybuf, 0);
        DRelease(&prevbuf, 0);
        return EEXIST;
@@ -175,9 +180,12 @@ afs_dir_Delete(dir_file_t dir, char *entry)
     struct DirBuffer entrybuf, prevbuf;
     struct DirEntry *firstitem;
     unsigned short *previtem;
+    int code;
 
-    if (FindItem(dir, entry, &prevbuf, &entrybuf) != 0)
-       return ENOENT;
+    code = FindItem(dir, entry, &prevbuf, &entrybuf);
+    if (code) {
+        return code;
+    }
 
     firstitem = (struct DirEntry *)entrybuf.data;
     previtem = (unsigned short *)prevbuf.data;
@@ -361,9 +369,12 @@ afs_dir_Lookup(dir_file_t dir, char *entry, void *voidfid)
     afs_int32 *fid = (afs_int32 *) voidfid;
     struct DirBuffer firstbuf, prevbuf;
     struct DirEntry *firstitem;
+    int code;
 
-    if (FindItem(dir, entry, &prevbuf, &firstbuf) != 0)
-       return ENOENT;
+    code = FindItem(dir, entry, &prevbuf, &firstbuf);
+    if (code) {
+        return code;
+    }
     DRelease(&prevbuf, 0);
     firstitem = (struct DirEntry *)firstbuf.data;
 
@@ -382,9 +393,12 @@ afs_dir_LookupOffset(dir_file_t dir, char *entry, void *voidfid,
     afs_int32 *fid = (afs_int32 *) voidfid;
     struct DirBuffer firstbuf, prevbuf;
     struct DirEntry *firstitem;
+    int code;
 
-    if (FindItem(dir, entry, &prevbuf, &firstbuf) != 0)
-       return ENOENT;
+    code = FindItem(dir, entry, &prevbuf, &firstbuf);
+    if (code) {
+        return code;
+    }
     DRelease(&prevbuf, 0);
     firstitem = (struct DirEntry *)firstbuf.data;
 
@@ -607,16 +621,15 @@ FindItem(dir_file_t dir, char *ename, struct DirBuffer *prevbuf,
     i = afs_dir_DirHash(ename);
     if (dhp->hashTable[i] == 0) {
        /* no such entry */
-       DRelease(&prev, 0);
-       return ENOENT;
+       code = ENOENT;
+       goto out;
     }
 
     code = afs_dir_GetVerifiedBlob(dir,
                                   (u_short) ntohs(dhp->hashTable[i]),
                                   &curr);
     if (code) {
-       DRelease(&prev, 0);
-       return code;
+       goto out;
     }
 
     prev.data = &(dhp->hashTable[i]);
@@ -639,8 +652,11 @@ FindItem(dir_file_t dir, char *ename, struct DirBuffer *prevbuf,
        prev = curr;
        prev.data = &(tp->next);
 
-       if (tp->next == 0)
-           goto out; /* The end of the line */
+       if (tp->next == 0) {
+           /* The end of the line */
+           code = ENOENT;
+           goto out;
+       }
 
        code = afs_dir_GetVerifiedBlob(dir, (u_short) ntohs(tp->next),
                                       &curr);
@@ -648,9 +664,13 @@ FindItem(dir_file_t dir, char *ename, struct DirBuffer *prevbuf,
            goto out;
     }
 
+    /* If we've reached here, we've hit our loop limit. Something is weird with
+     * the directory; maybe a circular hash chain? */
+    code = EIO;
+
 out:
     DRelease(&prev, 0);
-    return ENOENT;
+    return code;
 }
 
 static int
@@ -725,8 +745,10 @@ afs_dir_InverseLookup(void *dir, afs_uint32 vnode, afs_uint32 unique,
     struct DirEntry *entry;
     int code = 0;
 
-    if (FindFid(dir, vnode, unique, &entrybuf) != 0)
-       return ENOENT;
+    code = FindFid(dir, vnode, unique, &entrybuf);
+    if (code) {
+        return code;
+    }
     entry = (struct DirEntry *)entrybuf.data;
 
     if (strlen(entry->name) >= length)
@@ -754,10 +776,13 @@ afs_dir_ChangeFid(dir_file_t dir, char *entry, afs_uint32 *old_fid,
     struct DirEntry *firstitem;
     struct MKFid *fid_old = (struct MKFid *) old_fid;
     struct MKFid *fid_new = (struct MKFid *) new_fid;
+    int code;
 
     /* Find entry. */
-    if (FindItem(dir, entry, &prevbuf, &entrybuf) != 0)
-       return ENOENT;
+    code = FindItem(dir, entry, &prevbuf, &entrybuf);
+    if (code) {
+        return code;
+    }
     firstitem = (struct DirEntry *)entrybuf.data;
     DRelease(&prevbuf, 1);
 
index 8f214c0..9dcdd29 100644 (file)
@@ -1483,8 +1483,13 @@ DeleteTarget(Vnode * parentptr, Volume * volptr, Vnode ** targetptr,
 
     /* check that the file is in the directory */
     SetDirHandle(dir, parentptr);
-    if (afs_dir_Lookup(dir, Name, fileFid))
-       return (ENOENT);
+    errorCode = afs_dir_Lookup(dir, Name, fileFid);
+    if (errorCode && errorCode != ENOENT) {
+        errorCode = EIO;
+    }
+    if (errorCode) {
+       return errorCode;
+    }
     fileFid->Volume = V_id(volptr);
 
     /* just-in-case check for something causing deadlock */
@@ -3655,8 +3660,11 @@ SAFSS_Rename(struct rx_call *acall, struct AFSFid *OldDirFid, char *OldName,
     SetDirHandle(&newdir, newvptr);
 
     /* Lookup the file to delete its vnode */
-    if (afs_dir_Lookup(&olddir, OldName, &fileFid)) {
-       errorCode = ENOENT;
+    errorCode = afs_dir_Lookup(&olddir, OldName, &fileFid);
+    if (errorCode && errorCode != ENOENT) {
+        errorCode = EIO;
+    }
+    if (errorCode) {
        goto Bad_Rename;
     }
     if (fileFid.Vnode == oldvptr->vnodeNumber
@@ -3695,7 +3703,12 @@ SAFSS_Rename(struct rx_call *acall, struct AFSFid *OldDirFid, char *OldName,
     }
 
     /* Lookup the new file  */
-    if (!(afs_dir_Lookup(&newdir, NewName, &newFileFid))) {
+    code = afs_dir_Lookup(&newdir, NewName, &newFileFid);
+    if (code && code != ENOENT) {
+        errorCode = EIO;
+        goto Bad_Rename;
+    }
+    if (!code) {
        if (readonlyServer) {
            errorCode = VREADONLY;
            goto Bad_Rename;
@@ -3804,6 +3817,10 @@ SAFSS_Rename(struct rx_call *acall, struct AFSFid *OldDirFid, char *OldName,
            struct AFSFid unused;
 
            code = afs_dir_Lookup(&filedir, "..", &unused);
+            if (code && code != ENOENT) {
+                errorCode = EIO;
+                goto Bad_Rename;
+            }
            if (code == ENOENT) {
                /* only update .. if it doesn't already exist */
                updatefile = 1;