afs: Avoid panics on failed return from afs_CFileOpen 41/14241/8
authorCheyenne Wills <cwills@sinenomine.net>
Fri, 19 Jun 2020 14:01:14 +0000 (08:01 -0600)
committerBenjamin Kaduk <kaduk@mit.edu>
Fri, 3 Jul 2020 15:50:42 +0000 (11:50 -0400)
afs_CFileOpen is a macro that invokes the open "method" of the
afs_cacheOps structure, and for disk caches the osi_UFSOpen function is
used.

Currently osi_UFSOpen will panic if there is an error encountered while
opening a file.

Prepare to handle osi_UFSOpen function returning a NULL instead of
issuing a panic (future commit).

Update callers of afs_CFileOpen to test for an error and to return an
error instead of issuing a panic.

While this commit eliminates some panics, it does not address some of the
more complex cases associated with errors from afs_CFileOpen.

Change-Id: I2bdd525633dd44ebf8e26fcfd7059dfdfffb6142
Reviewed-on: https://gerrit.openafs.org/14241
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

src/afs/VNOPS/afs_vnop_symlink.c
src/afs/afs_buffer.c
src/afs/afs_dcache.c
src/afs/afs_disconnected.c
src/afs/afs_fetchstore.c
src/afs/afs_init.c
src/afs/afs_segments.c

index 02ba3b4..e88e09e 100644 (file)
@@ -43,6 +43,7 @@ afs_DisconCreateSymlink(struct vcache *avc, char *aname,
     struct dcache *tdc;
     struct osi_file *tfile;
     afs_size_t offset, len;
+    int code = 0;
 
     tdc = afs_GetDCache(avc, 0, areq, &offset, &len, 0);
     if (!tdc) {
@@ -54,14 +55,18 @@ afs_DisconCreateSymlink(struct vcache *avc, char *aname,
     avc->f.m.Length = len;
 
     ObtainWriteLock(&tdc->lock, 720);
+    tfile = afs_CFileOpen(&tdc->f.inode);
+    if (!tfile) {
+       code = EIO;
+       goto done;
+    }
     afs_AdjustSize(tdc, len);
     tdc->validPos = len;
-    tfile = afs_CFileOpen(&tdc->f.inode);
-    osi_Assert(tfile);
     afs_CFileWrite(tfile, 0, aname, len);
     afs_CFileClose(tfile);
+ done:
     ReleaseWriteLock(&tdc->lock);
-    return 0;
+    return code;
 }
 
 /* don't set CDirty in here because RPC is called synchronously */
index 8bccee4..1f2a470 100644 (file)
@@ -236,28 +236,24 @@ DRead(struct dcache *adc, int page, struct DirBuffer *entry)
         /* The directory blob is empty, apparently. This is not a valid dir
          * blob, so throw an error. */
         code = EIO;
+       goto error;
     } else if (page * AFS_BUFFER_PAGESIZE >= adc->f.chunkBytes) {
         code = ENOENT; /* past the end */
+       goto error;
     }
-    if (code) {
-       tb->fid = NULLIDX;
-       afs_reset_inode(&tb->inode);
-       tb->lockers--;
-       ReleaseWriteLock(&tb->lock);
-       return code;
-    }
+
     tfile = afs_CFileOpen(&adc->f.inode);
-    osi_Assert(tfile);
+    if (!tfile) {
+       code = EIO;
+       goto error;
+    }
     code =
        afs_CFileRead(tfile, tb->page * AFS_BUFFER_PAGESIZE, tb->data,
                      AFS_BUFFER_PAGESIZE);
     afs_CFileClose(tfile);
     if (code < AFS_BUFFER_PAGESIZE) {
-       tb->fid = NULLIDX;
-       afs_reset_inode(&tb->inode);
-       tb->lockers--;
-       ReleaseWriteLock(&tb->lock);
-       return EIO;
+       code = EIO;
+       goto error;
     }
     /* Note that findslot sets the page field in the buffer equal to
      * what it is searching for. */
@@ -265,6 +261,13 @@ DRead(struct dcache *adc, int page, struct DirBuffer *entry)
     entry->buffer = tb;
     entry->data = tb->data;
     return 0;
+
+ error:
+    tb->fid = NULLIDX;
+    afs_reset_inode(&tb->inode);
+    tb->lockers--;
+    ReleaseWriteLock(&tb->lock);
+    return code;
 }
 
 static void
