salvager: Trust inode-based special data over OGM

Currently the salvaging code looks for special inodes, and infers the
volume id and inode type from the OGM data in each special inode file.
However, we can already derive this information from the inode number
itself for the special inode, so if they disagree, use the values
based off of the inode number and correct the OGM data.

The inode number should be more likely to be correct, since that is
how we look up the special inode from the header when attaching the
volume. It is also impossible to get special inode files with the same
name, so this ensures we don't get duplicates. And for people that go
snooping around /vicepX/AFSIDat even though we tell them not to, it
seems more likely that they go around 'chmod'ing or 'chown'ing rather
than 'mv'ing.

This change avoids an abort in the salvaging code when the OGM data is
wrong. If we trust the OGM data when it is incorrect, we assume the
special inode file is for a different volume. So when we go to
recreate one of the special files for the volume we're actually
working with, the IH_CREATE fails (from EEXIST) and so we abort.

Reviewed-on: http://gerrit.openafs.org/6946
Tested-by: Derrick Brashear <shadow@dementix.org>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
(cherry picked from commit df522b588396aaac3ec662b516ef7287f3b7c47b)

Change-Id: I7ae826b6d6747e3ab3a1d8086ff17503a904a6d5
Reviewed-on: http://gerrit.openafs.org/9282
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>

diff --git a/src/vol/namei_ops.c b/src/vol/namei_ops.c
index 3b7dd9597dc2cb5a0c47aef1462c6805727059ec..3c7469814b2c3f9ed893d732bf9b1338738f61f1 100644 (file)
--- a/src/vol/namei_ops.c
+++ b/src/vol/namei_ops.c
@@ -714,6 +714,31 @@ CheckOGM(FdHandle_t *fdP, int p1)
 
     return 0;
 }
+
+static int
+FixSpecialOGM(FdHandle_t *fdP, int check)
+{
+    Inode ino = fdP->fd_ih->ih_ino;
+    VnodeId vno = NAMEI_VNODESPECIAL, ogm_vno;
+    int ogm_volid;
+
+    if (GetWinOGM(fdP->fd_fd, &ogm_volid, &ogm_vno)) {
+       return -1;
+    }
+
+    /* the only thing we can check is the vnode number; for the volid we have
+     * nothing else to compare against */
+    if (vno != ogm_vno) {
+       if (check) {
+           return -1;
+       }
+       if (SetWinOGM(fdP->fd_fd, ogm_volid, vno)) {
+           return -1;
+       }
+    }
+    return 0;
+}
+
 #else /* AFS_NT40_ENV */
 /* SetOGM - set owner group and mode bits from parm and tag */
 static int
@@ -769,8 +794,68 @@ CheckOGM(FdHandle_t *fdP, int p1)
 
     return 0;
 }
+
+static int
+FixSpecialOGM(FdHandle_t *fdP, int check)
+{
+    int inode_volid, ogm_volid;
+    int inode_type, ogm_type;
+    Inode ino = fdP->fd_ih->ih_ino;
+
+    inode_volid = ((ino >> NAMEI_UNIQSHIFT) & NAMEI_UNIQMASK);
+    inode_type = (int)((ino >> NAMEI_TAGSHIFT) & NAMEI_TAGMASK);
+
+    if (GetOGM(fdP, &ogm_volid, &ogm_type) < 0) {
+       Log("Error retrieving OGM info\n");
+       return -1;
+    }
+
+    if (inode_volid != ogm_volid || inode_type != ogm_type) {
+       Log("%sIncorrect OGM data (ino: vol %u type %d) (ogm: vol %u type %d)\n",
+           check?"":"Fixing ", inode_volid, inode_type, ogm_volid, ogm_type);
+
+       if (check) {
+           return -1;
+       }
+
+       if (SetOGM(fdP->fd_fd, inode_volid, inode_type) < 0) {
+           Log("Error setting OGM data\n");
+           return -1;
+       }
+    }
+    return 0;
+}
+
 #endif /* !AFS_NT40_ENV */
 
