[libgtop] Fix numerous bigs in the FreeBSD backend

From: Jasper Lievisse Adriaanse <jasperla src gnome org>
To: commits-list gnome org
Cc:
Subject: [libgtop] Fix numerous bigs in the FreeBSD backend
Date: Mon, 27 Jun 2011 14:32:59 +0000 (UTC)
commit 08fd95d2536d95776e89c6d0e4fe0b6938d8112d
Author: Joe Marcus Clarke <marcus freebsd org>
Date:   Mon Jun 27 16:32:20 2011 +0200

    Fix numerous bigs in the FreeBSD backend
    
    * Correct calculation of CPU time.
    * Fix a bug on newer versions of FreeBSD where computing the process map leads
    to a tight error loop.
    * Add support for the new procstat API to obtain the list of open files and
    cwd.
    * Use kvm_openfiles instead of kvm_open to better handle error messages.
    * Split some modules out into non-suid modules.
    * Properly determine the number of CPUs.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=605431

 sysdeps/freebsd/Makefile.am       |    4 +-
 sysdeps/freebsd/cpu.c             |   33 +++++++++++--
 sysdeps/freebsd/glibtop_machine.h |    1 +
 sysdeps/freebsd/nosuid.c          |    8 ---
 sysdeps/freebsd/open.c            |   67 +++++----------------------
 sysdeps/freebsd/procmap.c         |   10 ++--
 sysdeps/freebsd/procopenfiles.c   |   23 +++++++++-
 sysdeps/freebsd/procwd.c          |   45 +++++++++++++++---
 sysdeps/freebsd/suid_open.c       |   92 +++++++++++++++++++++++++++++++++++++
 sysdeps/freebsd/sysinfo.c         |    4 +-
 10 files changed, 201 insertions(+), 86 deletions(-)
---
diff --git a/sysdeps/freebsd/Makefile.am b/sysdeps/freebsd/Makefile.am
index e3409e8..ee3df90 100644
--- a/sysdeps/freebsd/Makefile.am
+++ b/sysdeps/freebsd/Makefile.am
@@ -7,12 +7,12 @@ libgtop_sysdeps_2_0_la_SOURCES	= nosuid.c siglist.c sysinfo.c shm_limits.c \
 				  cpu.c msg_limits.c sem_limits.c loadavg.c \
 				  uptime.c netlist.c fsusage.c mem.c \
 				  mountlist.c procopenfiles.c procwd.c \
-				  procaffinity.c glibtop_private.c
+				  procaffinity.c glibtop_private.c open.c
 
 libgtop_sysdeps_2_0_la_LDFLAGS	= $(LT_VERSION_INFO)
 
 libgtop_sysdeps_suid_2_0_la_LIBADD	= $(KVM_LIBS) $(EXTRA_SYSDEPS_LIBS)
-libgtop_sysdeps_suid_2_0_la_SOURCES	= open.c close.c swap.c \
+libgtop_sysdeps_suid_2_0_la_SOURCES	= suid_open.c close.c swap.c \
 				  proclist.c procstate.c procuid.c \
 				  proctime.c procmem.c procsignal.c \
 				  prockernel.c procsegment.c procargs.c \
diff --git a/sysdeps/freebsd/cpu.c b/sysdeps/freebsd/cpu.c
index 9577703..96e96da 100644
--- a/sysdeps/freebsd/cpu.c
+++ b/sysdeps/freebsd/cpu.c
@@ -54,6 +54,7 @@ void
 glibtop_get_cpu_s (glibtop *server, glibtop_cpu *buf)
 {
 	long cpts [CPUSTATES];
+	long *cp_times = NULL;
 	struct clockinfo ci;
 	size_t length;
 	int ncpu, i;
@@ -75,6 +76,16 @@ glibtop_get_cpu_s (glibtop *server, glibtop_cpu *buf)
 		return;
 	}
 
