BUG/MINOR: mux-quic: properly handle STREAM frame alloc failure

Previously, if a STREAM frame cannot be allocated for emission, a crash
would occurs due to an ABORT_NOW() statement in _qc_send_qcs().

Replace this by proper error code handling. Each stream were sending
fails are removed temporarily from qcc::send_list to a list local to
_qc_send_qcs(). Once emission has been conducted for all streams,
reinsert failed stream to qcc::send_list. This avoids to reloop on
failed streams on the second while loop at the end of _qc_send_qcs().

This crash was reproduced using -dMfail.

This should be backported up to 2.6.
This commit is contained in:
Amaury Denoyelle 2023-04-19 11:42:24 +02:00
parent ed820823f0
commit 93d2ebe9f3

View File

@ -1785,7 +1785,8 @@ static int qcs_send_stop_sending(struct qcs *qcs)
* is then generated and inserted in <frms> list.
*
* Returns the total bytes transferred between qcs and quic_stream buffers. Can
* be null if out buffer cannot be allocated.
* be null if out buffer cannot be allocated. On error a negative error code is
* used.
*/
static int _qc_send_qcs(struct qcs *qcs, struct list *frms)
{
@ -1835,14 +1836,17 @@ static int _qc_send_qcs(struct qcs *qcs, struct list *frms)
/* Build a new STREAM frame with <out> buffer. */
if (qcs->tx.sent_offset != qcs->tx.offset || fin) {
int ret;
ret = qcs_build_stream_frm(qcs, out, fin, frms);
if (ret < 0) { ABORT_NOW(); /* TODO handle this properly */ }
if (qcs_build_stream_frm(qcs, out, fin, frms) < 0)
goto err;
}
out:
TRACE_LEAVE(QMUX_EV_QCS_SEND, qcc->conn, qcs);
return xfer;
err:
TRACE_DEVEL("leaving on error", QMUX_EV_QCS_SEND, qcc->conn, qcs);
return -1;
}
/* Proceed to sending. Loop through all available streams for the <qcc>
@ -1853,8 +1857,10 @@ static int _qc_send_qcs(struct qcs *qcs, struct list *frms)
static int qc_send(struct qcc *qcc)
{
struct list frms = LIST_HEAD_INIT(frms);
/* Temporary list for QCS on error. */
struct list qcs_failed = LIST_HEAD_INIT(qcs_failed);
struct qcs *qcs, *qcs_tmp;
int total = 0;
int ret, total = 0;
TRACE_ENTER(QMUX_EV_QCC_SEND, qcc->conn);
@ -1913,8 +1919,16 @@ static int qc_send(struct qcc *qcc)
continue;
}
if (!(qcs->flags & QC_SF_BLK_SFCTL))
total += _qc_send_qcs(qcs, &frms);
if (!(qcs->flags & QC_SF_BLK_SFCTL)) {
if ((ret = _qc_send_qcs(qcs, &frms)) < 0) {
/* Temporarily remove QCS from send-list. */
LIST_DEL_INIT(&qcs->el_send);
LIST_APPEND(&qcs_failed, &qcs->el_send);
continue;
}
total += ret;
}
}
/* Retry sending until no frame to send, data rejected or connection
@ -1924,15 +1938,22 @@ static int qc_send(struct qcc *qcc)
/* Reloop over <qcc.send_list>. Useful for streams which have
* fulfilled their qc_stream_desc buf and have now release it.
*/
list_for_each_entry(qcs, &qcc->send_list, el_send) {
list_for_each_entry_safe(qcs, qcs_tmp, &qcc->send_list, el_send) {
/* Only streams blocked on flow-control or waiting on a
* new qc_stream_desc should be present in send_list as
* long as transport layer can handle all data.
*/
BUG_ON(qcs->stream->buf && !(qcs->flags & QC_SF_BLK_SFCTL));
if (!(qcs->flags & QC_SF_BLK_SFCTL))
total += _qc_send_qcs(qcs, &frms);
if (!(qcs->flags & QC_SF_BLK_SFCTL)) {
if ((ret = _qc_send_qcs(qcs, &frms)) < 0) {
LIST_DEL_INIT(&qcs->el_send);
LIST_APPEND(&qcs_failed, &qcs->el_send);
continue;
}
total += ret;
}
}
}
@ -1945,6 +1966,17 @@ static int qc_send(struct qcc *qcc)
qc_frm_free(&frm);
}
/* Re-insert on-error QCS at the end of the send-list. */
if (!LIST_ISEMPTY(&qcs_failed)) {
list_for_each_entry_safe(qcs, qcs_tmp, &qcs_failed, el_send) {
LIST_DEL_INIT(&qcs->el_send);
LIST_APPEND(&qcc->send_list, &qcs->el_send);
}
if (!(qcc->flags & QC_CF_BLK_MFCTL))
tasklet_wakeup(qcc->wait_event.tasklet);
}
TRACE_LEAVE(QMUX_EV_QCC_SEND, qcc->conn);
return total;