--uh9ZiVrAOUUm9fzH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hello,

install(1) currently does not metalog hard links correctly; e.g. syss?tat
entries from my /etc/mtree/set.base look like

./usr/bin/systat  type=file mode=02555 uname=root gname=kmem sha1=...
./usr/bin/sysstat type=file mode=0555                        sha1=...

Obviously, only the first entry is correct:

$ ls -li /usr/bin/sys*tat
107653 -r-xr-sr-x  2 root  kmem  96480 Jul  7 15:43 /usr/bin/sysstat
107653 -r-xr-sr-x  2 root  kmem  96480 Jul  7 15:43 /usr/bin/systat

$ mtree -e -p / -f /etc/mtree/set.base
[...]
usr/bin/sysstat: 
	gid (0, 2)
	permissions (0555, 02555)


Two questions about makelink() from src/usr.bin/xinstall/xinstall.c:

- The mode being metalogged for hard links is the mode of the source link
  masked with 0777 - shouldn't this be 07777? I.e. why are e.g. the s[ug]id
  bits cleared? That's why the mode of entries like the sysstat entry above
  is logged wrong (even when running privleged).

- Why are owner, group and fflags ignored?


To fix the problem for privileged installs seems to be easy: just use the
source link's owner, group, permissions, etc.; see attached patch.

For unprivileged installs it's probably more difficult: the source link
is possibly installed with "wrong" owner, group, etc. I see two possible
solutions: either read the METALOG to determine the information about the
source link (probably a bad idea...), or simply pass the information to
install(1) on invocation, as it's done if a regular file is installed,
and don't ignore it when metalogging (see patch).

With the patch applied, install can be used as

$ install -U -M $log -l h -o $owner -g $group -m $mode ... $dest $link

However, I'm not sure about how to implement this in bsd.links.mk; how
do I determine the mode logged for the source link? And how about owner
and group? BINOWN and BINGRP?


Comments are appreciated!

Cheers, Jukka

P.S.: there's a bug in xinstall.c: if more than one flag is specified
(install -f flag1,flag2,...), only the last one is used; that's because
string_to_flags() modifies the pointer and uses strsep(3) on it. Maybe
this should be fixed in src/bin/ls/stat_flags.c, but I didn't check if
any other programs depend on this behaviour. It's probably easier to just
restore the string (see patch).

-- 
bashian roulette:
$ ((RANDOM%6)) || rm -rf ~

--uh9ZiVrAOUUm9fzH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="xinstall.c.diff"

Index: xinstall.c
===================================================================
RCS file: /cvsroot/src/usr.bin/xinstall/xinstall.c,v
retrieving revision 1.91
diff -u -p -r1.91 xinstall.c
--- xinstall.c	11 Jun 2005 22:59:05 -0000	1.91
+++ xinstall.c	8 Jul 2005 14:49:53 -0000
@@ -270,8 +270,9 @@ main(int argc, char *argv[])
 	if ((dostrip || dolink) && dodir)
 		usage();
 
-	/* strip and flags make no sense with links */
-	if ((dostrip || fflags) && dolink)
+	/* strip and flags make no sense with links, but allow flags if
+	   running unprivileged */
+	if ((dostrip || (fflags && !dounpriv)) && dolink)
 		usage();
 
 	/* must have at least two arguments, except when creating directories */
@@ -310,6 +311,8 @@ main(int argc, char *argv[])
 	if (fflags && !dounpriv) {
 		if (string_to_flags(&fflags, &fileflags, NULL))
 			errx(1, "%s: invalid flag", fflags);
+		/* restore fflags since string_to_flags() changed it */
+		fflags = flags_to_string(fileflags, "-");
 		iflags |= SETFLAGS;
 	}
 #endif
@@ -468,15 +471,26 @@ makelink(char *from_name, char *to_name)
 				char *oowner, *ogroup, *offlags;
 				char *dres;
 
+				if (!dounpriv) {
 					/* XXX: use underlying perms */
-				omode = mode;
-				mode = (to_sb.st_mode & 0777);
-				oowner = owner;
-				owner = NULL;
-				ogroup = group;
-				group = NULL;
-				offlags = fflags;
-				fflags = NULL;
+					omode = mode;
+					mode = (to_sb.st_mode & 07777);
+					oowner = owner;
+					owner = (char *)user_from_uid(
+						to_sb.st_uid, 0);
+					ogroup = group;
+					group = (char *)group_from_gid(
+						to_sb.st_gid, 0);
+					offlags = fflags;
+					fflags = NULL;
+#if ! HAVE_NBTOOL_CONFIG_H
+					fflags = flags_to_string(
+						to_sb.st_flags, "-");
+					if (strcmp(fflags, "-") == 0)
+						fflags = NULL;
+#endif
+				}
+
 				switch (digesttype) {
 				case DIGEST_MD5:
 					dres = MD5File(from_name, NULL);
@@ -492,10 +506,13 @@ makelink(char *from_name, char *to_name)
 				}
 				metadata_log(to_name, "file", NULL, NULL, dres);
 				free(dres);
-				mode = omode;
-				owner = oowner;
-				group = ogroup;
-				fflags = offlags;
+
+				if (!dounpriv) {
+					mode = omode;
+					owner = oowner;
+					group = ogroup;
+					fflags = offlags;
+				}
 			}
 			return;
 		}

--uh9ZiVrAOUUm9fzH--