Sun, 02 Jun 2024 13:07:48 +0200
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);