BUG/MEDIUM: mux-pt: Never fully close the connection on shutdown

When a shutdown is reported to the mux (shutdown for reads or writes), the
connexion is immediately fully closed if the mux detects the connexion is
closed in both directions. Only the passthrough multiplexer is able to
perform this action at this stage because there is no stream and no internal
data. Other muxes perform a full connection close during the mux's release
stage. It was working quite well since recently. But, in theory, the bug is
quite old.

In fact, it seems possible for the lower layer to report an error on the
connection in same time a shutdown is performed on the mux. Depending on how
events are scheduled, the following may happen:

 1. An connection error is detected at the fd layer and a wakeup is
    scheduled on the mux to handle the event.

 2. A shutdown for writes is performed on the mux. Here the mux decides to
    fully close the connexion. If the xprt is not used to log info, it is
    released.

 3. The mux is finally woken up. It tries to retrieve data from the xprt
    because it is not awayre there was an error. This leads to a crash
    because of a NULL-deref.

By reading the code, it is not obvious. But it seems possible with SSL
connection when the handshake is rearmed. It happens when a
SSL_ERROR_WANT_WRITE is reported on a SSL_read() attempt or a
SSL_ERROR_WANT_READ on a SSL_write() attempt.

This bug is only visible if the XPRT is not used to log info. So it is no so
common.

This patch should fix the 2nd crash reported in the issue #2656. It must
first be backported as far as 2.9 and then slowly to all stable versions.
This commit is contained in:
Christopher Faulet 2024-09-02 15:30:11 +02:00
parent f9adcdf039
commit 76fa71f7a8
1 changed files with 1 additions and 5 deletions

View File

@ -470,9 +470,7 @@ static void mux_pt_shut(struct stconn *sc, unsigned int mode, struct se_abort_in
if (mode & (SE_SHW_SILENT|SE_SHW_NORMAL)) { if (mode & (SE_SHW_SILENT|SE_SHW_NORMAL)) {
if (conn_xprt_ready(conn) && conn->xprt->shutw) if (conn_xprt_ready(conn) && conn->xprt->shutw)
conn->xprt->shutw(conn, conn->xprt_ctx, (mode & SE_SHW_NORMAL)); conn->xprt->shutw(conn, conn->xprt_ctx, (mode & SE_SHW_NORMAL));
if (conn->flags & CO_FL_SOCK_RD_SH) if (!(conn->flags & CO_FL_SOCK_RD_SH))
conn_full_close(conn);
else
conn_sock_shutw(conn, (mode & SE_SHW_NORMAL)); conn_sock_shutw(conn, (mode & SE_SHW_NORMAL));
} }
@ -482,8 +480,6 @@ static void mux_pt_shut(struct stconn *sc, unsigned int mode, struct se_abort_in
conn->xprt->shutr(conn, conn->xprt_ctx, (mode & SE_SHR_DRAIN)); conn->xprt->shutr(conn, conn->xprt_ctx, (mode & SE_SHR_DRAIN));
else if (mode & SE_SHR_DRAIN) else if (mode & SE_SHR_DRAIN)
conn_ctrl_drain(conn); conn_ctrl_drain(conn);
if (conn->flags & CO_FL_SOCK_WR_SH)
conn_full_close(conn);
} }
TRACE_LEAVE(PT_EV_STRM_SHUT, conn, sc); TRACE_LEAVE(PT_EV_STRM_SHUT, conn, sc);