svn commit: r278494 - head/sys/kern
Konstantin Belousov
kostikbel at gmail.com
Tue Feb 10 08:32:28 UTC 2015
On Tue, Feb 10, 2015 at 04:34:40AM +0000, Rui Paulo wrote:
> Author: rpaulo
> Date: Tue Feb 10 04:34:39 2015
> New Revision: 278494
> URL: https://svnweb.freebsd.org/changeset/base/278494
>
> Log:
> Sanitise the coredump file names sent to devd.
>
> While there, add a sysctl to turn this feature off as requested by
> kib at .
>
> Modified:
> head/sys/kern/kern_sig.c
>
> Modified: head/sys/kern/kern_sig.c
> ==============================================================================
> --- head/sys/kern/kern_sig.c Tue Feb 10 03:34:42 2015 (r278493)
> +++ head/sys/kern/kern_sig.c Tue Feb 10 04:34:39 2015 (r278494)
> @@ -42,6 +42,7 @@ __FBSDID("$FreeBSD$");
> #include "opt_core.h"
>
> #include <sys/param.h>
> +#include <sys/ctype.h>
> #include <sys/systm.h>
> #include <sys/signalvar.h>
> #include <sys/vnode.h>
> @@ -179,6 +180,10 @@ static int set_core_nodump_flag = 0;
> SYSCTL_INT(_kern, OID_AUTO, nodump_coredump, CTLFLAG_RW, &set_core_nodump_flag,
> 0, "Enable setting the NODUMP flag on coredump files");
>
> +static int coredump_devctl = 1;
> +SYSCTL_INT(_kern, OID_AUTO, coredump_devctl, CTLFLAG_RW, &coredump_devctl,
> + 0, "Generate a devctl notification when processes coredump");
> +
> /*
> * Signal properties and actions.
> * The array below categorizes the signals and their default actions
> @@ -3217,6 +3222,25 @@ out:
> return (0);
> }
>
> +static int
> +coredump_sanitise_path(const char *path)
> +{
> + size_t len, i;
> +
> + /*
> + * Only send a subset of ASCII to devd(8) because it
> + * might pass these strings to sh -c.
> + */
> + len = strlen(path);
> + for (i = 0; i < len; i++)
You may check for the nul byte, avoiding first iteration over the string.
> + if (!(isalpha(path[i]) || isdigit(path[i])) &&
> + path[i] != '/' && path[i] != '.' &&
> + path[i] != '-')
> + return (0);
> +
> + return (1);
> +}
> +
> /*
> * Dump a process' core. The main routine does some
> * policy checking, and creates the name of the coredump;
> @@ -3238,9 +3262,9 @@ coredump(struct thread *td)
> void *rl_cookie;
> off_t limit;
> int compress;
> - char *data = NULL;
> - size_t len;
> + char data[MAXPATHLEN * 2 + 16]; /* space for devctl notification */
As I explained in another mail, I did not suggested doing this.
> char *fullpath, *freepath = NULL;
> + size_t len;
>
> #ifdef COMPRESS_USER_CORES
> compress = compress_user_cores;
> @@ -3332,30 +3356,29 @@ close:
> * Notify the userland helper that a process triggered a core dump.
> * This allows the helper to run an automated debugging session.
> */
> - len = MAXPATHLEN * 2 + 5 /* comm= */ + 5 /* core= */ + 1;
> - data = malloc(len, M_TEMP, M_NOWAIT);
> - if (data == NULL)
> + if (coredump_devctl == 0)
> goto out;
> if (vn_fullpath_global(td, p->p_textvp, &fullpath, &freepath) != 0)
> goto out;
> - snprintf(data, len, "comm=%s", fullpath);
> - if (freepath != NULL) {
> - free(freepath, M_TEMP);
> - freepath = NULL;
> + if (!coredump_sanitise_path(fullpath))
> + goto out;
> + snprintf(data, sizeof(data), "comm=%s ", fullpath);
> + free(freepath, M_TEMP);
> + freepath = NULL;
> + if (vn_fullpath_global(td, vp, &fullpath, &freepath) != 0) {
> + printf("could not find coredump\n");
This printf should be removed.
> + goto out;
> }
> - if (vn_fullpath_global(td, vp, &fullpath, &freepath) != 0)
> + if (!coredump_sanitise_path(fullpath))
> goto out;
> - snprintf(data, len, "%s core=%s", data, fullpath);
> + strlcat(data, "core=", sizeof(data));
> + len = strlcat(data, fullpath, sizeof(data));
> devctl_notify("kernel", "signal", "coredump", data);
> - free(name, M_TEMP);
> out:
> #ifdef AUDIT
> audit_proc_coredump(td, name, error);
> #endif
> - if (freepath != NULL)
> - free(freepath, M_TEMP);
> - if (data != NULL)
> - free(data, M_TEMP);
> + free(freepath, M_TEMP);
> free(name, M_TEMP);
> return (error);
> }
And there is double-free somewhere.
More information about the svn-src-all
mailing list