Bug 1268

Summary: SIGSEGV after log_reopen
Product: unbound Reporter: samozrejmost
Component: serverAssignee: unbound team <unbound-team>
Severity: normal CC: cathya, wouter
Priority: P5    
Version: 1.6.2   
Hardware: x86_64   
OS: Linux   

Description samozrejmost 2017-05-20 01:04:23 CEST
This only happens in rare situations, but I'm reporting it anyway, as this crashes the server.

How to reproduce:
 - start the unbound daemon
 - change the ownership of the logfile so that the unbound process cannot write to it
 - issue 'unbound-control log_reopen' command
 - do something which would make the unbound process write to the logfile

What *I think* happens is that after the 'unbound-control log_reopen' command the code runs the log_init() function where:

	if(logfile && logfile != stderr)

'logfile' is then undefined, but not NULL. Then in the same function later:

	f = fopen(filename, "a");
	if(!f) {
		log_err("Could not open logfile %s: %s", filename, 

fopen() returns NULL (Permission denied), so then f=NULL, logfile is undefined. Any subsequent writes to the logfile (fprint(logfile,...)) then follow the undefined pointer 'logfile'.

Hope this helps!

PS: I came across this when I made a logrotate script which didn't handle the write rights properly.
Comment 1 Wouter Wijngaards 2017-05-22 09:24:04 CEST
Hi Samozremost,

Thanks for the report!  I tried to fix this by moving an =NULL before the fclose() event happens.  So that the other function has no issues, and encounters a NULL straight away.

Index: util/log.c
--- util/log.c	(revision 4176)
+++ util/log.c	(working copy)
@@ -103,8 +103,12 @@
-	if(logfile && logfile != stderr)
-		fclose(logfile);
+	if(logfile && logfile != stderr) {
+		FILE* cl = logfile;
+		logfile = NULL; /* set to NULL before it is closed, so that
+			other threads have a valid logfile or NULL */
+		fclose(cl);
+	}
 	if(logging_to_syslog) {
Comment 2 samozrejmost 2017-05-23 23:37:55 CEST
I applied the patch and it works for me (didn't test for more than 1 thread).

Thank you for your work!