On Wed, Aug 19, 2015 at 04:59:11PM +0300, Yishai Hadas wrote: > On 8/18/2015 8:50 PM, Jason Gunthorpe wrote: > >On Thu, Aug 13, 2015 at 06:32:07PM +0300, Yishai Hadas wrote: > >>@@ -501,10 +586,24 @@ static ssize_t ucma_destroy_id(struct ucma_file *file, const char __user *inbuf, > >>+ if (!ctx->closing) { > >>+ mutex_unlock(&mut); > >>+ ucma_put_ctx(ctx); > >>+ wait_for_completion(&ctx->comp); > >>+ rdma_destroy_id(ctx->cm_id); > > > >Suggest nulling cm_id after it is destroyed in all places, this code > >is very complicated, I'd rather see a nice clean risk of > >null-pointer-deref than an undetected use-after free if it gets messed > >up. > > It can be helpful for debugging but usually nulling is not done when it's > not really needed, because it is considered redundant. > Currently it's not the usage in this module and in cma.c when calling > rdma_destroy_id. Well, 'not really needed' should be something simple to prove, and this is not at all simple. > >Isn't the list_for_each_safe version needed if list_del/kfree is called > >within the body? > No need for a safe version here. Okay 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