[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    [BusyBox] SIGSEGV with klogd
From:       William Barsse <wbarsse () pfn ! com>
Date:       2004-07-27 18:12:37
Message-ID: 41069B15.5000604 () pfn ! com
[Download RAW message or body]

Hi all,

I've run into an issue with klogd causing a segv. It seems to be due to 
the way the syslog call is being made with data read from the kernel's 
log buffer. The call looks like this

syslog(priority, stuff);

however this breaks if 'stuff' contains a % character since it is 
regarded as a format string. Calling it this way solves things :

syslog(priority, "%s", stuff);

I've attached a mini-patch that corrects this 
(busybox-1.00-rc2-klogd-simple.patch).

Since I was lurking in the klogd code, I noticed a problem that could 
occur if the kernel buffer was longer than the buffer size (4096) used 
by klogd and a log entry 'straddles' the 4096 limit; it would be 
disregarded. I've put it another patch to address the issue, 
unfortunately it is somewhat larger (it also contains the fixed call to 
syslog). (busybox-1.00-rc2-klogd.patch)

Anyway, hope this is useful
- William

["busybox-1.00-rc2-klogd-simple.patch" (text/x-patch)]

diff -Naur busybox-1.00-rc2.orig/sysklogd/klogd.c busybox-1.00-rc2/sysklogd/klogd.c
--- busybox-1.00-rc2.orig/sysklogd/klogd.c	2004-06-25 07:23:03.000000000 -0400
+++ busybox-1.00-rc2/sysklogd/klogd.c	2004-07-27 13:55:23.000000000 -0400
@@ -51,8 +51,8 @@
 	exit(EXIT_SUCCESS);
 }
 
-static void doKlogd(const int console_log_level) __attribute__ ((noreturn));
-static void doKlogd(const int console_log_level)
+static void doKlogd(const unsigned console_log_level) __attribute__ ((noreturn));
+static void doKlogd(const unsigned console_log_level)
 {
 	int priority = LOG_INFO;
 	char log_buffer[4096];
@@ -71,7 +71,7 @@
 	klogctl(1, NULL, 0);
 
 	/* Set level of kernel console messaging.. */
-	if (console_log_level != -1)
+	if (console_log_level != -1L)
 		klogctl(8, NULL, console_log_level);
 
 	syslog(LOG_NOTICE, "klogd started: " BB_BANNER);
@@ -104,7 +104,7 @@
 			}
 			if (log_buffer[i] == '\n') {
 				log_buffer[i] = '\0';	/* zero terminate this message */
-				syslog(priority, start);
+				syslog(priority, "%s", start);
 				start = &log_buffer[i + 1];
 				priority = LOG_INFO;
 			}
@@ -118,7 +118,7 @@
 	/* no options, no getopt */
 	int opt;
 	int doFork = TRUE;
-	unsigned char console_log_level = -1;
+	unsigned console_log_level = -1L;
 
 	/* do normal option parsing */
 	while ((opt = getopt(argc, argv, "c:n")) > 0) {

["busybox-1.00-rc2-klogd.patch" (text/x-patch)]

diff -Naur busybox-1.00-rc2.orig/sysklogd/klogd.c busybox-1.00-rc2/sysklogd/klogd.c
--- busybox-1.00-rc2.orig/sysklogd/klogd.c	2004-06-25 07:23:03.000000000 -0400
+++ busybox-1.00-rc2/sysklogd/klogd.c	2004-07-27 13:51:21.000000000 -0400
@@ -51,14 +51,21 @@
 	exit(EXIT_SUCCESS);
 }
 