@@ -381,7 +384,9 @@ afs_newslot(struct dcache *adc, afs_int32 apage, struct buffer *lp)
     if (lp->dirty) {
        /* see DFlush for rationale for not getting and locking the dcache */
         tfile = afs_CFileOpen(&lp->inode);
-        osi_Assert(tfile);
+       if (!tfile)
+           return NULL;    /* Callers will flag as EIO */
+
        afs_CFileWrite(tfile, lp->page * AFS_BUFFER_PAGESIZE, lp->data,
                       AFS_BUFFER_PAGESIZE);
        lp->dirty = 0;
index 8d97143..d39350c 100644 (file)
@@ -3810,9 +3810,9 @@ afs_MakeShadowDir(struct vcache *avc, struct dcache *adc)
 {
     int i, code, ret_code = 0, written, trans_size;
     struct dcache *new_dc = NULL;
-    struct osi_file *tfile_src, *tfile_dst;
+    struct osi_file *tfile_src = NULL, *tfile_dst = NULL;
     struct VenusFid shadow_fid;
-    char *data;
+    char *data = NULL;
 
     /* Is this a dir? */
     if (vType(avc) != VDIR)
@@ -3867,9 +3867,16 @@ afs_MakeShadowDir(struct vcache *avc, struct dcache *adc)
 
     /* Open the files. */
     tfile_src = afs_CFileOpen(&adc->f.inode);
+    if (!tfile_src) {
+       ret_code = EIO;
+       goto done;
+    }
+
     tfile_dst = afs_CFileOpen(&new_dc->f.inode);
-    osi_Assert(tfile_src);
-    osi_Assert(tfile_dst);
+    if (!tfile_dst) {
+       ret_code = EIO;
+       goto done;
+    }
 
     /* And now copy dir dcache data into this dcache,
      * 4k at a time.
@@ -3897,10 +3904,14 @@ afs_MakeShadowDir(struct vcache *avc, struct dcache *adc)
        written+=trans_size;
     }
 
-    afs_CFileClose(tfile_dst);
-    afs_CFileClose(tfile_src);
+ done:
+    if (tfile_dst)
+       afs_CFileClose(tfile_dst);
+    if (tfile_src)
+       afs_CFileClose(tfile_src);
 
-    afs_osi_Free(data, 4096);
+    if (data)
+       afs_osi_Free(data, 4096);
 
     ReleaseWriteLock(&new_dc->lock);
     afs_PutDCache(new_dc);
@@ -3917,7 +3928,6 @@ afs_MakeShadowDir(struct vcache *avc, struct dcache *adc)
        avc->f.shadow.unique = shadow_fid.Fid.Unique;
     }
 
-done:
     return ret_code;
 }
 
index ee79d31..3b88585 100644 (file)
@@ -709,7 +709,12 @@ afs_ProcessOpCreate(struct vcache *avc, struct vrequest *areq,
        }
        ObtainReadLock(&tdc->lock);
        tfile = afs_CFileOpen(&tdc->f.inode);
-        osi_Assert(tfile);
+       if (!tfile) {
+           ReleaseReadLock(&tdc->lock);
+           afs_PutDCache(tdc);
+           code = EIO;
+           goto end;
+       }
        code = afs_CFileRead(tfile, 0, ttargetName, tlen);
        ttargetName[tlen-1] = '\0';
        afs_CFileClose(tfile);
index 06325b9..e2b3f2b 100644 (file)
@@ -270,7 +270,9 @@ afs_GenericStoreProc(struct storeOps *ops, void *rock,
     size = tdc->f.chunkBytes;
 
     tfile = afs_CFileOpen(&tdc->f.inode);
-    osi_Assert(tfile);
+    if (!tfile) {
+       return EIO;
+    }
 
     while ( size > 0 ) {
        code = (*ops->prepare)(rock, size, &tlen);
index f2414f7..5d72319 100644 (file)
@@ -334,7 +334,9 @@ afs_InitVolumeInfo(char *afile)
     if (code)
        return code;
     tfile = afs_CFileOpen(&volumeInode);
-    osi_Assert(tfile);
+    if (!tfile) {
+       return EIO;
+    }
     afs_CFileTruncate(tfile, 0);
     afs_CFileClose(tfile);
     return 0;
index b1de015..91e6f28 100644 (file)
@@ -727,7 +727,12 @@ afs_ExtendSegments(struct vcache *avc, afs_size_t alen, struct vrequest *areq)
            toAdd = AFS_CHUNKTOSIZE(tdc->f.chunk) - offset;
        }
         tfile = afs_CFileOpen(&tdc->f.inode);
-        osi_Assert(tfile);
+       if (!tfile) {
+           ReleaseWriteLock(&tdc->lock);
+           afs_PutDCache(tdc);
+           code = EIO;
+           break;
+       }
        while(tdc->validPos < avc->f.m.Length + toAdd) {
             afs_size_t towrite;