On Fri, Feb 25, 2022 at 01:57:45PM -0600, Bob Pearson wrote:
> Currently the rdma_rxe driver has a security weakness due to adding
> objects which are partially initialized to indices allowing external
> actors to gain access to them by sending packets which refer to
> their index (e.g. qpn, rkey, etc).
>
> This patch adds a member to the pool element struct indicating whether
> the object should/or should not allow looking up from its index. This
> variable is set only after the object is completely created and unset
> as soon as possible when the object is destroyed.
Why do we have to put incompletely initialized pointers into the
xarray?
Either:
1) Do the xa_alloc after everything is setup properly, splitting
allocation and ID assignment.
2) Do xa_alloc(XA_ZERO_ENTRY) at the start to reserve the ID
then xa_store to set the pointer (can't fail) or xa_erase()
to abort it
> @@ -81,4 +82,8 @@ int __rxe_drop_ref(struct rxe_pool_elem *elem);
>
> #define rxe_read_ref(obj) kref_read(&(obj)->elem.ref_cnt)
>
> +#define rxe_enable(obj) ((obj)->elem.enabled = true)
> +
> +#define rxe_disable(obj) ((obj)->elem.enabled = false)
None of this is locked properly. A release/acquire needs to happen to
ensure all the stores that initialized the memory are visible to the
reader. Both of the above will ensure that happens.
Jason
