Re: [PATCH for-next v2 2/2] RDMA/rxe: Protect user space index loads/stores

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, May 25, 2021 at 11:51:40PM -0500, Bob Pearson wrote:
> Modify the queue APIs to protect all user space index loads
> with smp_load_acquire() and all user space index stores with
> smp_store_release(). Base this on the types of the queues which
> can be one of ..KERNEL, ..FROM_USER, ..TO_USER. Kernel space
> indices are protected by locks which also provide memory barriers.
> 
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> v2:
>   In v2 use queue type to selectively protect user space indices.
>  drivers/infiniband/sw/rxe/rxe_queue.h | 168 ++++++++++++++++++--------
>  1 file changed, 117 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_queue.h b/drivers/infiniband/sw/rxe/rxe_queue.h
> index 4512745419f8..6e705e09d357 100644
> +++ b/drivers/infiniband/sw/rxe/rxe_queue.h
> @@ -66,12 +66,22 @@ static inline int queue_empty(struct rxe_queue *q)
>  	u32 prod;
>  	u32 cons;
>  
> -	/* make sure all changes to queue complete before
> -	 * testing queue empty
> -	 */
> -	prod = smp_load_acquire(&q->buf->producer_index);
> -	/* same */
> -	cons = smp_load_acquire(&q->buf->consumer_index);
> +	switch (q->type) {
> +	case QUEUE_TYPE_FROM_USER:
> +		/* protect user space index */
> +		prod = smp_load_acquire(&q->buf->producer_index);
> +		cons = q->buf->consumer_index;

The other issue is you can't store the kernel owned consumer_index in
the 'buf'

It should be stored in 'q' and only on write copied to be buf

Kernel never reads the user memory it writes to

This is why splitting it makes sense because it really needs to be
reading different memory, not just using the correct load primitive

Jason