d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix some filemon path logging issues.

Browse Source 
- 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
This commit is contained in:
bdrewery 2018-08-03 19:24:04 +00:00
parent 201eff6e02
commit e5c8c49fd8
2 changed files with 36 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
sys/dev/filemon/filemon.c
View File

@ -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;

76
sys/dev/filemon/filemon_wrapper.c
View File

@ -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, size_t len)
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_args *uap)
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 proc *p,
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, int flags, int fd)
* 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, int flags, int fd)
* 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 rename_args *uap)
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, char *upath2)
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,14 +319,11 @@ filemon_wrapper_linkat(struct thread *td, struct linkat_args *uap)
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(filemon, filemon->msgbufr, len);
filemon_output_event(filemon, "X %d %d %d\n",
p->p_pid, p->p_xexit, p->p_xsig);
/*
* filemon_untrack_processes() may have dropped this p_filemon
@ -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 unlink_args *uap)
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,16 +368,12 @@ 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
* filemon_ioctl(FILEMON_SET_PID) may have changed the parent's