Hi!

Some guys using dnsmasq in virtual machines and OpenStack use custom 
dhcp_script to manage leases of clients.
However they complain if there is anything wrong with them, then are just told 
broken pipe and no information.

We understand it should not produce any output under normal operation. But it 
would be really helpful if at least anything was visible in logs. Especially 
for errors happening under rare circumstances.
I have prepared patch to forward events from helper. It prevents SIGPIPE 
receiving if script does write anything. And logs it from dnsmasq.
It seems very handy to me.

It was not simple to forward it to main log. I would like opinions if it is 
useful or dangerous.
Do you consider it worth merging Simon?

Best Regards,
Petr
--
Petr Menšík
Software Engineer
Red Hat, http://www.redhat.com/
email: pemen...@redhat.com  PGP: 65C6C973

From a7b740b12525b085efcab5d9c8a8260df2ae33dd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Fri, 24 Mar 2017 18:23:53 +0100
Subject: [PATCH] Log output of dhcp_script called from handler

---
 src/dnsmasq.c | 13 ++++++++++++
 src/dnsmasq.h | 49 +++++++++++++++++++++++-----------------------
 src/helper.c  | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 98 insertions(+), 27 deletions(-)

diff --git a/src/dnsmasq.c b/src/dnsmasq.c
index d2cc7cc..e2e42bf 100644
--- a/src/dnsmasq.c
+++ b/src/dnsmasq.c
@@ -1315,6 +1315,19 @@ static void async_event(int pipe, time_t now)
 		  daemon->lease_change_command, strerror(ev.data));
 	break;
 
+#if defined(HAVE_SCRIPT) && defined(HAVE_DHCP)
+      case EVENT_SCRIPT_ERR:
+	if (ev.data != 0)
+	  my_syslog(LOG_ERR, _("dhcp-script output: %s: %s"),
+                    strerror(ev.data), msg ? msg : "");
+        else
+	  my_syslog(LOG_DEBUG, _("dhcp-script output: %s"),
+                    msg ? msg : "");
+        free(msg);
+	msg = NULL;
+	break;
+#endif
+
 	/* necessary for fatal errors in helper */
       case EVENT_USER_ERR:
       case EVENT_DIE:
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index 6b44e53..f51ae3b 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -145,30 +145,31 @@ struct event_desc {
   int event, data, msg_sz;
 };
 
-#define EVENT_RELOAD    1
-#define EVENT_DUMP      2
-#define EVENT_ALARM     3
-#define EVENT_TERM      4
-#define EVENT_CHILD     5
-#define EVENT_REOPEN    6
-#define EVENT_EXITED    7
-#define EVENT_KILLED    8
-#define EVENT_EXEC_ERR  9
-#define EVENT_PIPE_ERR  10
-#define EVENT_USER_ERR  11
-#define EVENT_CAP_ERR   12
-#define EVENT_PIDFILE   13
-#define EVENT_HUSER_ERR 14
-#define EVENT_GROUP_ERR 15
-#define EVENT_DIE       16
-#define EVENT_LOG_ERR   17
-#define EVENT_FORK_ERR  18
-#define EVENT_LUA_ERR   19
-#define EVENT_TFTP_ERR  20
-#define EVENT_INIT      21
-#define EVENT_NEWADDR   22
-#define EVENT_NEWROUTE  23
-#define EVENT_TIME_ERR  24
+#define EVENT_RELOAD     1
+#define EVENT_DUMP       2
+#define EVENT_ALARM      3
+#define EVENT_TERM       4
+#define EVENT_CHILD      5
+#define EVENT_REOPEN     6
+#define EVENT_EXITED     7
+#define EVENT_KILLED     8
+#define EVENT_EXEC_ERR   9
+#define EVENT_PIPE_ERR   10
+#define EVENT_USER_ERR   11
+#define EVENT_CAP_ERR    12
+#define EVENT_PIDFILE    13
+#define EVENT_HUSER_ERR  14
+#define EVENT_GROUP_ERR  15
+#define EVENT_DIE        16
+#define EVENT_LOG_ERR    17
+#define EVENT_FORK_ERR   18
+#define EVENT_LUA_ERR    19
+#define EVENT_TFTP_ERR   20
+#define EVENT_INIT       21
+#define EVENT_NEWADDR    22
+#define EVENT_NEWROUTE   23
+#define EVENT_TIME_ERR   24
+#define EVENT_SCRIPT_ERR 25
 
 /* Exit codes. */
 #define EC_GOOD        0
