- Allow root, the owner of the uprobes build directory and the members of the

group owner of the uprobes buld directory to build uprobes.ko.

- When building uprobes.ko, make all generated files writable by the group
owner of the uprobes build directory.

- Don't change the group owner of the uprobes build directory during
'make install'

7 files changed, 94 insertions, 69 deletions

diff --git a/Makefile.am b/Makefile.am
index 7d084324..469601c3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -242,10 +242,6 @@ install-data-local:
 		do $(INSTALL_DATA) -D $$f $(DESTDIR)$(pkgdatadir)/runtime/transport/$$f; done)
 	(cd $(srcdir)/runtime/uprobes; for f in Makefile *.[ch]; \
 		do $(INSTALL_DATA) -D $$f $(DESTDIR)$(pkgdatadir)/runtime/uprobes/$$f; done)
-	if getent group stap-server >/dev/null; then \
-		chgrp stap-server $(DESTDIR)$(pkgdatadir)/runtime/uprobes && \
-		chmod 775 $(DESTDIR)$(pkgdatadir)/runtime/uprobes; \
-	fi
 	(cd $(srcdir)/runtime/uprobes2; for f in *.[ch]; \
 		do $(INSTALL_DATA) -D $$f $(DESTDIR)$(pkgdatadir)/runtime/uprobes2/$$f; done)
 	(cd $(srcdir)/tapset; find . \( -name '*.stp' -o -name README \) -print \
diff --git a/Makefile.in b/Makefile.in
index 5b0fbaf1..22744afc 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -1839,10 +1839,6 @@ install-data-local:
 		do $(INSTALL_DATA) -D $$f $(DESTDIR)$(pkgdatadir)/runtime/transport/$$f; done)
 	(cd $(srcdir)/runtime/uprobes; for f in Makefile *.[ch]; \
 		do $(INSTALL_DATA) -D $$f $(DESTDIR)$(pkgdatadir)/runtime/uprobes/$$f; done)
