Re: [PATCH V3 1/5] RDMA/core: Transport-independent access flags

Jason Gunthorpe
2015-07-09T22:53:06Z

On Thu, Jul 09, 2015 at 06:18:26PM -0400, Doug Ledford wrote:

> A lot of this discussion is interesting and has gone off in an area that
> I think is worth pursuing in the long run. However, in the short run,
> this patch was a minor cleanup/simplification patch. These discussions
> are moving into complete redesigns and rearchitecting. Steve, I'm OK
> with the cleanup and would prefer that it stay separate from these
> larger issues.

So, I was originally of the view the flags change was fine.

But when I realized that it basically hides a
ib_get_dma_mr(IB_ACCESS_REMOTE_WRITE) behind an opaque API:

rdma_get_dma_mr(RDMA_MRR_READ_DEST)

I changed my mind.

For iWarp the above creates a rkey that can RDMA WRITE to any place in
system memory. If a remote guesses that rkey your system is totally
compromised. So it is insecure, and contrary to the advice in RFC5042.

Seeing rdma_get_dma_mr(RDMA_MRR_READ_DEST) added all over the code
base terrifies me, because it is utterly wrong, and looks harmless.

The mistep, is that enabling iSER for iWarp is not just this trivial
change:

- device->mr = ib_get_dma_mr(device->pd, IB_ACCESS_LOCAL_WRITE);
+ mr_roles = RDMA_MRR_RECV | RDMA_MRR_SEND | RDMA_MRR_WRITE_SOURCE |
+ RDMA_MRR_READ_DEST;
+ device->mr = rdma_get_dma_mr(device->pd, mr_roles, 0);

But, it must also follow the path of NFS and use FRMR on iWarp for
RDMA READ lkeys. This is the real quirk of iWarp, not that the MR
flags are slightly different.

>From there, it is a logical wish: If Steve is going to FRMR'ize iSER
to be acceptable security wise, I'd rather see that work done on in a
general way. Hence the API discussion.

What do you think?

Jason