diff --git a/src/helper.c b/src/helper.c
index 2b8164b..b777f77 100644
--- a/src/helper.c
+++ b/src/helper.c
@@ -14,6 +14,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
+#include <stdio.h>
 #include "dnsmasq.h"
 
 #ifdef HAVE_SCRIPT
@@ -77,6 +78,33 @@ struct script_data
 static struct script_data *buf = NULL;
 static size_t bytes_in_buf = 0, buf_size = 0;
 
+/* Send output of script to main process, line by line. */
+static int
+helper_stdin_resend(int event_fd, int outfd)
+{
+  FILE *fp;
+  size_t len;
+
+  fp = fdopen(outfd, "r");
+  if (!fp)
+    return 0;
+  while (fgets(daemon->packet, daemon->packet_buff_sz, fp))
+    {
+      /* do not include new lines, log will append them */
+      len = strlen(daemon->packet);
+      if (len > 0)
+        {
+          --len;
+          if (daemon->packet[len] == '\n')
+            daemon->packet[len] = 0;
+        }
+      send_event(event_fd, EVENT_SCRIPT_ERR, 0, daemon->packet);
+    }
+  fclose(fp);
+  return 1;
+}
+
+
 int create_helper(int event_fd, int err_fd, uid_t uid, gid_t gid, long max_fd)
 {
   pid_t pid;
@@ -135,7 +163,7 @@ int create_helper(int event_fd, int err_fd, uid_t uid, gid_t gid, long max_fd)
 	max_fd != STDIN_FILENO && max_fd != pipefd[0] && 
 	max_fd != event_fd && max_fd != err_fd)
       close(max_fd);
-  
+
 #ifdef HAVE_LUASCRIPT
   if (daemon->luascript)
     {
@@ -189,6 +217,8 @@ int create_helper(int event_fd, int err_fd, uid_t uid, gid_t gid, long max_fd)
       unsigned char *buf = (unsigned char *)daemon->namebuff;
       unsigned char *end, *extradata, *alloc_buff = NULL;
       int is6, err = 0;
+      int pipeout[2];
+      int popened;
 
       free(alloc_buff);
       
@@ -472,21 +502,39 @@ int create_helper(int event_fd, int err_fd, uid_t uid, gid_t gid, long max_fd)
       if (!daemon->lease_change_command)
 	continue;
 
+      popened = (pipe(pipeout) != -1);
+      if (!popened)
+        send_event(event_fd, EVENT_SCRIPT_ERR, errno, "creation of stdout pipe failed");
+
       /* possible fork errors are all temporary resource problems */
       while ((pid = fork()) == -1 && (errno == EAGAIN || errno == ENOMEM))
 	sleep(2);
 
       if (pid == -1)
-	continue;
+        {
+          if (popened)
+            {
+              close(pipeout[0]);
+              close(pipeout[1]);
+            }
+	  continue;
+        }
       
       /* wait for child to complete */
       if (pid != 0)
 	{
+	  close(pipeout[1]);
+
 	  /* reap our children's children, if necessary */
 	  while (1)
 	    {
 	      int status;
-	      pid_t rc = wait(&status);
+	      pid_t rc;
+
+	      if (popened)
+	        helper_stdin_resend(event_fd, pipeout[0]);
+
+	      rc = wait(&status);
 	      
 	      if (rc == pid)
 		{
@@ -505,6 +553,15 @@ int create_helper(int event_fd, int err_fd, uid_t uid, gid_t gid, long max_fd)
 	  continue;
 	}
       
+      /* map output of script to pipeout */
+      if (popened)
+        {
+          close(pipeout[0]);
+          dup2(pipeout[1], STDOUT_FILENO);
+          dup2(pipeout[1], STDERR_FILENO);
+          close(pipeout[1]);
+        }
+
       if (data.action != ACTION_TFTP && data.action != ACTION_ARP)
 	{
 #ifdef HAVE_DHCP6
-- 
2.9.3

_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

Reply via email to