-	if getent group stap-server >/dev/null; then \
-		chgrp stap-server $(DESTDIR)$(pkgdatadir)/runtime/uprobes && \
-		chmod 775 $(DESTDIR)$(pkgdatadir)/runtime/uprobes; \
-	fi
 	(cd $(srcdir)/runtime/uprobes2; for f in *.[ch]; \
 		do $(INSTALL_DATA) -D $$f $(DESTDIR)$(pkgdatadir)/runtime/uprobes2/$$f; done)
 	(cd $(srcdir)/tapset; find . \( -name '*.stp' -o -name README \) -print \
diff --git a/buildrun.cxx b/buildrun.cxx
index 601d4a10..0f2f05b5 100644
--- a/buildrun.cxx
+++ b/buildrun.cxx
@@ -19,6 +19,7 @@ extern "C" {
 #include <signal.h>
 #include <sys/wait.h>
 #include <pwd.h>
+#include <grp.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
@@ -263,6 +264,42 @@ kernel_built_uprobes (systemtap_session& s)
   return (stap_system (s.verbose, grep_cmd) == 0);
 }
 
+/*
+ * We only want root, the owner of the uprobes build directory
+ * and members of the group owning the uprobes build directory
+ * modifying uprobes.
+ */
+static bool
+may_build_uprobes (const systemtap_session& s)
+{
+  // root may build uprobes.
+  uid_t euid = geteuid ();
+  if (euid == 0)
+    return true;
+
+  // Get information on the build directory.
+  string uprobes_home = s.runtime_path + "/uprobes";
+  struct stat file_info;
+  if (stat(uprobes_home.c_str(), &file_info) != 0) {
+    clog << "Unable to obtain information on " << uprobes_home << '.' << endl;
+    return false;
+  }
+
+  // The owner of the build directory may build uprobes.
+  if (euid == file_info.st_uid)
+    return true;
+
+  // Members of the group owner of the build directory may build uprobes.
+  if (in_group_id (file_info.st_gid))
+    return true;
+
+  return false;
+}
+
+/*
+ * Use "make -q" with a fake target to
+ * verify that uprobes doesn't need to be rebuilt.
+ */
 static bool
 verify_uprobes_uptodate (systemtap_session& s)
 {
@@ -277,8 +314,24 @@ verify_uprobes_uptodate (systemtap_session& s)
   int rc = run_make_cmd(s, make_cmd);
   if (rc) {
     clog << "SystemTap's version of uprobes is out of date." << endl;
-    clog << "As root, or a member of the 'stap-server' group, run" << endl;
-    clog << "\"make -C " << uprobes_home << "\"." << endl;
+
+    struct stat file_info;
+    if (stat(uprobes_home.c_str(), &file_info) != 0) {
+      clog << "Unable to obtain information on " << uprobes_home << '.' << endl;
+    }
+    else {
+      struct passwd *owner = getpwuid (file_info.st_uid);
+      string owner_name = owner == NULL ? "The owner of " + uprobes_home :
+	                                  owner->pw_name;
+      if (owner_name == "root")
+	owner_name = "";
+      struct group *owner_group = getgrgid (file_info.st_gid);
+      string owner_group_name = owner_group == NULL ? "The owner group of " + uprobes_home :
+	                                              owner_group->gr_name;
+      clog << "As root, " << owner_name << (owner_name.empty () ? "" : ", ")
+	   << "or a member of the '" << owner_group_name << "' group, run" << endl;
+      clog << "\"make -C " << uprobes_home << "\"." << endl;
+    }
   }
 
   return rc;
@@ -306,7 +359,7 @@ make_uprobes (systemtap_session& s)
  * so the script-module build can find them.
  */
 static int
-copy_uprobes_symbols (systemtap_session& s)
+copy_uprobes_symbols (const systemtap_session& s)
 {
   string uprobes_home = s.runtime_path + "/uprobes";
   string cp_cmd = string("/bin/cp ") + uprobes_home +
@@ -323,18 +376,16 @@ uprobes_pass (systemtap_session& s)
   /*
    * We need to use the version of uprobes that comes with SystemTap, so
    * we may need to rebuild uprobes.ko there.  Unfortunately, this is
-   * never a no-op; e.g., the modpost step gets run every time.  We
-   * only want root and members of the 'stap-server' group
-   * modifying uprobes, so we keep the uprobes directory writable only by
-   * those users.  But that means that other users can't run the make
-   * even if everything's up to date.
+   * never a no-op; e.g., the modpost step gets run every time.  Only
+   * certain users can build uprobes, so we keep the uprobes directory
+   * writable only by those users.  But that means that other users
+   * can't run the make even if everything's up to date.
    *
-   * So for the other users, we just use "make -q" with a fake target to
-   * verify that uprobes doesn't need to be rebuilt.  If that's not so,
-   * stap must fail.
+   * So for the other users, we just verify that uprobes doesn't need
+   * to be rebuilt.  If that's not so, stap must fail.
    */
   int rc;
-  if (geteuid() == 0 || in_group ("stap-server"))
+  if (may_build_uprobes (s))
     rc = make_uprobes(s);
   else
     rc = verify_uprobes_uptodate(s);
diff --git a/runtime/uprobes/Makefile b/runtime/uprobes/Makefile
index 35942052..4e1280ef 100644
--- a/runtime/uprobes/Makefile
+++ b/runtime/uprobes/Makefile
@@ -10,12 +10,9 @@ CLEAN_FILES += Module.markers modules.order Module.symvers
 CLEAN_DIRS  := .tmp_versions
 
 # Build the module and sign it.
-# Make sure that all of the generated files belong the group stap-server
-# and are writable by that group. This is so that the stap-server service
-# can build or rebuild the module, if necessary.
-# If the chgrp and/or chmod commands fail, it is because we are not the
-# owner of the file for directory. In this case the file or directory
-# was created during a previous build and already has the correct permissions.
+# Ensure that the generated files are writeable by the group which
+# owns this build directory. This is so that the stap-server service
+# can rebuild the module, if necessary.
 default:
 	$(MAKE) -C $(KDIR) SUBDIRS=$(PWD) modules
 	if test -f ../../../../bin/stap-sign-module; then \
@@ -25,26 +22,25 @@ default:
 			fi \
 		done \
 	fi
-	if getent group stap-server >/dev/null; then \
-		for f in $(CLEAN_FILES); do \
-			test ! -f $$f && continue; \
-			chgrp stap-server $$f 2>/dev/null; \
-			chmod 664 $$f 2>/dev/null; \
+	group=`stat -c %G $(PWD)`; \
+	for f in $(CLEAN_FILES); do \
+		test ! -f $$f && continue; \
+		chgrp -f $$group $$f; \
+		chmod -f 664 $$f; \
+	done; \
+	for d in $(CLEAN_DIRS); do \
+		test ! -d $$d && continue; \
+		chgrp -f $$group $$d; \
+		chmod -f 775 $$d; \
+		for dd in `find $$d -type d`; do \
+			chgrp -f $$group $$dd; \
+			chmod -f 775 $$dd; \
 		done; \
-		for d in $(CLEAN_DIRS); do \
-			test ! -d $$d && continue; \
-			chgrp stap-server $$d 2>/dev/null; \
-			chmod 775 $$d 2>/dev/null; \
-			for dd in `find $$d -type d`; do \
-				chgrp stap-server $$dd 2>/dev/null; \
-				chmod 775 $$dd 2>/dev/null; \
-			done; \
-			for f in `find $$d -type f`; do \
-				chgrp stap-server $$f 2>/dev/null; \
-				chmod 664 $$f 2>/dev/null; \
-			done; \
+		for f in `find $$d -type f`; do \
+			chgrp -f $$group $$f; \
+			chmod -f 664 $$f; \
 		done; \
-	fi
+	done
 
 # This target is used with "make -q" to see whether a "real" build is needed.
 uprobes.ko: $(DEPENDENCIES)
diff --git a/testsuite/lib/systemtap.exp b/testsuite/lib/systemtap.exp
index 1a73f7cf..f16facc2 100644
--- a/testsuite/lib/systemtap.exp
+++ b/testsuite/lib/systemtap.exp
@@ -34,16 +34,13 @@ proc uprobes_p {} {
     set uprobes $env(SYSTEMTAP_RUNTIME)/uprobes
     set res [catch "exec make -q -C $uprobes uprobes.ko" output]
     if {$res != 0} {
-        if {! [installtest_p]} {
-            # build as user in the source tree
-            verbose -log "exec make -C $uprobes"
-            set res [catch "exec make -C $uprobes" output]
-            verbose -log "OUT $output"
-            verbose -log "RC $res"
-        } else {
-            # build as root in the installed location
-            set res [as_root "make -C $uprobes"]
-        }
+        # build as user at $uprobes which will be the source
+	# tree for 'make check' and the install tree for
+	# 'make installcheck'.
+	verbose -log "exec make -C $uprobes"
+	set res [catch "exec make -C $uprobes" output]
+	verbose -log "OUT $output"
+	verbose -log "RC $res"
     }
     if {$res == 0} { return 1 } else { return 0 }
 }
diff --git a/util.cxx b/util.cxx
index 6d461446..319af098 100644
--- a/util.cxx
+++ b/util.cxx
@@ -189,24 +189,13 @@ remove_file_or_dir (const char *name)
   return 0;
 }
 
-// Determine whether the current user is in the given group.
+// Determine whether the current user is in the given group
+// by gid.
 bool
-in_group (const char *gname)
+in_group_id (gid_t target_gid)
 {
   gid_t gid, gidlist[NGROUPS_MAX];
-  gid_t target_gid;
   int i, ngids;
-  struct group *group;
-
-  // Lookup the gid for the target group
-  errno = 0;
-  group = getgrnam(gname);
-
-  // If we couldn't find the group, then we can't be part of it.
-  if (group == NULL)
-    return false;
-
-  target_gid = group->gr_gid;
 
   // According to the getgroups() man page, getgroups() may not
   // return the effective gid, so try to match it first. */
diff --git a/util.h b/util.h
index b65f89e2..bb6c71bc 100644
--- a/util.h
+++ b/util.h
@@ -12,7 +12,7 @@ bool copy_file(const std::string& src, const std::string& dest,
                bool verbose=false);
 int create_dir(const char *dir);
 int remove_file_or_dir(const char *dir);
-bool in_group (const char *gname);
+bool in_group_id (gid_t target_gid);
 void tokenize(const std::string& str, std::vector<std::string>& tokens,
 	      const std::string& delimiters);
 std::string find_executable(const std::string& name);