Date: Mon, 28 May 2001 18:10:03 -0700 (PDT) From: Dima Dorfman <dima@unixfreak.org> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/26787: sysctl change request Message-ID: <200105290110.f4T1A3u68170@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/26787; it has been noted by GNATS.
From: Dima Dorfman <dima@unixfreak.org>
To: Poul-Henning Kamp <phk@critter.freebsd.dk>
Cc: freebsd-gnats-submit@FreeBSD.ORG
Subject: Re: kern/26787: sysctl change request
Date: Mon, 28 May 2001 18:03:21 -0700
Poul-Henning Kamp <phk@critter.freebsd.dk> writes:
> In message <200105240330.f4O3U3b93675@freefall.freebsd.org>, Dima Dorfman writes:
> >The following reply was made to PR kern/26787; it has been noted by GNATS.
> > <phk@FreeBSD.org> writes:
> > > Synopsis: sysctl change request
> > >
> > > State-Changed-From-To: open->analyzed
> > > State-Changed-By: phk
> > > State-Changed-When: Wed May 23 13:14:17 PDT 2001
> > > State-Changed-Why:
> > > This is a sensible wish, but unfortunately there are sysctl variables
> > > which take opaque data types so doing this in a general way may be tricky.
> >
> > I think the best way would be to have the individual handlers print
> > this message.
>
> I think that would be a needless replication of code.
>
> I think the feature is rather marginal in the first place, so I don't
> want to see 1000 lines of code in the kernel to implement it.
Either you misunderstood, or I'm missing something really, really big.
I meant that each *handler* should decide how to log it, not each
sysctl instance. The majority of sysctls in the system that anybody
is interested in are ints and strings, which are handled by
sysctl_handle_int and sysctl_handle_string, respectively.
I've attached a proof-of-concept patch to demonstrate what I mean; it
adds logging for sysctls defined with SYSCTL_INT, SYSCTL_LONG, and
SYSCTL_STRING, conditional on the kern.log_sysctl sysctl. This
comprises most, if not all, of the interesting[*] sysctls, and only
adds about 50 lines. At this point, it only logs the last component
of the sysctl (e.g., if the sysctl is "kern.hostname" it will only log
"hostname") because I couldn't find a routine which gave the full
name, and writing one wasn't necessary to demonstrate the concept.
Do you still think this would add too much bloat for too little gain?
If so, what approach do you suggest? I don't think having the sysctl
framework guess at the type of value would be considered "clean".
Thanks in advance,
Dima Dorfman
dima@unixfreak.org
[*] In this context, a sysctl is considered "interesting" if the
administrator of the system might want to know if it's written to.
E.g., "kern.hostname" is interesting, but "vfs.ffs.adjrefcnt" is not.
Index: kern/kern_sysctl.c
===================================================================
RCS file: /stl/src/FreeBSD/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.107
diff -u -r1.107 kern_sysctl.c
--- kern/kern_sysctl.c 2001/05/19 05:45:55 1.107
+++ kern/kern_sysctl.c 2001/05/29 00:48:21
@@ -48,13 +48,19 @@
#include <sys/sysctl.h>
#include <sys/malloc.h>
#include <sys/proc.h>
+#include <sys/syslog.h>
#include <sys/sysproto.h>
#include <vm/vm.h>
#include <vm/vm_extern.h>
+#include <machine/stdarg.h>
+
static MALLOC_DEFINE(M_SYSCTL, "sysctl", "sysctl internal magic");
static MALLOC_DEFINE(M_SYSCTLOID, "sysctloid", "sysctl dynamic oids");
+static int sysctl_want_log = 0;
+SYSCTL_INT(_kern, OID_AUTO, log_sysctl, CTLFLAG_RW, &sysctl_want_log, 0, "");
+
/*
* Locking and stats
*/
@@ -707,6 +713,24 @@
SYSCTL_NODE(_sysctl, 4, oidfmt, CTLFLAG_RD, sysctl_sysctl_oidfmt, "");
+void
+sysctl_log(struct sysctl_oid *oidp, struct sysctl_req *req,
+ const char *fmt, ...)
+{
+ char buf[128]; /* XXX */
+ va_list ap;
+
+ if (!sysctl_want_log || oidp->oid_kind & CTLFLAG_NOLOG)
+ return;
+
+ va_start(ap, fmt);
+ vsnprintf(buf, sizeof(buf), fmt, ap);
+ va_end(ap);
+ log(LOG_NOTICE, "pid %d (%s), uid %d, sysctl %s: %s\n", req->p->p_pid,
+ req->p->p_comm, req->p->p_ucred ? req->p->p_ucred->cr_uid : -1,
+ oidp->oid_name, buf);
+}
+
/*
* Default "handler" functions.
*/
@@ -722,6 +746,7 @@
sysctl_handle_int(SYSCTL_HANDLER_ARGS)
{
int error = 0;
+ int oldval;
if (arg1)
error = SYSCTL_OUT(req, arg1, sizeof(int));
@@ -733,8 +758,13 @@
if (!arg1)
error = EPERM;
- else
+ else {
+ oldval = *(int *)arg1;
error = SYSCTL_IN(req, arg1, sizeof(int));
+ if (!error)
+ sysctl_log(oidp, req, "0x%x -> 0x%x",
+ oldval, *(int *)arg1);
+ }
return (error);
}
@@ -746,6 +776,7 @@
sysctl_handle_long(SYSCTL_HANDLER_ARGS)
{
int error = 0;
+ long oldval;
if (!arg1)
return (EINVAL);
@@ -754,7 +785,10 @@
if (error || !req->newptr)
return (error);
+ oldval = *(long *)arg1;
error = SYSCTL_IN(req, arg1, sizeof(long));
+ if (!error)
+ sysctl_log(oidp, req, "0x%lx -> 0x%lx", oldval, *(long *)arg1);
return (error);
}
@@ -769,6 +803,8 @@
sysctl_handle_string(SYSCTL_HANDLER_ARGS)
{
int error=0;
+ char *oldval;
+ size_t oldvalsize;
error = SYSCTL_OUT(req, arg1, strlen((char *)arg1)+1);
@@ -778,9 +814,17 @@
if ((req->newlen - req->newidx) >= arg2) {
error = EINVAL;
} else {
+ oldvalsize = strlen((char*)arg1) + 1;
+ oldval = malloc(oldvalsize, M_TEMP, M_WAITOK);
+ bcopy(arg1, oldval, oldvalsize);
arg2 = (req->newlen - req->newidx);
error = SYSCTL_IN(req, arg1, arg2);
((char *)arg1)[arg2] = '\0';
+ if (!error)
+ sysctl_log(oidp, req, "%s -> %s",
+ *oldval == '\0' ? "(blank)" : oldval,
+ *(char *)arg1 == '\0' ? "(blank)" : (char *)arg1);
+ free(oldval, M_TEMP);
}
return (error);
Index: sys/sysctl.h
===================================================================
RCS file: /stl/src/FreeBSD/src/sys/sys/sysctl.h,v
retrieving revision 1.92
diff -u -r1.92 sysctl.h
--- sys/sysctl.h 2001/05/19 05:45:55 1.92
+++ sys/sysctl.h 2001/05/29 00:48:21
@@ -82,6 +82,7 @@
#define CTLFLAG_SECURE 0x08000000 /* Permit set only if securelevel<=0 */
#define CTLFLAG_PRISON 0x04000000 /* Prisoned roots can fiddle */
#define CTLFLAG_DYN 0x02000000 /* Dynamic oid - can be freed */
+#define CTLFLAG_NOLOG 0x01000000 /* Never log writes for this oid */
/*
* USE THIS instead of a hardwired number from the categories below
@@ -134,6 +135,8 @@
#define SYSCTL_IN(r, p, l) (r->newfunc)(r, p, l)
#define SYSCTL_OUT(r, p, l) (r->oldfunc)(r, p, l)
+
+void sysctl_log(struct sysctl_oid *, struct sysctl_req *, const char *, ...);
int sysctl_handle_int(SYSCTL_HANDLER_ARGS);
int sysctl_handle_long(SYSCTL_HANDLER_ARGS);
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200105290110.f4T1A3u68170>