+	length = 0;
+	if (sysctlbyname ("kern.cp_times", NULL, &length, NULL, 0) == 0) {
+		cp_times = g_malloc (length);
+		length = sizeof(long) * CPUSTATES * (length / (sizeof(long) * CPUSTATES));
+		if (sysctlbyname ("kern.cp_times", cp_times, &length, NULL, 0)) {
+			g_free (cp_times);
+			cp_times = NULL;
+		}
+	}
+
 	/* set user time */
 	buf->user = cpts [CP_USER];
 	/* set nice time */
@@ -89,22 +100,32 @@ glibtop_get_cpu_s (glibtop *server, glibtop_cpu *buf)
 	/* set frequency */
 	buf->frequency = (ci.stathz ? ci.stathz : ci.hz);
 	/* set total */
-	buf->total = cpts [CP_USER] + cpts [CP_NICE]
+	buf->total = cpts [CP_USER] + cpts [CP_NICE] \
 		+ cpts [CP_SYS] + cpts [CP_IDLE] + cpts [CP_INTR];
 
 	ncpu = server->ncpu + 1;
 
 	for (i = 0; i < ncpu; i++) {
-		buf->xcpu_user[i] = cpts [CP_USER] / ncpu;
-		buf->xcpu_nice[i] = cpts [CP_NICE] / ncpu;
-		buf->xcpu_sys[i] = cpts [CP_SYS] / ncpu;
-		buf->xcpu_idle[i] = cpts [CP_IDLE] / ncpu;
-		buf->xcpu_irq[i] = cpts [CP_INTR] / ncpu;
+		if (cp_times) {
+			buf->xcpu_user[i] = cp_times[i * CPUSTATES + CP_USER];
+			buf->xcpu_nice[i] = cp_times[i * CPUSTATES + CP_NICE];
+			buf->xcpu_sys[i] = cp_times[i * CPUSTATES + CP_SYS];
+			buf->xcpu_idle[i] = cp_times[i * CPUSTATES + CP_IDLE];
+			buf->xcpu_irq[i] = cp_times[i * CPUSTATES + CP_INTR];
+		} else {
+			buf->xcpu_user[i] = cpts [CP_USER] / ncpu;
+			buf->xcpu_nice[i] = cpts [CP_NICE] / ncpu;
+			buf->xcpu_sys[i] = cpts [CP_SYS] / ncpu;
+			buf->xcpu_idle[i] = cpts [CP_IDLE] / ncpu;
+			buf->xcpu_irq[i] = cpts [CP_INTR] / ncpu;
+		}
 		buf->xcpu_total[i] = buf->xcpu_user[i] + buf->xcpu_nice[i] \
 				     + buf->xcpu_sys[i] + buf->xcpu_idle[i] \
 				     + buf->xcpu_irq[i];
 	}
 
+	g_free (cp_times);
+
 	/* Set the flags last. */
 	buf->flags = _glibtop_sysdeps_cpu;
 
diff --git a/sysdeps/freebsd/glibtop_machine.h b/sysdeps/freebsd/glibtop_machine.h
index 5728902..ca6266e 100644
--- a/sysdeps/freebsd/glibtop_machine.h
+++ b/sysdeps/freebsd/glibtop_machine.h
@@ -24,6 +24,7 @@
 
 #include <sys/param.h>
 #include <nlist.h>
+#include <limits.h>
 #include <kvm.h>
 #include <sys/dkstat.h>
 #include <time.h>
diff --git a/sysdeps/freebsd/nosuid.c b/sysdeps/freebsd/nosuid.c
index b43eaca..0394540 100644
--- a/sysdeps/freebsd/nosuid.c
+++ b/sysdeps/freebsd/nosuid.c
@@ -21,16 +21,8 @@
 
 #include <config.h>
 #include <glibtop.h>
-#include <glibtop/open.h>
 #include <glibtop/close.h>
 
 void
-glibtop_open_s (glibtop *server,
-		const char *program_name,
-		const unsigned long features,
-		const unsigned flags)
-{ }
-
-void
 glibtop_close_s (glibtop *server)
 { }
diff --git a/sysdeps/freebsd/open.c b/sysdeps/freebsd/open.c
index 89fac3b..39a768d 100644
--- a/sysdeps/freebsd/open.c
+++ b/sysdeps/freebsd/open.c
@@ -20,76 +20,33 @@
 */
 
 #include <config.h>
+#include <sys/types.h>
+#include <sys/sysctl.h>
 #include <glibtop.h>
-#include <glibtop/error.h>
 #include <glibtop/open.h>
-#include <glibtop/init_hooks.h>
-
-/* !!! THIS FUNCTION RUNS SUID ROOT - CHANGE WITH CAUTION !!! */
-
-void
-glibtop_init_p (glibtop *server, const unsigned long features,
-		const unsigned flags)
-{
-	const _glibtop_init_func_t *init_fkt;
-
-	if (server == NULL)
-		glibtop_error_r (NULL, "glibtop_init_p (server == NULL)");
-
-	/* Do the initialization, but only if not already initialized. */
-
-	if ((server->flags & _GLIBTOP_INIT_STATE_SYSDEPS) == 0) {
-		glibtop_open_p (server, "glibtop", features, flags);
+#include <glibtop/cpu.h>
+#include <glibtop/error.h>
 
-		for (init_fkt = _glibtop_init_hook_p; *init_fkt; init_fkt++)
-			(*init_fkt) (server);
+#include <glibtop_private.h>
 
-		server->flags |= _GLIBTOP_INIT_STATE_SYSDEPS;
-	}
-}
 
 void
