I would like to request a change to sysctl. When you change a sysctl varible, I would like it to either make a note of it in syslog or perhaps something else. Maybe have a varb in defaults/rc.conf for something such as, SYSCTL_LOG_CHANGES=YES (This would help tracking down a problem or maybe it can be useful for other features?)
State Changed From-To: open->analyzed 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. Either way: we need somebody to write a patch for it, you may want to put it up on http://phantom.cris.net/freebsd/projects/
<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. The sysctl framework could provide a sysctl_log routine which would call the appropriate *printf if kern.log_sysctls (example name) is set. The standard handlers for int, string, and friends could be changed to do this, which would cover a lot of cases. It may also be feasible to add a CTLFLAG_LOGWRITE which would just cause a "sysctl kern.blah was written to" message. Thoughts? I'd be willing to implement this stuff. Dima Dorfman dima@unixfreak.org > > Either way: we need somebody to write a patch for it, you may want > to put it up on http://phantom.cris.net/freebsd/projects/ > > > http://www.FreeBSD.org/cgi/query-pr.cgi?pr=26787 > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-bugs" in the body of the message >
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);
State Changed From-To: analyzed->open
Responsible Changed From-To: freebsd-bugs->dd I'm working on this.
Responsible Changed From-To: dd->freebsd-bugs Assignee confirms that he is not likely to look at this for a while.