Prev: [PATCH] x86, cacheinfo: Turn off L3 cache index disable feature in virtualized environments
Next: uio_pci_generic: extensions to allow access for non-privileged processes
From: Dan Carpenter on 1 Apr 2010 11:40
On Wed, Mar 31, 2010 at 02:06:21PM -0700, Roland Dreier wrote:
> So looking at merging this finally, I think I see one problem with the
> proposed patch. We have:
>
> > @@ -183,7 +180,7 @@ static int iser_create_ib_conn_res(struct iser_conn *ib_conn)
> > ib_conn->fmr_pool = ib_create_fmr_pool(device->pd, &params);
> > if (IS_ERR(ib_conn->fmr_pool)) {
> > ret = PTR_ERR(ib_conn->fmr_pool);
> > - goto fmr_pool_err;
> > + goto out_err;
> > }
>
> and
>
> > @@ -209,12 +206,7 @@ static int iser_create_ib_conn_res(struct iser_conn *ib_conn)
> > ib_conn->fmr_pool, ib_conn->cma_id->qp);
> > return ret;
> >
> > -qp_err:
> > - (void)ib_destroy_fmr_pool(ib_conn->fmr_pool);
> > -fmr_pool_err:
> > - kfree(ib_conn->page_vec);
> > - kfree(ib_conn->login_buf);
> > -alloc_err:
> > +out_err:
> > iser_err("unable to alloc mem or create resource, err %d\n", ret);
> > return ret;
> > }
>
> so if ib_create_fmr_pool() fails, we're left with ib_conn->fmr_pool
> holding an error pointer, right? But we're relying on
> iser_free_ib_conn_res() to clean up after us, and that has:
>
> if (ib_conn->fmr_pool != NULL)
> ib_destroy_fmr_pool(ib_conn->fmr_pool);
>
> so we're going to end up trying to free an error pointer, which will
> probably crash.
>
> I think.
>
> Dan or Or, am I wrong here or do we need another iteration of this
> patch? (the login_buf and page_vec changes do look correct to me, since
> a failed kmalloc() will leave us with a NULL pointer that it is safe to
> kfree() later)

You're right. I missed that.

I will send an updated patch tomorrow.

regards,
dan carpenter

>
> - R.
> --
> Roland Dreier <rolandd(a)cisco.com> || For corporate legal information go to:
> http://www.cisco.com/web/about/doing_business/legal/cri/index.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: [PATCH] x86, cacheinfo: Turn off L3 cache index disable feature in virtualized environments
Next: uio_pci_generic: extensions to allow access for non-privileged processes