Date: Fri, 3 Aug 2018 19:24:05 +0000 (UTC) From: Bryan Drewery <bdrewery@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r337272 - head/sys/dev/filemon Message-ID: <201808031924.w73JO5iM032770@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: bdrewery Date: Fri Aug 3 19:24:04 2018 New Revision: 337272 URL: https://svnweb.freebsd.org/changeset/base/337272 Log: Fix some filemon path logging issues. - Properly handle snprintf return value for truncation and avoid overflowing the later write with the bogus length. - Increase the msgbufr size to handle a rename of 2 full files. The larger allocation causes a slight performance hit which will be mitigated in the future. A rewrite with sbufs will likely be done as well. Reported by: Ilja Van Sprundel <ivansprundel@ioactive.com> MFC after: 2 weeks Approved by: so (gtetlow) Reviewed by: kib Sponsored by: Dell EMC Differential Revision: https://reviews.freebsd.org/D16098 Modified: head/sys/dev/filemon/filemon.c head/sys/dev/filemon/filemon_wrapper.c Modified: head/sys/dev/filemon/filemon.c ============================================================================== --- head/sys/dev/filemon/filemon.c Fri Aug 3 19:11:00 2018 (r337271) +++ head/sys/dev/filemon/filemon.c Fri Aug 3 19:24:04 2018 (r337272) @@ -88,7 +88,7 @@ struct filemon { struct ucred *cred; /* Credential of tracer. */ char fname1[MAXPATHLEN]; /* Temporary filename buffer. */ char fname2[MAXPATHLEN]; /* Temporary filename buffer. */ - char msgbufr[1024]; /* Output message buffer. */ + char msgbufr[2*MAXPATHLEN + 100]; /* Output message buffer. */ int error; /* Log write error, returned on close(2). */ u_int refcnt; /* Pointer reference count. */ u_int proccnt; /* Process count. */ @@ -200,8 +200,8 @@ filemon_write_header(struct filemon *filemon) "# filemon version %d\n# Target pid %d\n# Start %ju.%06ju\nV %d\n", FILEMON_VERSION, curproc->p_pid, (uintmax_t)now.tv_sec, (uintmax_t)now.tv_usec, FILEMON_VERSION); - - filemon_output(filemon, filemon->msgbufr, len); + if (len < sizeof(filemon->msgbufr)) + filemon_output(filemon, filemon->msgbufr, len); } /* @@ -268,7 +268,8 @@ filemon_close_log(struct filemon *filemon) "# Stop %ju.%06ju\n# Bye bye\n", (uintmax_t)now.tv_sec, (uintmax_t)now.tv_usec); - filemon_output(filemon, filemon->msgbufr, len); + if (len < sizeof(filemon->msgbufr)) + filemon_output(filemon, filemon->msgbufr, len); fp = filemon->fp; filemon->fp = NULL; Modified: head/sys/dev/filemon/filemon_wrapper.c ============================================================================== --- head/sys/dev/filemon/filemon_wrapper.c Fri Aug 3 19:11:00 2018 (r337271) +++ head/sys/dev/filemon/filemon_wrapper.c Fri Aug 3 19:24:04 2018 (r337272) @@ -39,6 +39,11 @@ __FBSDID("$FreeBSD$"); #include <sys/sysent.h> #include <sys/vnode.h> +#include <machine/stdarg.h> + +static void filemon_output_event(struct filemon *filemon, const char *fmt, ...) + __printflike(2, 3); + static eventhandler_tag filemon_exec_tag; static eventhandler_tag filemon_exit_tag; static eventhandler_tag filemon_fork_tag; @@ -71,11 +76,25 @@ filemon_output(struct filemon *filemon, char *msg, siz filemon->error = error; } +static void +filemon_output_event(struct filemon *filemon, const char *fmt, ...) +{ + va_list ap; + size_t len; + + va_start(ap, fmt); + len = vsnprintf(filemon->msgbufr, sizeof(filemon->msgbufr), fmt, ap); + va_end(ap); + /* The event is truncated but still worth logging. */ + if (len >= sizeof(filemon->msgbufr)) + len = sizeof(filemon->msgbufr) - 1; + filemon_output(filemon, filemon->msgbufr, len); +} + static int filemon_wrapper_chdir(struct thread *td, struct chdir_args *uap) { int error, ret; - size_t len; struct filemon *filemon; if ((ret = sys_chdir(td, uap)) == 0) { @@ -86,11 +105,8 @@ filemon_wrapper_chdir(struct thread *td, struct chdir_ goto copyfail; } - len = snprintf(filemon->msgbufr, - sizeof(filemon->msgbufr), "C %d %s\n", + filemon_output_event(filemon, "C %d %s\n", curproc->p_pid, filemon->fname1); - - filemon_output(filemon, filemon->msgbufr, len); copyfail: filemon_drop(filemon); } @@ -104,16 +120,12 @@ filemon_event_process_exec(void *arg __unused, struct struct image_params *imgp) { struct filemon *filemon; - size_t len; if ((filemon = filemon_proc_get(p)) != NULL) { - len = snprintf(filemon->msgbufr, - sizeof(filemon->msgbufr), "E %d %s\n", + filemon_output_event(filemon, "E %d %s\n", p->p_pid, imgp->execpath != NULL ? imgp->execpath : "<unknown>"); - filemon_output(filemon, filemon->msgbufr, len); - /* If the credentials changed then cease tracing. */ if (imgp->newcred != NULL && imgp->credential_setid && @@ -140,7 +152,6 @@ static void _filemon_wrapper_openat(struct thread *td, char *upath, int flags, int fd) { int error; - size_t len; struct file *fp; struct filemon *filemon; char *atpath, *freepath; @@ -166,10 +177,8 @@ _filemon_wrapper_openat(struct thread *td, char *upath * XXX: This may be able to come out with * the namecache lookup now. */ - len = snprintf(filemon->msgbufr, - sizeof(filemon->msgbufr), "A %d %s\n", + filemon_output_event(filemon, "A %d %s\n", curproc->p_pid, filemon->fname1); - filemon_output(filemon, filemon->msgbufr, len); /* * Try to resolve the path from the vnode using the * namecache. It may be inaccurate, but better @@ -187,19 +196,15 @@ _filemon_wrapper_openat(struct thread *td, char *upath * to also output an R to distinguish from * O_WRONLY. */ - len = snprintf(filemon->msgbufr, - sizeof(filemon->msgbufr), "R %d %s%s%s\n", + filemon_output_event(filemon, "R %d %s%s%s\n", curproc->p_pid, atpath, atpath[0] != '\0' ? "/" : "", filemon->fname1); - filemon_output(filemon, filemon->msgbufr, len); } - len = snprintf(filemon->msgbufr, - sizeof(filemon->msgbufr), "%c %d %s%s%s\n", + filemon_output_event(filemon, "%c %d %s%s%s\n", (flags & O_ACCMODE) ? 'W':'R', curproc->p_pid, atpath, atpath[0] != '\0' ? "/" : "", filemon->fname1); - filemon_output(filemon, filemon->msgbufr, len); copyfail: filemon_drop(filemon); if (fp != NULL) @@ -234,7 +239,6 @@ static int filemon_wrapper_rename(struct thread *td, struct rename_args *uap) { int error, ret; - size_t len; struct filemon *filemon; if ((ret = sys_rename(td, uap)) == 0) { @@ -247,11 +251,8 @@ filemon_wrapper_rename(struct thread *td, struct renam goto copyfail; } - len = snprintf(filemon->msgbufr, - sizeof(filemon->msgbufr), "M %d '%s' '%s'\n", + filemon_output_event(filemon, "M %d '%s' '%s'\n", curproc->p_pid, filemon->fname1, filemon->fname2); - - filemon_output(filemon, filemon->msgbufr, len); copyfail: filemon_drop(filemon); } @@ -264,7 +265,6 @@ static void _filemon_wrapper_link(struct thread *td, char *upath1, char *upath2) { struct filemon *filemon; - size_t len; int error; if ((filemon = filemon_proc_get(curproc)) != NULL) { @@ -276,11 +276,8 @@ _filemon_wrapper_link(struct thread *td, char *upath1, goto copyfail; } - len = snprintf(filemon->msgbufr, - sizeof(filemon->msgbufr), "L %d '%s' '%s'\n", + filemon_output_event(filemon, "L %d '%s' '%s'\n", curproc->p_pid, filemon->fname1, filemon->fname2); - - filemon_output(filemon, filemon->msgbufr, len); copyfail: filemon_drop(filemon); } @@ -322,15 +319,12 @@ filemon_wrapper_linkat(struct thread *td, struct linka static void filemon_event_process_exit(void *arg __unused, struct proc *p) { - size_t len; struct filemon *filemon; if ((filemon = filemon_proc_get(p)) != NULL) { - len = snprintf(filemon->msgbufr, sizeof(filemon->msgbufr), - "X %d %d %d\n", p->p_pid, p->p_xexit, p->p_xsig); + filemon_output_event(filemon, "X %d %d %d\n", + p->p_pid, p->p_xexit, p->p_xsig); - filemon_output(filemon, filemon->msgbufr, len); - /* * filemon_untrack_processes() may have dropped this p_filemon * already while in filemon_proc_get() before acquiring the @@ -350,7 +344,6 @@ static int filemon_wrapper_unlink(struct thread *td, struct unlink_args *uap) { int error, ret; - size_t len; struct filemon *filemon; if ((ret = sys_unlink(td, uap)) == 0) { @@ -361,11 +354,8 @@ filemon_wrapper_unlink(struct thread *td, struct unlin goto copyfail; } - len = snprintf(filemon->msgbufr, - sizeof(filemon->msgbufr), "D %d %s\n", + filemon_output_event(filemon, "D %d %s\n", curproc->p_pid, filemon->fname1); - - filemon_output(filemon, filemon->msgbufr, len); copyfail: filemon_drop(filemon); } @@ -378,15 +368,11 @@ static void filemon_event_process_fork(void *arg __unused, struct proc *p1, struct proc *p2, int flags __unused) { - size_t len; struct filemon *filemon; if ((filemon = filemon_proc_get(p1)) != NULL) { - len = snprintf(filemon->msgbufr, - sizeof(filemon->msgbufr), "F %d %d\n", + filemon_output_event(filemon, "F %d %d\n", p1->p_pid, p2->p_pid); - - filemon_output(filemon, filemon->msgbufr, len); /* * filemon_untrack_processes() or
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201808031924.w73JO5iM032770>