-glibtop_open_p (glibtop *server, const char *program_name,
+glibtop_open_s (glibtop *server, const char *program_name,
 		const unsigned long features,
 		const unsigned flags)
 {
-#ifdef DEBUG
-	fprintf (stderr, "DEBUG (%d): glibtop_open_p ()\n", getpid ());
-#endif
+	int ncpus;
+	size_t len;
 
-	/* !!! WE ARE ROOT HERE - CHANGE WITH CAUTION !!! */
+	len = sizeof (ncpus);
+	sysctlbyname ("hw.ncpu", &ncpus, &len, NULL, 0);
+	server->real_ncpu = ncpus - 1;
+	server->ncpu = MIN(GLIBTOP_NCPU - 1, server->real_ncpu);
 
-	server->machine.uid = getuid ();
-	server->machine.euid = geteuid ();
-	server->machine.gid = getgid ();
-	server->machine.egid = getegid ();
 #if defined(__FreeBSD_kernel__)
 	server->os_version_code = __FreeBSD_kernel_version;
 #else
 	server->os_version_code = __FreeBSD_version;
 #endif
-	/* Setup machine-specific data */
-	server->machine.kd = kvm_open (NULL, NULL, NULL, O_RDONLY, "kvm_open");
-
-	if (server->machine.kd == NULL)
-		glibtop_error_io_r (server, "kvm_open");
-
-	/* Drop priviledges. */
-
-	if (setreuid (server->machine.euid, server->machine.uid))
-		_exit (1);
-
-	if (setregid (server->machine.egid, server->machine.gid))
-		_exit (1);
-
-	/* !!! END OF SUID ROOT PART !!! */
-
-	/* Our effective uid is now those of the user invoking the server,
-	 * so we do no longer have any priviledges. */
 
-	/* NOTE: On FreeBSD, we do not need to be suid root, we just need to
-	 * be sgid kmem.
-	 *
-	 * The server will only use setegid() to get back it's priviledges,
-	 * so it will fail if it is suid root and not sgid kmem. */
 }
diff --git a/sysdeps/freebsd/procmap.c b/sysdeps/freebsd/procmap.c
index d4022dd..c29691a 100644
--- a/sysdeps/freebsd/procmap.c
+++ b/sysdeps/freebsd/procmap.c
@@ -85,7 +85,9 @@ _glibtop_sysdeps_freebsd_dev_inode (glibtop *server, struct vnode *vnode,
         char tagstr[12];
         struct inode inode;
         struct cdev_priv priv;
+#if __FreeBSD_version < 800039
         struct cdev si;
+#endif
 
         *inum = 0;
         *dev = 0;
@@ -111,18 +113,18 @@ _glibtop_sysdeps_freebsd_dev_inode (glibtop *server, struct vnode *vnode,
                 return;
         }
 
-        if (kvm_read (server->machine.kd, (gulong) inode.i_dev, (char *) &si,
-	              sizeof (si)) != sizeof (si) ||
 #if __FreeBSD_version >= 800039
-            kvm_read (server->machine.kd, (gulong)  cdev2priv(&si), (char *) &priv,
+        if (kvm_read (server->machine.kd, (gulong) cdev2priv(inode.i_dev), (char *) &priv,
 		      sizeof (priv))
 #else
+        if (kvm_read (server->machine.kd, (gulong) inode.i_dev, (char *) &si,
+	              sizeof (si)) != sizeof (si) ||
             kvm_read (server->machine.kd, (gulong) si.si_priv, (char *) &priv,
 		      sizeof (priv))
 #endif
 	    != sizeof (priv))
         {
-                glibtop_warn_io_r (server, "kvm_read (si)");
+                glibtop_warn_io_r (server, "kvm_read (priv)");
                 return;
         }
 
diff --git a/sysdeps/freebsd/procopenfiles.c b/sysdeps/freebsd/procopenfiles.c
index f85e7af..2d3f7d7 100644
--- a/sysdeps/freebsd/procopenfiles.c
+++ b/sysdeps/freebsd/procopenfiles.c
@@ -35,6 +35,9 @@
 #include <sys/user.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
+#ifdef HAVE_KINFO_GETFILE
+#include <libutil.h>
+#endif
 #include <string.h>
 #include <stdlib.h>
 
@@ -263,9 +266,13 @@ glibtop_get_proc_open_files_s (glibtop *server, glibtop_proc_open_files *buf,	pi
 {
 #if __FreeBSD_version > 800018 || (__FreeBSD_version < 800000 && __FreeBSD_version >= 700104)
 	struct kinfo_file *freep, *kif;
+#ifndef HAVE_KINFO_GETFILE
 	int name[4];
 	size_t len;
-	size_t i;
+#else
+	int cnt;
+#endif
+	ssize_t i;
 #else
 	char *output;
 #endif
@@ -274,6 +281,7 @@ glibtop_get_proc_open_files_s (glibtop *server, glibtop_proc_open_files *buf,	pi
 	memset(buf, 0, sizeof (glibtop_proc_open_files));
 
 #if __FreeBSD_version > 800018 || (__FreeBSD_version < 800000 && __FreeBSD_version >= 700104)
+#ifndef HAVE_KINFO_GETFILE
 	name[0] = CTL_KERN;
 	name[1] = KERN_PROC;
 	name[2] = KERN_PROC_FILEDESC;
@@ -287,12 +295,25 @@ glibtop_get_proc_open_files_s (glibtop *server, glibtop_proc_open_files *buf,	pi
 		g_free(freep);
 		return NULL;
 	}
+#else
+	freep = kinfo_getfile(pid, &cnt);
+#endif
 
 	entries = g_array_new(FALSE, FALSE, sizeof(glibtop_open_files_entry));
 
+#ifndef HAVE_KINFO_GETFILE
 	for (i = 0; i < len / sizeof(*kif); i++, kif++) {
 		glibtop_open_files_entry entry = {0};
 
+		if (kif->kf_structsize != sizeof(*kif))
+			continue;
+#else
+	for (i = 0; i < cnt; i++) {
+		glibtop_open_files_entry entry = {0};
+
+		kif = &freep[i];
+#endif
+
 		if (kif->kf_fd < 0)
 			continue;
 
diff --git a/sysdeps/freebsd/procwd.c b/sysdeps/freebsd/procwd.c
index ae6793c..7970574 100644
--- a/sysdeps/freebsd/procwd.c
+++ b/sysdeps/freebsd/procwd.c
@@ -27,6 +27,9 @@
 #include <sys/sysctl.h>
 #include <sys/param.h>
 #include <sys/user.h>
+#ifdef HAVE_KINFO_GETFILE
+#include <libutil.h>
+#endif
 #include <string.h>
 
 static const unsigned long _glibtop_sysdeps_proc_wd =
@@ -40,7 +43,7 @@ _glibtop_init_proc_wd_s(glibtop *server)
 	server->sysdeps.proc_wd = _glibtop_sysdeps_proc_wd;
 }
 
-#if (__FreeBSD_version >= 800000 && __FreeBSD_version < 800019) || _FreeBSD_version < 700104
+#if (__FreeBSD_version >= 800000 && __FreeBSD_version < 800019) || __FreeBSD_version < 700104
 static GPtrArray *
 parse_output(const char *output, glibtop_proc_wd *buf)
 {
@@ -97,24 +100,39 @@ parse_output(const char *output, glibtop_proc_wd *buf)
 char**
 glibtop_get_proc_wd_s(glibtop *server, glibtop_proc_wd *buf, pid_t pid)
 {
-	char path[MAXPATHLEN];
+	int exe_mib[4];
+	size_t len;
 #if __FreeBSD_version > 800018 || (__FreeBSD_version < 800000 && __FreeBSD_version >= 700104)
 	struct kinfo_file *freep, *kif;
 	GPtrArray *dirs;
-	size_t len;
-	int i;
+#ifndef HAVE_KINFO_GETFILE
 	int name[4];
 #else
+	int cnt;
+#endif
+	int i;
+#else
 	char *output;
 #endif
 
 	memset (buf, 0, sizeof (glibtop_proc_wd));
-
-	g_snprintf(path, sizeof(path), "/proc/%u/file", pid);
-	if (safe_readlink(path, buf->exe, sizeof(buf->exe)))
-		buf->flags |= (1 << GLIBTOP_PROC_WD_EXE);
+	len = 0;
+
+	exe_mib[0] = CTL_KERN;
+	exe_mib[1] = KERN_PROC;
+	exe_mib[2] = KERN_PROC_PATHNAME;
+	exe_mib[3] = pid;
+
+	if (sysctl(exe_mib, 4, NULL, &len, NULL, 0) == 0) {
+		if (len > sizeof(buf->exe))
+			len = sizeof(buf->exe);
+		if (sysctl(exe_mib, 4, buf->exe, &len, NULL, 0) == 0)
+			buf->flags |= (1 << GLIBTOP_PROC_WD_EXE);
+	}
 
 #if __FreeBSD_version > 800018 || (__FreeBSD_version < 800000 && __FreeBSD_version >= 700104)
+#ifndef HAVE_KINFO_GETFILE
+	len = 0;
 	name[0] = CTL_KERN;
 	name[1] = KERN_PROC;
 	name[2] = KERN_PROC_FILEDESC;
@@ -127,10 +145,21 @@ glibtop_get_proc_wd_s(glibtop *server, glibtop_proc_wd *buf, pid_t pid)
 		g_free(freep);
 		return NULL;
 	}
+#else
+	freep = kinfo_getfile(pid, &cnt);
+#endif
 
 	dirs = g_ptr_array_sized_new(1);
 
+#ifndef HAVE_KINFO_GETFILE
 	for (i = 0; i < len / sizeof(*kif); i++, kif++) {
+		if (kif->kf_structsize != sizeof(*kif))
+			continue;
+#else
+	for (i = 0; i < cnt; i++) {
+		kif = &freep[i];
+#endif
+
 		switch (kif->kf_fd) {
 			case KF_FD_TYPE_ROOT:
 				g_strlcpy(buf->root, kif->kf_path,
diff --git a/sysdeps/freebsd/suid_open.c b/sysdeps/freebsd/suid_open.c
new file mode 100644
index 0000000..10415ef
--- /dev/null
+++ b/sysdeps/freebsd/suid_open.c
@@ -0,0 +1,92 @@
+/* Copyright (C) 1998 Joshua Sled
+   This file is part of LibGTop 1.0.
+
+   Contributed by Joshua Sled <jsled xcf berkeley edu>, July 1998.
+
+   LibGTop is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License,
+   or (at your option) any later version.
+
+   LibGTop is distributed in the hope that it will be useful, but WITHOUT
+   ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+   FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+   for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with LibGTop; see the file COPYING. If not, write to the
+   Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.
+*/
+
+#include <config.h>
+#include <glibtop.h>
+#include <glibtop/error.h>
+#include <glibtop/cpu.h>
+#include <glibtop/open.h>
+#include <glibtop/init_hooks.h>
+
+
+/* !!! THIS FUNCTION RUNS SUID ROOT - CHANGE WITH CAUTION !!! */
+
+void
+glibtop_init_p (glibtop *server, const unsigned long features,
+		const unsigned flags)
+{
+	const _glibtop_init_func_t *init_fkt;
+
+	if (server == NULL)
+		glibtop_error_r (NULL, "glibtop_init_p (server == NULL)");
+
+	/* Do the initialization, but only if not already initialized. */
+
+	if ((server->flags & _GLIBTOP_INIT_STATE_SYSDEPS) == 0) {
+		glibtop_open_p (server, "glibtop", features, flags);
+
+		for (init_fkt = _glibtop_init_hook_p; *init_fkt; init_fkt++)
+			(*init_fkt) (server);
+
+		server->flags |= _GLIBTOP_INIT_STATE_SYSDEPS;
+	}
+}
+
+void
+glibtop_open_p (glibtop *server, const char *program_name,
+		const unsigned long features,
+		const unsigned flags)
+{
+	char errbuf[_POSIX2_LINE_MAX];
+#ifdef DEBUG
+	fprintf (stderr, "DEBUG (%d): glibtop_open_p ()\n", getpid ());
+#endif
+
+	/* !!! WE ARE ROOT HERE - CHANGE WITH CAUTION !!! */
+	server->machine.uid = getuid ();
+	server->machine.euid = geteuid ();
+	server->machine.gid = getgid ();
+	server->machine.egid = getegid ();
+	/* Setup machine-specific data */
+	server->machine.kd = kvm_openfiles (NULL, NULL, NULL, O_RDONLY, errbuf);
+
+	if (server->machine.kd == NULL)
+		glibtop_error_io_r (server, "kvm_open");
+
+	/* Drop priviledges. */
+
+	if (setreuid (server->machine.euid, server->machine.uid))
+		_exit (1);
+
+	if (setregid (server->machine.egid, server->machine.gid))
+		_exit (1);
+
+	/* !!! END OF SUID ROOT PART !!! */
+
+	/* Our effective uid is now those of the user invoking the server,
+	 * so we do no longer have any priviledges. */
+
+	/* NOTE: On FreeBSD, we do not need to be suid root, we just need to
+	 * be sgid kmem.
+	 *
+	 * The server will only use setegid() to get back it's priviledges,
+	 * so it will fail if it is suid root and not sgid kmem. */
+}
diff --git a/sysdeps/freebsd/sysinfo.c b/sysdeps/freebsd/sysinfo.c
index 9e281ca..aeea1de 100644
--- a/sysdeps/freebsd/sysinfo.c
+++ b/sysdeps/freebsd/sysinfo.c
@@ -45,8 +45,8 @@ init_sysinfo (glibtop *server)
 
 	glibtop_init_s (&server, GLIBTOP_SYSDEPS_CPU, 0);
 
-	len = sizeof (ncpus);
-	sysctlbyname ("hw.ncpu", &ncpus, &len, NULL, 0);
+	ncpus = server->ncpu + 1;
+
 	len = 0;
 	sysctlbyname ("hw.model", NULL, &len, NULL, 0);
 	model = g_malloc (len);
[Date Prev][Date Next] [Thread Prev][Thread Next] [Thread Index] [Date Index] [Author Index]