-static void doKlogd(const int console_log_level) __attribute__ ((noreturn));
-static void doKlogd(const int console_log_level)
+static int read_priority(char **pstart, int *ppriority, const char * const end);
+static char *read_entries(char *start, const char * const end);
+
+static void doKlogd(const unsigned console_log_level) __attribute__ ((noreturn));
+static void doKlogd(const unsigned console_log_level)
 {
-	int priority = LOG_INFO;
 	char log_buffer[4096];
-	int i, n, lastc;
-	char *start;
-
+	char *read_start = &log_buffer[0]; /* where to start reading in kernel buffer,
+										  within log_buffer */
+	int n;	/* number of bytes read from kernel buffer */
+	
+	/* do we need to find the beginning of the next entry before attempting to
+	 * read it, or are we already aligned ? */
+	int need_realign = 0; /* we start off "aligned" */
+		
 	openlog("kernel", 0, LOG_KERN);
 
 	/* Set up sig handlers */
@@ -71,46 +78,112 @@
 	klogctl(1, NULL, 0);
 
 	/* Set level of kernel console messaging.. */
-	if (console_log_level != -1)
+	if (console_log_level != -1L)
 		klogctl(8, NULL, console_log_level);
 
 	syslog(LOG_NOTICE, "klogd started: " BB_BANNER);
 
 	while (1) {
+		char *start; /* the start of the current entry */
+
+		/* point up to which we may read */
+		char const *end = &log_buffer[0] + sizeof(log_buffer);
+		
 		/* Use kernel syscalls */
-		memset(log_buffer, '\0', sizeof(log_buffer));
-		n = klogctl(2, log_buffer, sizeof(log_buffer));
+		memset(read_start, '\0', end - read_start);	/* is this necessary ? */
+		n = klogctl(2, read_start, end - read_start);
 		if (n < 0) {
 			if (errno == EINTR)
 				continue;
-			syslog(LOG_ERR, "klogd: Error return from sys_sycall: %d - %m.\n", errno);
+			syslog(LOG_ERR, "klogd: Error return from sys_sycall: %d - %m.", errno);
 			exit(EXIT_FAILURE);
 		}
 
-		/* klogctl buffer parsing modelled after code in dmesg.c */
-		start = &log_buffer[0];
-		lastc = '\0';
-		for (i = 0; i < n; i++) {
-			if (lastc == '\0' && log_buffer[i] == '<') {
-				priority = 0;
-				i++;
-				while (isdigit(log_buffer[i])) {
-					priority = priority * 10 + (log_buffer[i] - '0');
-					i++;
-				}
-				if (log_buffer[i] == '>')
-					i++;
-				start = &log_buffer[i];
+		/* update 'end' to reflect the amount of data actually read */
+		end = &log_buffer[0] + n;
+
+		/* align 'start' on beginning of next entry */
+		for (start = &log_buffer[0]; need_realign && (start < end); start++) {
+			if (*start == '\n') {
+					need_realign = 0;
 			}
-			if (log_buffer[i] == '\n') {
-				log_buffer[i] = '\0';	/* zero terminate this message */
-				syslog(priority, start);
-				start = &log_buffer[i + 1];
-				priority = LOG_INFO;
+		}
+		
+		/* log available entries */
+		start = read_entries(start,end);
+		
+		/* setup read_start for the next read from the kernel buffer */
+		if (start != NULL) { /* didn't find the end of the last entry in the
+								buffer move data between 'start' and 'end' to
+								the beginning of the log_buffer */
+			if (start == &log_buffer[0]) {
+				/* special case : entry is larger than our buffer, the
+				 * following read will not be aligned (since it will start with
+				 * the end of the too large entry), mark it as so */
+				syslog(LOG_WARNING, "klogd: log buffer is too small for kernel log entry, "
+					   "skipping it.");
+				need_realign = 1;
+				read_start = &log_buffer[0];
+			} else {
+				memmove(&log_buffer[0], start, end - start);
+				read_start = &log_buffer[0] + (end - start);
+			}
+		} else {
+			read_start = &log_buffer[0];
+		}
+	}
+}
+
+/* read in priority field if present, and update the 'start' pointer
+ * to have it point to the first character after the priority field
+ * return 0 if start is withing the buffer delimited by *pstart and
+ * end, 1 otherwise. Returns 0 even if no priority was read */
+static int
+read_priority(char **pstart, int *ppriority, const char * const end)
+{
+	char *start = *pstart;
+	int priority = 0;
+	if ((start < end) && (*start == '<')) {
+		start ++;
+		while ((start < end) && isdigit(*start)) {
+			priority = priority * 10 + (*start - '0');
+			start++;
+		}
+		if ((start < end) && (*start == '>')) {
+			*pstart = start + 1;
+			*ppriority = priority;
+		}
+	}
+	return start >= end;
+}
+
+/* read in log entries from 'start' to 'end'.
+ * return the address of the beginning of the next incomplete entry, or NULL if
+ *        the last entry was complete */
+static char *read_entries(char *start, const char * const end)
+{
+	int priority = LOG_INFO;	/* priority of the current entry */
+
+	/* loop over available entries */
+	for (; start < end ; start ++) {
+		char *eol;	/* the end of the current entry */
+
+		/* read in priority (if present) */
+		if (read_priority(&start, &priority, end)) return start;
+
+		/* find the next \n (marks the end of an entry), if any */
+		for (eol = start; eol < end; eol++) {
+			if (*eol == '\n') { /* found the end of the entry */
+				*eol = '\0';   /* mark it as an end of string */
+				syslog(priority, "%s", start); /* and log it */
+				start = eol; /* position 'start' at the end of the entry */
+				break;
 			}
-			lastc = log_buffer[i];
 		}
+		if (eol > end) return start; /* entry is incomplete, need to read more
+										data to complete it */
 	}
+	return NULL;	/* last entry was complete */
 }
 
 extern int klogd_main(int argc, char **argv)
@@ -118,7 +191,7 @@
 	/* no options, no getopt */
 	int opt;
 	int doFork = TRUE;
-	unsigned char console_log_level = -1;
+	unsigned console_log_level = -1L;
 
 	/* do normal option parsing */
 	while ((opt = getopt(argc, argv, "c:n")) > 0) {


_______________________________________________
busybox mailing list
busybox@mail.busybox.net
http://codepoet.org/mailman/listinfo/busybox


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic