d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

iflib: fix invalid free during queue allocation failure

Browse Source 
In r301567, code was added to cleanup to prevent memory leaks for the
Tx and Rx ring structs. This code carefully tracked txq and rxq, and
made sure to free them properly during cleanup.

Because we assigned the txq and rxq pointers into the ctx->ifc_txqs and
ctx->ifc_rxqs, we carefully reset these pointers to NULL, so that
cleanup code would not accidentally free the memory twice.

This was changed by r304021 ("Update iflib to support more NIC designs"),
which removed this resetting of the pointers to NULL, because it re-used
the txq and rxq pointers as an index into the queue set array.

Unfortunately, the cleanup code was left alone. Thus, if we fail to
allocate DMA or fail to configure the queues using the drivers ifdi
methods, we will attempt to free txq and rxq. These variables would now
incorrectly point to the wrong location, resulting in a page fault.

There are a number of methods to correct this, but ultimately the root
cause was that we reuse the txq and rxq pointers for two different
purposes.

Instead, when allocating, store the returned pointer directly into
ctx->ifc_txqs and ctx->ifc_rxqs. Then, assign this to txq and rxq as
index pointers before starting the loop to allocate each queue.
Drop the cleanup code for txq and rxq, and only use ctx->ifc_txqs and
ctx->ifc_rxqs.

Thus, we no longer need to free txq or rxq under any error flow, and
intsead rely solely on the pointers stored in ctx->ifc_txqs and
ctx->ifc_rxqs. This prevents the invalid free(), and ensures that we
still properly cleanup after ourselves as before when failing to
allocate.

Submitted by:	Jacob Keller
Reviewed by:	gallatin, sbruno
Sponsored by:	Intel Corporation
Differential Revision:	https://reviews.freebsd.org/D15285
This commit is contained in:
Stephen Hurd 2018-05-04 15:20:34 +00:00
parent 4d613f5d04
commit b89827a052
1 changed files with 5 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
sys/net/iflib.c
View File

@ -4777,11 +4777,8 @@ iflib_queues_alloc(if_ctx_t ctx)
KASSERT(ntxqs > 0, ("number of queues per qset must be at least 1"));
KASSERT(nrxqs > 0, ("number of queues per qset must be at least 1"));
txq = NULL;
rxq = NULL;
/* Allocate the TX ring struct memory */
if (!(txq =
if (!(ctx->ifc_txqs =
(iflib_txq_t) malloc(sizeof(struct iflib_txq) *
ntxqsets, M_IFLIB, M_NOWAIT | M_ZERO))) {
device_printf(dev, "Unable to allocate TX ring memory\n");
@ -4790,7 +4787,7 @@ iflib_queues_alloc(if_ctx_t ctx)
}
/* Now allocate the RX */
if (!(rxq =
if (!(ctx->ifc_rxqs =
(iflib_rxq_t) malloc(sizeof(struct iflib_rxq) *
nrxqsets, M_IFLIB, M_NOWAIT | M_ZERO))) {
device_printf(dev, "Unable to allocate RX ring memory\n");
@ -4798,8 +4795,8 @@ iflib_queues_alloc(if_ctx_t ctx)
goto rx_fail;
}
ctx->ifc_txqs = txq;
ctx->ifc_rxqs = rxq;
txq = ctx->ifc_txqs;
rxq = ctx->ifc_rxqs;
/*
* XXX handle allocation failure
@ -4957,17 +4954,13 @@ iflib_queues_alloc(if_ctx_t ctx)
/* XXX handle allocation failure changes */
err_rx_desc:
err_tx_desc:
rx_fail:
if (ctx->ifc_rxqs != NULL)
free(ctx->ifc_rxqs, M_IFLIB);
ctx->ifc_rxqs = NULL;
if (ctx->ifc_txqs != NULL)
free(ctx->ifc_txqs, M_IFLIB);
ctx->ifc_txqs = NULL;
rx_fail:
if (rxq != NULL)
free(rxq, M_IFLIB);
if (txq != NULL)
free(txq, M_IFLIB);
fail:
return (err);
}