On Tue, Mar 20, 2018 at 04:18:40PM +0000, Hefty, Sean wrote:
> > ctx->uid = cmd.uid;
> > - ctx->cm_id = rdma_create_id(current->nsproxy->net_ns,
> > - ucma_event_handler, ctx, cmd.ps, qp_type);
> > - if (IS_ERR(ctx->cm_id)) {
> > - ret = PTR_ERR(ctx->cm_id);
> > + cm_id = rdma_create_id(current->nsproxy->net_ns,
> > + ucma_event_handler, ctx, cmd.ps, qp_type);
> > + if (IS_ERR(cm_id)) {
> > + ret = PTR_ERR(cm_id);
> > goto err1;
> > }
> > + ctx->cm_id = cm_id;
> >
> > resp.id = ctx->id;
> > if (copy_to_user((void __user *)(unsigned long)cmd.response,
>
> If copy_to_user() fails, we destroy the cm_id and ctx. But there's
> a gap immediately before that where user space could conceivably
> acquire the ctx and start using it. If we don't set ctx->cm_id
> until after copy_to_user() succeeds, then I think we close any races
> (with your fix in get_ctx to verify that cm_id is not NULL).
Agree.
It is clearly wrong to publish the pointer to other threads without
holding the ref and going through the wait_completion stuff prior to
destroy.
We can avoid doing that by moving the 'commit' stage to the end.
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
