afs: Avoid giving wrong 'tf' to afs_InitVolSlot

Commit 75e3a589 (libafs: afs_InitVolSlot function) split out a bit of
our code that initializes a struct volume into the afs_InitVolSlot
function. However, it caused us to almost always pass a non-NULL 'tf'
to afs_InitVolSlot, even if the target volume was not found.

That is, before that commit, our code roughly did this:

    for (...; j != 0; j = tf->next) {
        ...;
        tf = &staticVolume;
        if (tf->volume == volid)
            break;
    }
    if (tf && j != 0) {
        use_tf_data();
    } else {
        use_blank_data();
    }

The reason for the extra 'j != 0' check after the loop is to see if we
hit the end of the volume hash chain, or if we actually found a
matching 'tf' in the loop.

And after that commit, the code did this:

    for (...; j != 0; j = tf->next) {
        ...;
        if (j != 0) {
            tf = &staticVolume;
            if (tf->volume == volid)
                break;
        }
    }
    if (tf) {
        use_tf_data();
    } else {
        use_blank_data();
    }

The check for 'j != 0' was moved to inside the for loop, but 'j' is
always nonzero in the loop (otherwise, the for() would exit the loop).
This means that if we didn't find a matching 'tf' in the loop, our
'tf' would be non-NULL anyway, and so we'd initialize our volume slot
from just the last entry in the hash chain.

This means that for volumes that are not found in the VolumeItems
file, our struct volume will probably be initialized with arbitrary
data from another volume, instead of being initialized to the normal
defaults (the 'else' clause in afs_InitVolSlot).

This means that the 'dotdot' entry for the volume may be wrong, and so
we may report the wrong parent dir for the root of a volume. However,
the 'dotdot' entry should be fixed when the volume root is accessed
via a mountpoint, so any such issue should be temporary. And of
course, on some platforms (LINUX) we don't ever use the 'dotdot'
information for a volume, and even on other platforms, often resolving
the '..' entry is handled by other means (e.g. shells often calculate
it themselves). But some 'pwd' calculations and other '..' corner
cases may be affected.

To fix this, change the relevant loop so that we only set 'tf' to
non-NULL when we actually find a matching entry.

Reviewed-on: https://gerrit.openafs.org/13933
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 4a9078c6bbf51720a5eacf7e6ba21443e5103eee)

Change-Id: Ib1e7519db8f844872c4b88b54978f358ff7b299e
Reviewed-on: https://gerrit.openafs.org/13937
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>

diff --git a/src/afs/afs_volume.c b/src/afs/afs_volume.c
index ff7cc47..2793b28 100644 (file)
--- a/src/afs/afs_volume.c
+++ b/src/afs/afs_volume.c
@@ -248,8 +248,10 @@ afs_UFSGetVolSlot(afs_int32 volid, struct cell *tcell)
     }
 
     /* read volume item data from disk for the gotten slot */
-    for (j = fvTable[FVHash(tcell->cellNum, volid)]; j != 0; j = tf->next) {
+    for (j = fvTable[FVHash(tcell->cellNum, volid)]; j != 0; j = staticFVolume.next) {
        if (afs_FVIndex != j) {
+           /* The data in staticFVolume is currently for a different slot.
+            * Read the data for slot 'j' into staticFVolume. */
            tfile = osi_UFSOpen(&volumeInode);
            if (!tfile) {
                afs_warn("afs_UFSGetVolSlot: unable to open volumeinfo\n");
@@ -274,11 +276,9 @@ afs_UFSGetVolSlot(afs_int32 volid, struct cell *tcell)
            }
            afs_FVIndex = j;
        }
-       if (j != 0) {           /* volume items record 0 is not used */
+       if (staticFVolume.cell == tcell->cellNum && staticFVolume.volume == volid) {
            tf = &staticFVolume;
-           if (tf->cell == tcell->cellNum && tf->volume == volid) {
-               break;
-           }
+           break;
        }
     }