On Sun, May 14, 2017 at 11:40 AM, Leon Romanovsky <leon@xxxxxxxxxx> wrote:
> On Wed, May 10, 2017 at 03:45:34AM -0700, Selvin Xavier wrote:
>> Do not free context table entry if delete GID fails. Stack may call
>> back the add_gid for the same context again which could result in
>> panic
>
> "may call" ??? Will you leave memory leak if this "may call" won't happen?
>
This fix was added only to avoid system crash in some a specific scenario.
When bnxt_re driver is loaded and if user tries to change interface mac address,
our delete GID fails because QP1 is still associated with existing MAC
(default GID).
If the above command fails GID tables are not modified in the h/w or
driver, but the GID context
memory is freed. Now, if the user changes the mac back to the original
value, another add_gid
comes to the driver where the driver reports that the GID is already
present in its table
and tries to access the context which was already freed.
So, in this case, in order to avoid NULL pointer de-reference, this
patch removes the context memory
free if delete_gid fails and the same context memory is re-used in new add_gid.
Memory cleanup will be taken care during driver unload, while deleting
the GID table.
I understood that the "may call" comment is not detailing this problem.
I will update the commit log and send a v2.
Thanks for your comments.
Selvin Xavier
>
>>
>> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@xxxxxxxxxxxx>
>> Signed-off-by: Selvin Xavier <selvin.xavier@xxxxxxxxxxxx>
>> ---
>> drivers/infiniband/hw/bnxt_re/ib_verbs.c | 16 +++++++++-------
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
>> index 61703f3..525f4b0 100644
>> --- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
>> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
>> @@ -375,15 +375,17 @@ int bnxt_re_del_gid(struct ib_device *ibdev, u8 port_num,
>> return -EINVAL;
>> ctx->refcnt--;
>> if (!ctx->refcnt) {
>> - rc = bnxt_qplib_del_sgid
>> - (sgid_tbl,
>> - &sgid_tbl->tbl[ctx->idx], true);
>> - if (rc)
>> + rc = bnxt_qplib_del_sgid(sgid_tbl,
>> + &sgid_tbl->tbl[ctx->idx],
>> + true);
>> + if (rc) {
>> dev_err(rdev_to_dev(rdev),
>> "Failed to remove GID: %#x", rc);
>> - ctx_tbl = sgid_tbl->ctx;
>> - ctx_tbl[ctx->idx] = NULL;
>> - kfree(ctx);
>> + } else {
>> + ctx_tbl = sgid_tbl->ctx;
>> + ctx_tbl[ctx->idx] = NULL;
>> + kfree(ctx);
>> + }
>> }
>> } else {
>> return -EINVAL;
>> --
>> 2.5.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
