[PATCH] Finish the task 'Validate coredump format string'

[Tiwei Bie]

On Sun, Mar 22, 2015 at 08:06:55PM +0800, Tiwei Bie wrote:
> On Sun, Mar 22, 2015 at 01:38:22PM +0200, Konstantin Belousov wrote:
> > On Sun, Mar 22, 2015 at 07:25:55PM +0800, Tiwei Bie wrote:
> > > On Sun, Mar 22, 2015 at 11:14:01AM +0100, Mateusz Guzik wrote:
> > > > On Sun, Mar 22, 2015 at 05:19:40PM +0800, Tiwei Bie wrote:
> 
[..]
> I will submit my new patch later.
> 

Here is my new patch:

---
 sys/kern/kern_sig.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 64 insertions(+), 9 deletions(-)

diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c
index 8410d9d..5162ab6 100644
--- a/sys/kern/kern_sig.c
+++ b/sys/kern/kern_sig.c
@@ -3094,21 +3094,81 @@ static int compress_user_cores = 0;
  */
 #define	corefilename_lock	allproc_lock
 
-static char corefilename[MAXPATHLEN] = {"%N.core"};
+static char corefilename[MAXPATHLEN] = "%N.core";
+
+static bool
+corefilename_check(const char *format)
+{
+	int i, counti;
+
+	counti = 0;
+	for (i = 0; i < MAXPATHLEN && format[i] != '\0'; i++) {
+		if (format[i] == '%') {
+			i++;
+			switch (format[i]) {
+			case 'I':
+				counti++;
+				if (counti > 1)
+					return (false);
+			case '%':
+			case 'H':
+			case 'N':
+			case 'P':
+			case 'U':
+				break;
+			default:
+				return (false);
+			}
+		}
+	}
+
+	if (i == MAXPATHLEN)
+		return (false);
+
+	return (true);
+}
+
+static void
+corefilename_init(void *arg)
+{
+	char *format;
+
+	format = kern_getenv("kern.corefile");
+	if (format != NULL && corefilename_check(format))
+		strcpy(corefilename, format);
+}
+SYSINIT(corefilename, SI_SUB_KMEM, SI_ORDER_FIRST, corefilename_init, 0);
 
 static int
 sysctl_kern_corefile(SYSCTL_HANDLER_ARGS)
 {
+	char *format;
 	int error;
 
+	format = malloc(MAXPATHLEN, M_TEMP, M_WAITOK);
+
+	sx_slock(&corefilename_lock);
+	strcpy(format, corefilename);
+	sx_sunlock(&corefilename_lock);
+
+	error = sysctl_handle_string(oidp, format, MAXPATHLEN, req);
+	if (error || !req->newptr || strcmp(format, corefilename) == 0)
+		goto out;
+
+	if (!corefilename_check(format)) {
+		error = EINVAL;
+		goto out;
+	}
+
 	sx_xlock(&corefilename_lock);
-	error = sysctl_handle_string(oidp, corefilename, sizeof(corefilename),
-	    req);
+	strcpy(corefilename, format);
 	sx_xunlock(&corefilename_lock);
 
+out:
+	free(format, M_TEMP);
 	return (error);
 }
-SYSCTL_PROC(_kern, OID_AUTO, corefile, CTLTYPE_STRING | CTLFLAG_RWTUN |
+SYSCTL_PROC(_kern, OID_AUTO, corefile, CTLTYPE_STRING | CTLFLAG_RW |
     CTLFLAG_MPSAFE, 0, 0, sysctl_kern_corefile, "A",
     "Process corefile name format string");
 
@@ -3170,11 +3230,6 @@ corefile_open(const char *comm, uid_t uid, pid_t pid, struct thread *td,
 			case 'U':	/* user id */
 				sbuf_printf(&sb, "%u", uid);
 				break;
-			default:
-				log(LOG_ERR,
-				    "Unknown format character %c in "
-				    "corename `%s'\n", format[i], format);
-				break;
 			}
 			break;
 		default:
-- 
2.1.2

Best regards,
Tiwei Bie