From f01f910f19f62589ecc394da0ed22c0be27d9e66 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Tue, 5 Nov 2019 10:50:01 -0600 Subject: [PATCH] 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 Reviewed-by: Cheyenne Wills Reviewed-by: Michael Meffie Reviewed-by: Benjamin Kaduk (cherry picked from commit 4a9078c6bbf51720a5eacf7e6ba21443e5103eee) Change-Id: Ib1e7519db8f844872c4b88b54978f358ff7b299e Reviewed-on: https://gerrit.openafs.org/13937 Reviewed-by: Andrew Deason Tested-by: Andrew Deason Reviewed-by: Michael Meffie Reviewed-by: Cheyenne Wills Reviewed-by: Stephan Wiesand --- src/afs/afs_volume.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/afs/afs_volume.c b/src/afs/afs_volume.c index ff7cc4718..2793b2813 100644 --- 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; } } -- 2.39.5