RE: [PATCH for-next v2 2/4] IB/hfi1: Move rvt_cq_wc struct into uapi directory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> > I think you should change the definition to be:
> 
> > #define RDMA_ATOMIC_UAPI(_type, _name) struct {_type atomic_val;} 
> > _name
> 
> > And then add a kernel function
> 
> > #define RDMA_READ_UAPI_ATOMIC(_member) 
> > READ_ONCE((_memb)->atomic_val)
> 
> > So that the compiler protects against this mistake.
> 
> There is already a producer and consumer lock for the circular buffer 
> and only reading the opposition index needs the READ_ONCE control.

> No. 
> Any read of memory mmap'd into user space must use READ_ONCE or risk a security problem due to control-flow execution corruption.

As per your suggestion above, I have draft a quick prototype to understand and clear any assumption beforehand.  Below are 3 macros that will be introduce
in the rvt-abi header file.  The RDMA_READ_UAPI_ATOMIC and RDMA_WRITE_UAPI_ATOMIC macro are only applied to struct mmaped to  userspace. 

#define RDMA_ATOMIC_UAPI(_type, _name)struct{ _type val;}_name
#define RDMA_READ_UAPI_ATOMIC(member) READ_ONCE((member).val)
#define RDMA_WRITE_UAPI_ATOMIC(member, x) WRITE_ONCE((member).val, x)
struct rvt_cq_wc {
	/* index of next entry to fill */
	RDMA_ATOMIC_UAPI(__u32, head);
	/* index of next ib_poll_cq() entry */
	RDMA_ATOMIC_UAPI(__u32, tail);

	/* these are actually size ibcq.cqe + 1 */
	struct ib_uverbs_wc uqueue[];
};

Here is the use case of RDMA_READ_UAPI_ATOMIC macro  in cq.c file.  The READ_ONCE will be applied for any read on the indices of mmaped to user space. 
if (cq->ip) {
		u_wc = cq->queue;
		uqueue = &u_wc->uqueue[0];
		head = RDMA_READ_UAPI_ATOMIC(u_wc->head);
		tail = RDMA_READ_UAPI_ATOMIC(u_wc->tail);
	} else {
		k_wc = cq->kqueue;
		kqueue = &k_wc->kqueue[0];
		head = k_wc->head;
		tail = k_wc->tail;
	}


>Same story for WRITE_ONCE.

Here is the usage for RDMA_WRITE_UAPI_ATOMIC macro. For cq enter, we kept smp_wmb() in the code as it is now and we didn't adopt it into this macro. This macro
Is meant for compile barrier.

cq->ibcq.cqe = cqe;
	if (u_wc) {
		RDMA_WRITE_UAPI_ATOMIC(u_wc->head, n);
		RDMA_WRITE_UAPI_ATOMIC(u_wc->tail, 0);
		cq->queue = u_wc;
	} else {
		k_wc->head = n;
		k_wc->tail = 0;
		cq->kqueue = k_wc;
	}





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux