On Mon, Mar 19, 2018 at 07:11:09PM +0000, Hefty, Sean wrote:
> > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> >
> > The error in ucma_create_id() left ctx in the list of contexts belong
> > to ucma file descriptor. The attempt to close this file descriptor
> > causes to use-after-free accesses while iterating over such list.
> >
> > Fixes: 75216638572f ("RDMA/cma: Export rdma cm interface to
> > userspace")
> > Reported-by: <syzbot+dcfd344365a56fbebd0f@xxxxxxxxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
>
> Reviewed-by: Sean Hefty <sean.hefty@xxxxxxxxx>
>
> > drivers/infiniband/core/ucma.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/infiniband/core/ucma.c
> > b/drivers/infiniband/core/ucma.c index 718ed16688a8..07d017fee58f
> > 100644
> > +++ b/drivers/infiniband/core/ucma.c
> > @@ -497,6 +497,9 @@ static ssize_t ucma_create_id(struct ucma_file
> > *file, const char __user *inbuf,
> > mutex_lock(&mut);
> > idr_remove(&ctx_idr, ctx->id);
> > mutex_unlock(&mut);
> > + mutex_lock(&file->mut);
> > + list_del(&ctx->list);
> > + mutex_unlock(&file->mut);
> > kfree(ctx);
> > return ret;
>
> I think this bug increased the chance of hitting the problem fixed by
No doubt..
But is this overall design even sane? We add a partially ininitalized
ctx to the ctx_list and to the idr, then finish the initialization.
Doesn't that just make all the other entry points that much harder to
design?
Shouldn't the idr store and list_add_tail be after rdma_create_id completes?
Jason
--
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
