Re: [PATCH 00/10] IB: Replace safe uses for ib_get_dma_mr with pd->local_dma_lkey

[Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>]

On 07/23/2015 11:31 AM, Jason Gunthorpe wrote:
> It looks to me like register_always is similar to iSER, it is trying
> to avoid a MR if there is only 1 S/G entry. That performance behavior
> needs to default to disabled. The kernel must default to secure out of
> the box.
>
> [ ... ]
>
> This probably also takes care of the security issue for SRP, what do you
> think?
>[ ... ]
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> index fb9fed0fac28..a1e3818d0791 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -69,7 +69,7 @@ static unsigned int cmd_sg_entries;
>   static unsigned int indirect_sg_entries;
>   static bool allow_ext_sg;
>   static bool prefer_fr;
> -static bool register_always;
> +static bool register_always = true;

If we want security by default then I propose not only to change the 
default value of register_always from false into true but also to change 
the default value of prefer_fr from false into true such that fast 
registration becomes the default instead of FMR.

> +	if (register_always)
> +		mr_flags |= IB_ACCESS_REMOTE_READ | IB_ACCESS_REMOTE_WRITE;

Hmm ... did you perhaps intend if (!register_always) instead ?

Thanks,

Bart.
--
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