On 9/21/2018 8:56 PM, Stefan Metzmacher wrote:
Hi,
+ req->Channel = SMB2_CHANNEL_RDMA_V1_INVALIDATE;
+ if (need_invalidate)
+ req->Channel = SMB2_CHANNEL_RDMA_V1;
+ req->ReadChannelInfoOffset =
+ offsetof(struct smb2_read_plain_req, Buffer);
+ req->ReadChannelInfoLength =
+ sizeof(struct smbd_buffer_descriptor_v1);
+ v1 = (struct smbd_buffer_descriptor_v1 *) &req->Buffer[0];
+ v1->offset = rdata->mr->mr->iova;
It's unnecessary, and possibly leaking kernel information, to use
the IOVA as the offset of a memory region which is registered using
an FRWR. Because such regions are based on the exact bytes targeted
by the memory handle, the offset can be set to any value, typically
zero, but nearly arbitrary. As long as the (offset + length) does
not wrap or otherwise overflow, offset can be set to anything
convenient.
Since SMB reads and writes range up to 8MB, I'd suggest zeroing the
least significant 23 bits, which should guarantee it. The other 41
bits, party on. You could randomize them, pass some clever identifier
such as MID sequence, whatever.
I just tested that setting:
mr->iova &= (PAGE_SIZE - 1);
mr->iova |= 0xFFFFFFFF00000000;
after the ib_map_mr_sg() and before doing the IB_WR_REG_MR, seems to work.
Good! As you know, we were concerned about it after seeing that
the ib_dma_map_sg() code was unconditionally setting it to the
dma_mapped address. By salting those FFFF's with varying data,
this should give your FRWR regions stronger integrity in addition
to not leaking kernel "addresses" to the wire.
Tom.