+/**
+ * Check/fix the OGM data for an inode
+ *
+ * @param[in] fdP   Open file handle for the inode to check
+ * @param[in] check 1 to just check the OGM data, and return an error if it
+ *                  is incorrect. 0 to fix the OGM data if it is incorrect.
+ *
+ * @pre fdP must be for a special inode
+ *
+ * @return status
+ *  @retval 0 success
+ *  @retval -1 error
+ */
+int
+namei_FixSpecialOGM(FdHandle_t *fdP, int check)
+{
+    int vnode;
+    Inode ino = fdP->fd_ih->ih_ino;
+
+    vnode = (int)(ino & NAMEI_VNODEMASK);
+    if (vnode != NAMEI_VNODESPECIAL) {
+       Log("FixSpecialOGM: non-special vnode %u\n", vnode);
+       return -1;
+    }
+
+    return FixSpecialOGM(fdP, check);
+}
+
 int big_vno = 0;               /* Just in case we ever do 64 bit vnodes. */
 
 /* Derive the name and create it O_EXCL. If that fails we have an error.
@@ -1904,30 +1989,21 @@ _namei_examine_special(char * path1,
 {
     int ret = 0;
     struct ViceInodeInfo info;
-    afs_uint32 inode_vgid;
 
     if (DecodeInode(path1, dname, &info, myIH->ih_vid) < 0) {
        ret = 0;
        goto error;
     }
 
-#ifdef AFS_NT40_ENV
-    inode_vgid = myIH->ih_vid;
-#else
-    inode_vgid = (info.inodeNumber >> NAMEI_UNIQSHIFT) & NAMEI_UNIQMASK;
-#endif
-
     if (info.u.param[2] != VI_LINKTABLE) {
        info.linkCount = 1;
-    } else if ((info.u.param[0] != myIH->ih_vid) ||
-              (inode_vgid != myIH->ih_vid)) {
+    } else if (info.u.param[0] != myIH->ih_vid) {
        /* VGID encoded in linktable filename and/or OGM data isn't
         * consistent with VGID encoded in namei path */
        Log("namei_ListAFSSubDirs: warning: inconsistent linktable "
            "filename \"%s" OS_DIRSEP "%s\"; salvager will delete it "
-           "(dir_vgid=%u, inode_vgid=%u, ogm_vgid=%u)\n",
+           "(dir_vgid=%u, inode_vgid=%u)\n",
            path1, dname, myIH->ih_vid,
-           (unsigned int)inode_vgid,
            info.u.param[0]);
     } else {
        char path2[512];
@@ -2754,14 +2830,17 @@ DecodeInode(char *dpath, char *name, struct ViceInodeInfo *info,
     if (strcmp(name, check))
        return -1;
 
-    GetOGMFromStat(&status, &parm, &tag);
     if ((info->inodeNumber & NAMEI_INODESPECIAL) == NAMEI_INODESPECIAL) {
+       parm = ((info->inodeNumber >> NAMEI_UNIQSHIFT) & NAMEI_UNIQMASK);
+       tag = (int)((info->inodeNumber >> NAMEI_TAGSHIFT) & NAMEI_TAGMASK);
+
        /* p1 - vid, p2 - -1, p3 - type, p4 - rwvid */
        info->u.param[0] = parm;
        info->u.param[1] = -1;
        info->u.param[2] = tag;
        info->u.param[3] = volid;
     } else {
+       GetOGMFromStat(&status, &parm, &tag);
        /* p1 - vid, p2 - vno, p3 - uniq, p4 - dv */
        info->u.param[0] = volid;
        info->u.param[1] = (int)(info->inodeNumber & NAMEI_VNODEMASK);

diff --git a/src/vol/namei_ops.h b/src/vol/namei_ops.h
index 1d5d56077aa3bd5953916b39e53926ffaa14b75b..b42151278e1549ab7f3a92e7b8adbe5c7e5fb753 100644 (file)
--- a/src/vol/namei_ops.h
+++ b/src/vol/namei_ops.h
@@ -33,6 +33,7 @@ extern int namei_inc(IHandle_t * h, Inode ino, int p1);
 extern int namei_GetLinkCount(FdHandle_t * h, Inode ino, int lockit, int fixup, int nowrite);
 extern int namei_SetLinkCount(FdHandle_t * h, Inode ino, int count, int locked);
 extern int namei_ViceREADME(char *partition);
+extern int namei_FixSpecialOGM(FdHandle_t *h, int check);
 #include "nfs.h"
 #include "viceinode.h"
 int namei_ListAFSFiles(char *dev,

diff --git a/src/vol/vol-salvage.c b/src/vol/vol-salvage.c
index 55afa95b76ce7f27b90b325a11b561bb972ddd1b..8cd5e9cb2c977496abdc240a78d89a176227722d 100644 (file)
--- a/src/vol/vol-salvage.c
+++ b/src/vol/vol-salvage.c
@@ -2466,6 +2466,14 @@ SalvageHeader(struct SalvInfo *salvinfo, struct stuff *sp,
         * it below */
        memset(&header, 0, sizeof(header));
     }
+#ifdef AFS_NAMEI_ENV
+    if (namei_FixSpecialOGM(fdP, check)) {
+       Log("Error with namei header OGM data (%s)\n", sp->description);
+       FDH_REALLYCLOSE(fdP);
+       IH_RELEASE(specH);
+       return -1;
+    }
+#endif
     if (sp->inodeType == VI_VOLINFO
        && header.volumeInfo.destroyMe == DESTROY_ME) {
        if (deleteMe)