From: Claudio Jeker Subject: smtpd: convert to imsg_get_fd() To: tech@openbsd.org Date: Thu, 18 Jan 2024 16:48:42 +0100 smtpd is the last program in base that needs to be converted to use imsg_get_fd(). It is also a little bit the reason for this conversion since with imsg_get_fd() the owner of the fd is will known and imsg_free() will soon start to close unclaimed fds. This prevents unexpected fd leaks. Now smtpd does a lot of fd passing but most of it is simple. In log_imsg() imsg.fd can no longer be inspected. I hope this is OK. This inspection in debug code violates the ownership principle. In my opinion logging the fd is not very helpful since fds are reused quickly. Please test if you use smtpd in more than the base config. -- :wq Claudio Index: control.c =================================================================== RCS file: /cvs/src/usr.sbin/smtpd/control.c,v diff -u -p -r1.130 control.c --- control.c 31 May 2023 16:51:46 -0000 1.130 +++ control.c 13 Dec 2023 13:19:05 -0000 @@ -107,7 +107,8 @@ control_imsg(struct mproc *p, struct ims c = tree_get(&ctl_conns, imsg->hdr.peerid); if (c == NULL) return; - m_compose(&c->mproc, IMSG_CTL_OK, 0, 0, imsg->fd, NULL, 0); + m_compose(&c->mproc, IMSG_CTL_OK, 0, 0, imsg_get_fd(imsg), + NULL, 0); return; case IMSG_STAT_INCREMENT: Index: enqueue.c =================================================================== RCS file: /cvs/src/usr.sbin/smtpd/enqueue.c,v diff -u -p -r1.121 enqueue.c --- enqueue.c 31 May 2023 16:51:46 -0000 1.121 +++ enqueue.c 16 Jan 2024 13:31:25 -0000 @@ -808,7 +808,7 @@ open_connection(void) errx(1, "unexpected imsg reply type"); } - fd = imsg.fd; + fd = imsg_get_fd(&imsg); imsg_free(&imsg); break; Index: lka.c =================================================================== RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v diff -u -p -r1.247 lka.c --- lka.c 14 Jun 2021 17:58:15 -0000 1.247 +++ lka.c 16 Jan 2024 13:19:04 -0000 @@ -47,7 +47,7 @@ static void lka_imsg(struct mproc *p, struct imsg *imsg) { struct table *table; - int ret; + int ret, fd; struct sockaddr_storage ss; struct userinfo userinfo; struct addrname addrname; @@ -305,7 +305,7 @@ lka_imsg(struct mproc *p, struct imsg *i return; case IMSG_LKA_OPEN_FORWARD: - lka_session_forward_reply(imsg->data, imsg->fd); + lka_session_forward_reply(imsg->data, imsg_get_fd(imsg)); return; case IMSG_LKA_AUTHENTICATE: @@ -351,7 +351,7 @@ lka_imsg(struct mproc *p, struct imsg *i m_add_string(p, procname); m_close(p); - lka_proc_forked(procname, subsystems, imsg->fd); + lka_proc_forked(procname, subsystems, imsg_get_fd(imsg)); return; case IMSG_LKA_PROCESSOR_ERRFD: @@ -359,8 +359,9 @@ lka_imsg(struct mproc *p, struct imsg *i m_get_string(&m, &procname); m_end(&m); - lka_proc_errfd(procname, imsg->fd); - shutdown(imsg->fd, SHUT_WR); + fd = imsg_get_fd(imsg); + lka_proc_errfd(procname, fd); + shutdown(fd, SHUT_WR); return; case IMSG_REPORT_SMTP_LINK_CONNECT: Index: mda.c =================================================================== RCS file: /cvs/src/usr.sbin/smtpd/mda.c,v diff -u -p -r1.146 mda.c --- mda.c 31 May 2023 16:51:46 -0000 1.146 +++ mda.c 16 Jan 2024 13:21:59 -0000 @@ -113,7 +113,7 @@ mda_imsg(struct mproc *p, struct imsg *i uint64_t reqid; size_t sz; char out[256], buf[LINE_MAX]; - int n; + int n, fd; enum lka_resp_status status; enum mda_resp_status mda_status; int mda_sysexit; @@ -196,7 +196,8 @@ mda_imsg(struct mproc *p, struct imsg *i s = tree_xget(&sessions, reqid); e = s->evp; - if (imsg->fd == -1) { + fd = imsg_get_fd(imsg); + if (fd == -1) { log_debug("debug: mda: cannot get message fd"); mda_queue_tempfail(e->id, "Cannot get message fd", @@ -208,11 +209,11 @@ mda_imsg(struct mproc *p, struct imsg *i log_debug("debug: mda: got message fd %d " "for session %016"PRIx64 " evpid %016"PRIx64, - imsg->fd, s->id, e->id); + fd, s->id, e->id); - if ((s->datafp = fdopen(imsg->fd, "r")) == NULL) { + if ((s->datafp = fdopen(fd, "r")) == NULL) { log_warn("warn: mda: fdopen"); - close(imsg->fd); + close(fd); mda_queue_tempfail(e->id, "fdopen failed", ESC_OTHER_MAIL_SYSTEM_STATUS); mda_log(e, "TempFail", "fdopen failed"); @@ -283,7 +284,8 @@ mda_imsg(struct mproc *p, struct imsg *i s = tree_xget(&sessions, reqid); e = s->evp; - if (imsg->fd == -1) { + fd = imsg_get_fd(imsg); + if (fd == -1) { log_warn("warn: mda: fail to retrieve mda fd"); mda_queue_tempfail(e->id, "Cannot get mda fd", ESC_OTHER_MAIL_SYSTEM_STATUS); @@ -294,10 +296,10 @@ mda_imsg(struct mproc *p, struct imsg *i log_debug("debug: mda: got mda fd %d " "for session %016"PRIx64 " evpid %016"PRIx64, - imsg->fd, s->id, s->evp->id); + fd, s->id, s->evp->id); - io_set_nonblocking(imsg->fd); - io_set_fd(s->io, imsg->fd); + io_set_nonblocking(fd); + io_set_fd(s->io, fd); io_set_write(s->io); return; @@ -315,8 +317,9 @@ mda_imsg(struct mproc *p, struct imsg *i * Grab last line of mda stdout/stderr if available. */ out[0] = '\0'; - if (imsg->fd != -1) - mda_getlastline(imsg->fd, out, sizeof(out)); + fd = imsg_get_fd(imsg); + if (fd != -1) + mda_getlastline(fd, out, sizeof(out)); /* * Choose between parent's description of error and Index: mproc.c =================================================================== RCS file: /cvs/src/usr.sbin/smtpd/mproc.c,v diff -u -p -r1.39 mproc.c --- mproc.c 14 Jun 2021 17:58:15 -0000 1.39 +++ mproc.c 16 Jan 2024 13:22:43 -0000 @@ -223,7 +223,7 @@ void m_forward(struct mproc *p, struct imsg *imsg) { imsg_compose(&p->imsgbuf, imsg->hdr.type, imsg->hdr.peerid, - imsg->hdr.pid, imsg->fd, imsg->data, + imsg->hdr.pid, imsg_get_fd(imsg), imsg->data, imsg->hdr.len - sizeof(imsg->hdr)); if (imsg->hdr.type != IMSG_STAT_DECREMENT && Index: mta_session.c =================================================================== RCS file: /cvs/src/usr.sbin/smtpd/mta_session.c,v diff -u -p -r1.150 mta_session.c --- mta_session.c 3 Jan 2024 08:11:15 -0000 1.150 +++ mta_session.c 16 Jan 2024 13:25:06 -0000 @@ -269,7 +269,7 @@ mta_session_imsg(struct mproc *p, struct struct msg m; uint64_t reqid; const char *name; - int status; + int status, fd; struct stat sb; switch (imsg->hdr.type) { @@ -279,14 +279,15 @@ mta_session_imsg(struct mproc *p, struct m_get_id(&m, &reqid); m_end(&m); + fd = imsg_get_fd(imsg); s = mta_tree_pop(&wait_fd, reqid); if (s == NULL) { - if (imsg->fd != -1) - close(imsg->fd); + if (fd != -1) + close(fd); return; } - if (imsg->fd == -1) { + if (fd == -1) { log_debug("debug: mta: failed to obtain msg fd"); mta_flush_task(s, IMSG_MTA_DELIVERY_TEMPFAIL, "Could not get message fd", 0, 0); @@ -295,12 +296,12 @@ mta_session_imsg(struct mproc *p, struct } if ((s->ext & MTA_EXT_SIZE) && s->ext_size != 0) { - if (fstat(imsg->fd, &sb) == -1) { + if (fstat(fd, &sb) == -1) { log_debug("debug: mta: failed to stat msg fd"); mta_flush_task(s, IMSG_MTA_DELIVERY_TEMPFAIL, "Could not stat message fd", 0, 0); mta_enter_state(s, MTA_READY); - close(imsg->fd); + close(fd); return; } if (sb.st_size > (off_t)s->ext_size) { @@ -308,12 +309,12 @@ mta_session_imsg(struct mproc *p, struct mta_flush_task(s, IMSG_MTA_DELIVERY_PERMFAIL, "message too large for peer", 0, 0); mta_enter_state(s, MTA_READY); - close(imsg->fd); + close(fd); return; } } - s->datafp = fdopen(imsg->fd, "r"); + s->datafp = fdopen(fd, "r"); if (s->datafp == NULL) fatal("mta: fdopen"); Index: queue.c =================================================================== RCS file: /cvs/src/usr.sbin/smtpd/queue.c,v diff -u -p -r1.195 queue.c --- queue.c 31 May 2023 16:51:46 -0000 1.195 +++ queue.c 13 Dec 2023 13:21:18 -0000 @@ -126,7 +126,7 @@ queue_imsg(struct mproc *p, struct imsg return; case IMSG_QUEUE_SMTP_SESSION: - bounce_fd(imsg->fd); + bounce_fd(imsg_get_fd(imsg)); return; case IMSG_LKA_ENVELOPE_SUBMIT: Index: queue_proc.c =================================================================== RCS file: /cvs/src/usr.sbin/smtpd/queue_proc.c,v diff -u -p -r1.9 queue_proc.c --- queue_proc.c 14 Jun 2021 17:58:16 -0000 1.9 +++ queue_proc.c 13 Dec 2023 13:18:20 -0000 @@ -170,7 +170,7 @@ queue_proc_message_fd_r(uint32_t msgid) queue_proc_call(); queue_proc_end(); - return (imsg.fd); + return (imsg_get_fd(&imsg)); } static int Index: smtp_session.c =================================================================== RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v diff -u -p -r1.439 smtp_session.c --- smtp_session.c 3 Jan 2024 08:11:15 -0000 1.439 +++ smtp_session.c 16 Jan 2024 13:27:47 -0000 @@ -702,7 +702,7 @@ smtp_session_imsg(struct mproc *p, struc const char *line, *helo; uint64_t reqid, evpid; uint32_t msgid; - int status, success; + int status, success, fd; int filter_response; const char *filter_param; uint8_t i; @@ -802,19 +802,20 @@ smtp_session_imsg(struct mproc *p, struc m_get_int(&m, &success); m_end(&m); + fd = imsg_get_fd(imsg); s = tree_xpop(&wait_queue_fd, reqid); - if (!success || imsg->fd == -1) { - if (imsg->fd != -1) - close(imsg->fd); + if (!success || fd == -1) { + if (fd != -1) + close(fd); smtp_reply(s, "421 %s Temporary Error", esc_code(ESC_STATUS_TEMPFAIL, ESC_OTHER_MAIL_SYSTEM_STATUS)); smtp_enter_state(s, STATE_QUIT); return; } - log_debug("smtp: %p: fd %d from queue", s, imsg->fd); + log_debug("smtp: %p: fd %d from queue", s, fd); - if (smtp_message_fd(s->tx, imsg->fd)) { + if (smtp_message_fd(s->tx, fd)) { if (!SESSION_DATA_FILTERED(s)) smtp_message_begin(s->tx); else @@ -828,19 +829,20 @@ smtp_session_imsg(struct mproc *p, struc m_get_int(&m, &success); m_end(&m); + fd = imsg_get_fd(imsg); s = tree_xpop(&wait_filter_fd, reqid); - if (!success || imsg->fd == -1) { - if (imsg->fd != -1) - close(imsg->fd); + if (!success || fd == -1) { + if (fd != -1) + close(fd); smtp_reply(s, "421 %s Temporary Error", esc_code(ESC_STATUS_TEMPFAIL, ESC_OTHER_MAIL_SYSTEM_STATUS)); smtp_enter_state(s, STATE_QUIT); return; } - log_debug("smtp: %p: fd %d from lka", s, imsg->fd); + log_debug("smtp: %p: fd %d from lka", s, fd); - smtp_filter_fd(s->tx, imsg->fd); + smtp_filter_fd(s->tx, fd); smtp_message_begin(s->tx); return; Index: smtpd.c =================================================================== RCS file: /cvs/src/usr.sbin/smtpd/smtpd.c,v diff -u -p -r1.346 smtpd.c --- smtpd.c 18 Jun 2023 17:28:42 -0000 1.346 +++ smtpd.c 16 Jan 2024 13:30:58 -0000 @@ -917,7 +917,8 @@ setup_proc(void) env->sc_queue_key = strdup(imsg.data); break; case IMSG_SETUP_PEER: - setup_peer(imsg.hdr.peerid, imsg.hdr.pid, imsg.fd); + setup_peer(imsg.hdr.peerid, imsg.hdr.pid, + imsg_get_fd(&imsg)); break; case IMSG_SETUP_DONE: setup = 0; @@ -1866,19 +1867,11 @@ log_imsg(int to, int from, struct imsg * if (to == PROC_CONTROL && imsg->hdr.type == IMSG_STAT_SET) return; - if (imsg->fd != -1) - log_trace(TRACE_IMSG, "imsg: %s <- %s: %s (len=%zu, fd=%d)", - proc_name(to), - proc_name(from), - imsg_to_str(imsg->hdr.type), - imsg->hdr.len - IMSG_HEADER_SIZE, - imsg->fd); - else - log_trace(TRACE_IMSG, "imsg: %s <- %s: %s (len=%zu)", - proc_name(to), - proc_name(from), - imsg_to_str(imsg->hdr.type), - imsg->hdr.len - IMSG_HEADER_SIZE); + log_trace(TRACE_IMSG, "imsg: %s <- %s: %s (len=%zu)", + proc_name(to), + proc_name(from), + imsg_to_str(imsg->hdr.type), + imsg->hdr.len - IMSG_HEADER_SIZE); } const char *