From: Andrew Deason Date: Thu, 17 Jan 2019 06:04:36 +0000 (-0600) Subject: dir: Honor non-ENOENT lookup errors X-Git-Tag: openafs-devel-1_9_0~316 X-Git-Url: http://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=refs%2Fchanges%2F31%2F13431%2F4 dir: Honor non-ENOENT lookup errors 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 Reviewed-by: Michael Meffie Reviewed-by: Cheyenne Wills Reviewed-by: Benjamin Kaduk --- diff --git a/src/dir/dir.c b/src/dir/dir.c index 8f388dc..8aa336d 100644 --- a/src/dir/dir.c +++ b/src/dir/dir.c @@ -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); diff --git a/src/viced/afsfileprocs.c b/src/viced/afsfileprocs.c index 8f214c0..9dcdd29 100644 --- a/src/viced/afsfileprocs.c +++ b/src/viced/afsfileprocs.c @@ -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;