On Mon, May 15, 2017 at 04:24:04PM +0530, Selvin Xavier wrote:
> 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
>
> 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
Attachment:
signature.asc
Description: PGP signature
