handle eof in cgi_stdout_readevent to fix potential double free in cgi_event_finish

Sun, 02 Jun 2024 13:07:48 +0200

author
Olaf Wintermann <olaf.wintermann@gmail.com>
date
Sun, 02 Jun 2024 13:07:48 +0200
changeset 530
1e117b5d6710
parent 529
cd606400f0ba
child 531
9b15b1f72bef

handle eof in cgi_stdout_readevent to fix potential double free in cgi_event_finish

src/server/safs/cgi.c file | annotate | diff | comparison | revisions
src/server/safs/cgi.h file | annotate | diff | comparison | revisions
--- a/src/server/safs/cgi.c	Sun Jun 02 12:39:04 2024 +0200
+++ b/src/server/safs/cgi.c	Sun Jun 02 13:07:48 2024 +0200
@@ -313,6 +313,16 @@
         log_ereport(LOG_DEBUG, "cgi-send: req: %p debug_finished: 1 cgi_stdout_readevent events: %d", handler->parser->rq, handler->events);
     }
     
+    if(handler->cgi_eof) {
+        // cgi_eof will be set to true by cgi_read_output
+        // if it is true here, the cgi handling was finished by cgi_writeevent
+        // in that case, cgi_writeevent will finish the request processing
+        // and nothing needs to be done here
+        handler->wait_read = FALSE;
+        event->finish = NULL;
+        return 0;
+    }
+    
     event->finish = cgi_event_finish;
     handler->writeev->finish = NULL;
     CgiIOResult ret = cgi_read_output(handler, ev);
@@ -348,6 +358,10 @@
 int cgi_writeevent(EventHandler *ev, Event *event) {
     CGIHandler *handler = event->cookie;
     
+    if(handler->cgi_eof) {
+        log_ereport(LOG_DEBUG, "cgi-send: req: %p writeevent cgi_eof = TRUE", handler->parser->rq);
+    }
+    
     event->finish = cgi_event_finish;
     handler->readev->finish = NULL;
     CgiIOResult ret = cgi_read_output(handler, ev);
@@ -569,12 +583,13 @@
     
     handler->debug_finished = TRUE;
     
-    if(handler->result == REQ_ABORTED && handler->process.pid != 0) {
+    if(handler->result == REQ_ABORTED && handler->process.pid != 0 && handler->cgi_kill == 0) {
         log_ereport(LOG_FAILURE, "cgi-send: kill script: %s pid: %d", handler->path, (int)handler->process.pid);
         if(kill(handler->process.pid, SIGTERM)) {
             log_ereport(LOG_FAILURE, "cgi-send: pid: %d kill failed: %s", (int)handler->process.pid, strerror(errno));
         } else {
             log_ereport(LOG_DEBUG, "cgi-send: finish: req: %p kill %d successful", rq, (int)handler->process.pid);
+            handler->cgi_kill = SIGTERM;
         }
     }
     
@@ -592,29 +607,15 @@
         handler->poll_out = FALSE;
     }
     
-    /*
-    if(--handler->events > 0) {
-        if(handler->events == 1) {
-            if(handler->poll_out) {
-                // write event registered, however it will not be activated anymore
-                // we can safely remove the event
-                log_ereport(LOG_DEBUG, "cgi-send: req: %p finish: event: 1 remove-poll write", rq);
-                if(event_removepoll(ev, sn->csd)) {
-                    log_ereport(LOG_FAILURE, "cgi_event_finish: event_removepoll: %s", strerror(errno));
-                }
-            } else if(handler->cgi_eof && handler->wait_read) {
-                log_ereport(LOG_DEBUG, "cgi-send: req: %p finish: event: 1 remove-poll read", rq);
-                if(ev_remove_poll(ev, handler->process.out[0])) {
-                    log_ereport(LOG_FAILURE, "cgi_event_finish: ev_remove_poll: %s", strerror(errno));
-                }
-            } else {
-                return 0;
-            }
-        } else {
-            return 0;
+    if(handler->wait_read) {
+        // read event registered, however it will not be activated anymore
+        // (currently unsure if this can happen)
+        log_ereport(LOG_DEBUG, "cgi-send: req: %p finish: remove-poll read", rq);
+        if(ev_remove_poll(ev, handler->process.out[0])) {
+            log_ereport(LOG_FAILURE, "cgi_event_finish: req: %p ev_remove_poll: %s", rq, strerror(errno));
         }
+        handler->wait_read = FALSE;
     }
-    */
     
     log_ereport(LOG_DEBUG, "cgi-send: req: %p cgi_close", rq);
     
--- a/src/server/safs/cgi.h	Sun Jun 02 12:39:04 2024 +0200
+++ b/src/server/safs/cgi.h	Sun Jun 02 13:07:48 2024 +0200
@@ -140,6 +140,11 @@
      */
     WSBool cgi_eof;
     
+    /*
+     * Last kill signal sent to the cgi process
+     */
+    int cgi_kill;
+    
     WSBool debug_finished;
     
     /*

mercurial