On Wed, Nov 23, 2016 at 09:14:13AM +0530, Sunil Nimmagadda wrote:
>
> Eric Faurot writes:
>
> > Hi.
> >
> > When using the internal io api, the caller had to explicitely reset an
> > io event in some cases (basically when data is buffered outside of an
> > io callback context) which could easily be missed. This has been the
> > cause of some of smtpd bugs in the past (hanging sessions).
> >
> > After the recent changes, it's now possible to make it a lot simpler
> > by triggering an event reload internally when data is queued. So the
> > api user does not have to worry about it.
> >
> > Eric.
>
> Ok sunil@
>
been running with my server patched since yesterday, no regression
ok gilles@
> >
> > Index: ioev.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/smtpd/ioev.c,v
> > retrieving revision 1.31
> > diff -u -p -r1.31 ioev.c
> > --- ioev.c 22 Nov 2016 07:28:42 -0000 1.31
> > +++ ioev.c 22 Nov 2016 22:24:25 -0000
> > @@ -370,13 +370,25 @@ io_set_write(struct io *io)
> > int
> > io_write(struct io *io, const void *buf, size_t len)
> > {
> > - return iobuf_queue(io->iobuf, buf, len);
> > + int r;
> > +
> > + r = iobuf_queue(io->iobuf, buf, len);
> > +
> > + io_reload(io);
> > +
> > + return r;
> > }
> >
> > int
> > io_writev(struct io *io, const struct iovec *iov, int iovcount)
> > {
> > - return iobuf_queuev(io->iobuf, iov, iovcount);
> > + int r;
> > +
> > + r = iobuf_queuev(io->iobuf, iov, iovcount);
> > +
> > + io_reload(io);
> > +
> > + return r;
> > }
> >
> > int
> > Index: mta_session.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/smtpd/mta_session.c,v
> > retrieving revision 1.89
> > diff -u -p -r1.89 mta_session.c
> > --- mta_session.c 22 Nov 2016 07:28:42 -0000 1.89
> > +++ mta_session.c 22 Nov 2016 22:24:26 -0000
> > @@ -280,7 +280,6 @@ mta_session_imsg(struct mproc *p, struct
> > mta_flush_task(s, IMSG_MTA_DELIVERY_TEMPFAIL,
> > "Could not get message fd", 0, 0);
> > mta_enter_state(s, MTA_READY);
> > - io_reload(&s->io);
> > return;
> > }
> >
> > @@ -289,7 +288,6 @@ mta_session_imsg(struct mproc *p, struct
> > fatal("mta: fdopen");
> >
> > mta_enter_state(s, MTA_MAIL);
> > - io_reload(&s->io);
> > return;
> >
> > case IMSG_MTA_DNS_PTR:
> > @@ -366,7 +364,6 @@ mta_session_imsg(struct mproc *p, struct
> >
> > mta_tls_verified(s);
> > io_resume(&s->io, IO_PAUSE_IN);
> > - io_reload(&s->io);
> > return;
> >
> > case IMSG_MTA_LOOKUP_HELO:
> > @@ -456,7 +453,6 @@ mta_on_timeout(struct runq *runq, void *
> > s->hangon++;
> >
> > mta_enter_state(s, MTA_READY);
> > - io_reload(&s->io);
> > }
> >
> > static void
> > Index: smtp_session.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
> > retrieving revision 1.295
> > diff -u -p -r1.295 smtp_session.c
> > --- smtp_session.c 22 Nov 2016 07:28:42 -0000 1.295
> > +++ smtp_session.c 22 Nov 2016 22:24:26 -0000
> > @@ -738,14 +738,12 @@ smtp_session_imsg(struct mproc *p, struc
> > smtp_filter_tx_rollback(s);
> > smtp_tx_free(s->tx);
> > smtp_reply(s, "%d %s", 530, "Sender rejected");
> > - io_reload(&s->io);
> > break;
> > case LKA_TEMPFAIL:
> > smtp_filter_tx_rollback(s);
> > smtp_tx_free(s->tx);
> > smtp_reply(s, "421 %s: Temporary Error",
> > esc_code(ESC_STATUS_TEMPFAIL,
> > ESC_OTHER_MAIL_SYSTEM_STATUS));
> > - io_reload(&s->io);
> > break;
> > }
> > return;
> > @@ -766,7 +764,6 @@ smtp_session_imsg(struct mproc *p, struc
> > case LKA_TEMPFAIL:
> > smtp_reply(s, "%s", line);
> > }
> > - io_reload(&s->io);
> > return;
> >
> > case IMSG_SMTP_LOOKUP_HELO:
> > @@ -802,7 +799,6 @@ smtp_session_imsg(struct mproc *p, struc
> > smtp_enter_state(s, STATE_QUIT);
> > }
> > m_end(&m);
> > - io_reload(&s->io);
> > return;
> >
> > case IMSG_SMTP_MESSAGE_OPEN:
> > @@ -818,7 +814,6 @@ smtp_session_imsg(struct mproc *p, struc
> > smtp_reply(s, "421 %s: Temporary Error",
> > esc_code(ESC_STATUS_TEMPFAIL,
> > ESC_OTHER_MAIL_SYSTEM_STATUS));
> > smtp_enter_state(s, STATE_QUIT);
> > - io_reload(&s->io);
> > return;
> > }
> >
> > @@ -872,7 +867,6 @@ smtp_session_imsg(struct mproc *p, struc
> > esc_code(ESC_STATUS_OK,
> > ESC_DESTINATION_ADDRESS_VALID),
> > esc_description(ESC_DESTINATION_ADDRESS_VALID));
> > }
> > - io_reload(&s->io);
> > return;
> >
> > case IMSG_SMTP_MESSAGE_COMMIT:
> > @@ -887,7 +881,6 @@ smtp_session_imsg(struct mproc *p, struc
> > smtp_reply(s, "421 %s: Temporary failure",
> > esc_code(ESC_STATUS_TEMPFAIL,
> > ESC_OTHER_MAIL_SYSTEM_STATUS));
> > smtp_enter_state(s, STATE_QUIT);
> > - io_reload(&s->io);
> > return;
> > }
> >
> > @@ -915,7 +908,6 @@ smtp_session_imsg(struct mproc *p, struc
> > smtp_tx_free(s->tx);
> > s->mailcount++;
> > smtp_enter_state(s, STATE_HELO);
> > - io_reload(&s->io);
> > return;
> >
> > case IMSG_SMTP_AUTHENTICATE:
> > @@ -955,7 +947,6 @@ smtp_session_imsg(struct mproc *p, struc
> > fatalx("bad lka response");
> >
> > smtp_enter_state(s, STATE_HELO);
> > - io_reload(&s->io);
> > return;
> >
> > case IMSG_SMTP_TLS_INIT:
> > @@ -1045,7 +1036,6 @@ smtp_filter_response(uint64_t id, int qu
> > line = line ? line : "Temporary failure";
> > smtp_reply(s, "%d %s", code, line);
> > smtp_enter_state(s, STATE_QUIT);
> > - io_reload(&s->io);
> > return;
> > }
> >
> > @@ -1085,7 +1075,6 @@ smtp_filter_response(uint64_t id, int qu
> > code = code ? code : 530;
> > line = line ? line : "Hello rejected";
> > smtp_reply(s, "%d %s", code, line);
> > - io_reload(&s->io);
> > return;
> > }
> >
> > @@ -1108,7 +1097,6 @@ smtp_filter_response(uint64_t id, int qu
> > smtp_reply(s, "250-AUTH PLAIN LOGIN");
> > smtp_reply(s, "250 HELP");
> > }
> > - io_reload(&s->io);
> > return;
> >
> > case QUERY_MAIL:
> > @@ -1118,7 +1106,6 @@ smtp_filter_response(uint64_t id, int qu
> > code = code ? code : 530;
> > line = line ? line : "Sender rejected";
> > smtp_reply(s, "%d %s", code, line);
> > - io_reload(&s->io);
> > return;
> > }
> >
> > @@ -1141,7 +1128,6 @@ smtp_filter_response(uint64_t id, int qu
> > code = code ? code : 530;
> > line = line ? line : "Recipient rejected";
> > smtp_reply(s, "%d %s", code, line);
> > - io_reload(&s->io);
> > return;
> > }
> >
> > @@ -1157,7 +1143,6 @@ smtp_filter_response(uint64_t id, int qu
> > code = code ? code : 530;
> > line = line ? line : "Message rejected";
> > smtp_reply(s, "%d %s", code, line);
> > - io_reload(&s->io);
> > return;
> > }
> > smtp_queue_open_message(s);
> > @@ -1173,7 +1158,6 @@ smtp_filter_response(uint64_t id, int qu
> > line = line ? line : "Message rejected";
> > smtp_reply(s, "%d %s", code, line);
> > smtp_enter_state(s, STATE_HELO);
> > - io_reload(&s->io);
> > return;
> > }
> > smtp_message_end(s);
> > @@ -1198,7 +1182,6 @@ smtp_filter_fd(uint64_t id, int fd)
> > smtp_reply(s, "421 %s: Temporary Error",
> > esc_code(ESC_STATUS_TEMPFAIL,
> > ESC_OTHER_MAIL_SYSTEM_STATUS));
> > smtp_enter_state(s, STATE_QUIT);
> > - io_reload(&s->io);
> > return;
> > }
> >
> > @@ -1257,7 +1240,6 @@ smtp_filter_fd(uint64_t id, int fd)
> > " on a line by itself");
> >
> > tree_xset(&wait_filter_data, s->id, s);
> > - io_reload(&s->io);
> > }
> >
> > static void
> > @@ -1341,8 +1323,6 @@ smtp_io(struct io *io, int evt, void *ar
> > s->tx->dataeom = 1;
> > if (io_queued(&s->tx->oev) == 0)
> > smtp_data_io_done(s);
> > - else
> > - io_reload(&s->tx->oev);
> > return;
> > }
> >
> > @@ -1547,7 +1527,6 @@ smtp_data_io_done(struct smtp_session *s
> > smtp_reply(s, "421 Internal server error");
> > smtp_tx_free(s->tx);
> > smtp_enter_state(s, STATE_HELO);
> > - io_reload(&s->io);
> > }
> > else {
> > smtp_filter_eom(s);
> > @@ -2137,7 +2116,6 @@ static void
> > smtp_send_banner(struct smtp_session *s)
> > {
> > smtp_reply(s, "220 %s ESMTP %s", s->smtpname, SMTPD_NAME);
> > - io_reload(&s->io);
> > }
> >
> > void
> > @@ -2466,7 +2444,6 @@ smtp_auth_failure_resume(int fd, short e
> >
> > smtp_reply(s, "535 Authentication failed");
> > smtp_enter_state(s, STATE_HELO);
> > - io_reload(&s->io);
> > }
> >
> > static void
> > @@ -2662,7 +2639,6 @@ smtp_filter_dataline(struct smtp_session
> > log_debug("debug: smtp: %p: filter congestion over: pausing
> > session", s);
> > io_pause(&s->io, IO_PAUSE_IN);
> > }
> > - io_reload(&s->tx->oev);
> > }
> >
> > #define CASE(x) case x : return #x
>
--
Gilles Chehade
https://www.poolp.org @poolpOrg
