[OpenAFS] Reduce mount point/symlink name space abuse

[Rainer Toebbicke]

--------------050908080104060902070905
Content-Type: text/plain; charset="ISO-8859-15"; format=flowed
Content-Transfer-Encoding: 7bit

As people ever trying to create a symlink to a file starting with '%' 
or '#' (either by finger trouble, using Gnome, or on purpose) have 
certainly already experienced: this does not work as expected. The 
target is assumed to be a volume name in those cases, and is normally 
useless and difficult to get rid of unless it happens to correspond to 
a valid volumme name.

However, since eons "fs mkmount" (the official way) creates mount 
points not only starting with a '%' or a '#', but also ending with in 
a '.' (dot)!

Worse, on 'input' a mount is only recognized if it corresponds to the 
strict format imposed by fs mkmount, hence every now and then we end 
up with mount points which are not recognized as such by the cache 
manager, and become, de facto, unusable and unremovable.

With the attached patch symlinks to targets starting with '%' or '#' 
are henceforth only interpreted as mount points when they also end in 
a '.' (dot). Still ugly, but this reduces the part of the name space 
carved out for "fs mkmount". 'ln -s %hash percent' now works and the 
link works normally afterwards. (sigh, linking to '%hash.'remains 
problematic - the correct solution would involve a special "create 
mount" system call).

(remark: I'm speaking Unix here, I don't know about symlinks in 
Windows nor how the cache manager handles them there).

The patch also updates the salvager so that it deals correctly with 
the dead bodies created over the years by this discrepancy.


-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
Rainer Toebbicke
European Laboratory for Particle Physics(CERN) - Geneva, Switzerland
Phone: +41 22 767 8985       Fax: +41 22 767 7155

--------------050908080104060902070905
Content-Type: text/plain; name="p_mp_symlink"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="p_mp_symlink"
Content-Description: p_mp_symlink

--- openafs/src/afs/VNOPS/afs_vnop_symlink.c.o1	2005-10-15 04:33:12.000000000 +0200
+++ openafs/src/afs/VNOPS/afs_vnop_symlink.c	2008-03-27 14:44:58.000000000 +0100
@@ -107,7 +107,7 @@
     InStatus.Mask = AFS_SETMODTIME | AFS_SETMODE;
     InStatus.ClientModTime = osi_Time();
     alen = strlen(atargetName);	/* we want it to include the null */
-    if (*atargetName == '#' || *atargetName == '%') {
+    if ( (*atargetName == '#' || *atargetName == '%') && alen > 1 && atargetName[alen-1] == '.') {
 	InStatus.UnixModeBits = 0644;	/* mt pt: null from "." at end */
 	if (alen == 1)
 	    alen++;		/* Empty string */
--- openafs/src/vol/vol-salvage.c.o1	2008-01-07 10:01:37.000000000 +0100
+++ openafs/src/vol/vol-salvage.c	2008-03-27 13:53:28.000000000 +0100
@@ -3035,27 +3035,47 @@
     } else {
 	if (ShowSuid && (vnodeEssence->modeBits & 06000))
 	    Log("FOUND suid/sgid file: %s/%s (%u.%u %05o) author %u (vnode %u dir %u)\n", dir->name ? dir->name : "??", name, vnodeEssence->owner, vnodeEssence->group, vnodeEssence->modeBits, vnodeEssence->author, vnodeNumber, dir->vnodeNumber);
-	if (ShowMounts && (vnodeEssence->type == vSymlink)
+	if (/* ShowMounts && */ (vnodeEssence->type == vSymlink)
 	    && !(vnodeEssence->modeBits & 0111)) {
 	    int code, size;
-	    char buf[1024];
+	    char buf[1025];
 	    IHandle_t *ihP;
 	    FdHandle_t *fdP;
 
 	    IH_INIT(ihP, fileSysDevice, dir->dirHandle.dirh_handle->ih_vid,
 		    vnodeEssence->InodeNumber);
 	    fdP = IH_OPEN(ihP);
-	    assert(fdP != NULL);
+	    if (fdP == NULL) {
+		Log("ERROR %s could not open mount point vnode %u\n", dir->vname, vnodeNumber);
+		IH_RELEASE(ihP);
+		return;
+	    }
 	    size = FDH_SIZE(fdP);
-	    assert(size != -1);
-	    memset(buf, 0, 1024);
+	    if (size < 0) {
+		Log("ERROR %s mount point has invalid size %d, vnode %u\n", dir->vname, size, vnodeNumber);
+		FDH_REALLYCLOSE(fdP);
+		IH_RELEASE(ihP);
+		return;
+	    }
+	
 	    if (size > 1024)
 		size = 1024;
 	    code = FDH_READ(fdP, buf, size);
-	    assert(code == size);
-	    Log("In volume %u (%s) found mountpoint %s/%s to '%s'\n",
-		dir->dirHandle.dirh_handle->ih_vid, dir->vname,
-		dir->name ? dir->name : "??", name, buf);
+	    if (code == size) {
+		buf[size] = '\0';
+		if ( (*buf != '#' && *buf != '%') || buf[strlen(buf)-1] != '.' ) {
+		    Log("Volume %u (%s) mount point %s/%s to '%s' invalid, %s to symbolic link\n",
+			dir->dirHandle.dirh_handle->ih_vid, dir->vname, dir->name ? dir->name : "??", name, buf,
+			Testing ? "would convert" : "converted");
+		    vnodeEssence->modeBits |= 0111;
+		    vnodeEssence->changed = 1;
+		} else if (ShowMounts) Log("In volume %u (%s) found mountpoint %s/%s to '%s'\n",
+		    dir->dirHandle.dirh_handle->ih_vid, dir->vname,
+		    dir->name ? dir->name : "??", name, buf);
+	    } else {
+		Log("Volume %s cound not read mount point vnode %u size %d code %d\n",
+		    dir->vname, vnodeNumber, size, code);
+	    }
 	    FDH_REALLYCLOSE(fdP);
 	    IH_RELEASE(ihP);
 	}
@@ -3531,6 +3551,8 @@
 		    if (!Showmode) {
 			Log("Vnode %u: link count incorrect (was %d, %s %d)\n", vnodeNumber, oldCount, (Testing ? "would have changed to" : "now"), vnode.linkCount);
 		    }
+		} else {
+		    vnode.modeBits = vnp->modeBits;
 		}
 
 		vnode.dataVersion++;

--------------050908080104060902070905--