bug in ucma_accept()?

bug in ucma_accept()?

[Steve Wise]

Hey Sean,

Is this a bug?  I think it is...

diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index 2709ff5..fb24f05 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -806,8 +806,13 @@ static ssize_t ucma_accept(struct ucma_file *file, 
const ch
                 if (!ret)
                         ctx->uid = cmd.uid;
                 mutex_unlock(&file->mut);
-       } else
+       } else {
+               mutex_lock(&file->mut);
                 ret = rdma_accept(ctx->cm_id, NULL);
+               if (!ret)
+                       ctx->uid = cmd.uid;
+               mutex_unlock(&file->mut);
+       }

         ucma_put_ctx(ctx);
         return ret;

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: bug in ucma_accept()?

[Hefty, Sean]

 
> Is this a bug?  I think it is...
> 
> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index 2709ff5..fb24f05 100644
> --- a/drivers/infiniband/core/ucma.c
> +++ b/drivers/infiniband/core/ucma.c
> @@ -806,8 +806,13 @@ static ssize_t ucma_accept(struct ucma_file *file,
> const ch
>                  if (!ret)
>                          ctx->uid = cmd.uid;
>                  mutex_unlock(&file->mut);
> -       } else
> +       } else {
> +               mutex_lock(&file->mut);
>                  ret = rdma_accept(ctx->cm_id, NULL);
> +               if (!ret)
> +                       ctx->uid = cmd.uid;
> +               mutex_unlock(&file->mut);
> +       }

The ctx->uid has already been set in the else case when the user explicitly created the cm_id.  We only need to set it when the ctx has been created in the kernel for the user in response to a connection request (the 'if' part to the above else).

- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: bug in ucma_accept()?

[Steve Wise]

On 1/7/2013 1:22 PM, Hefty, Sean wrote:
>   
>> Is this a bug?  I think it is...
>>
>> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
>> index 2709ff5..fb24f05 100644
>> --- a/drivers/infiniband/core/ucma.c
>> +++ b/drivers/infiniband/core/ucma.c
>> @@ -806,8 +806,13 @@ static ssize_t ucma_accept(struct ucma_file *file,
>> const ch
>>                   if (!ret)
>>                           ctx->uid = cmd.uid;
>>                   mutex_unlock(&file->mut);
>> -       } else
>> +       } else {
>> +               mutex_lock(&file->mut);
>>                   ret = rdma_accept(ctx->cm_id, NULL);
>> +               if (!ret)
>> +                       ctx->uid = cmd.uid;
>> +               mutex_unlock(&file->mut);
>> +       }
> The ctx->uid has already been set in the else case when the user explicitly created the cm_id.  We only need to set it when the ctx has been created in the kernel for the user in response to a connection request (the 'if' part to the above else).
>
>

Ah.  Thanks for clarifying...

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html