Question on PD validation

Question on PD validation

[Devale, Sindhu]

Hi all,

Currently, when an application creates a PD, the ib uverbs creates a PD uobj resource and tracks it through the xarray which is looked up using an uobj id/pd_handle. 

If a user application were to create a verb resource, example QP, with some random ibv_pd object  [i.e. one not allocated by user process], whose pd_handle happens to match the id of created PDs, the QP creation would succeed though one would expect it to fail For example:
During an alloc PD:
irdma_ualloc_pd, 122], pd_id: 44, ibv_pd: 0x8887c0, pd_handle: 0 

During create QP:
[irdma_ucreate_qp, 1480], ibv_pd: 0x8889f0, pd_handle: 0


Clearly, the ibv_pd that the application wants the QP to be associated with is not the same as the ibv_pd created during the allocation of PD. Yet, the creation of the QP is successful as the pd handle of 0 matches. 

It appears there is missing infrastructure to check if the incoming ibv_pd during create QP, MR etc is a valid one.

Looking for inputs.

Thank you,
Sindhu

Re: Question on PD validation

[Jason Gunthorpe]

On Mon, Oct 11, 2021 at 11:05:02PM +0000, Devale, Sindhu wrote:
> Hi all,
> 
> Currently, when an application creates a PD, the ib uverbs creates a PD uobj resource and tracks it through the xarray which is looked up using an uobj id/pd_handle. 
> 
> If a user application were to create a verb resource, example QP, with some random ibv_pd object  [i.e. one not allocated by user process], whose pd_handle happens to match the id of created PDs, the QP creation would succeed though one would expect it to fail For example:
> During an alloc PD:
> irdma_ualloc_pd, 122], pd_id: 44, ibv_pd: 0x8887c0, pd_handle: 0 
> 
> During create QP:
> [irdma_ucreate_qp, 1480], ibv_pd: 0x8889f0, pd_handle: 0
> 
> 
> Clearly, the ibv_pd that the application wants the QP to be
> associated with is not the same as the ibv_pd created during the
> allocation of PD. Yet, the creation of the QP is successful as the
> pd handle of 0 matches.

Most likely handle 0 is a PD, generally all uobj's require a PD to be
created so PD is usually the thing in slot 0.

The validation that the index type matches is done here:

	UVERBS_ATTR_IDR(UVERBS_ATTR_CREATE_QP_PD_HANDLE,
			UVERBS_OBJECT_PD,
			UVERBS_ACCESS_READ,
			UA_OPTIONAL),
Which is passed into this:

static int uverbs_process_attr(struct bundle_priv *pbundle,
			       const struct uverbs_api_attr *attr_uapi,
			       struct ib_uverbs_attr *uattr, u32 attr_bkey)
{
	case UVERBS_ATTR_TYPE_IDR:
		o_attr->uobject = uverbs_get_uobject_from_file(
			spec->u.obj.obj_type, spec->u.obj.access,
			uattr->data_s64, &pbundle->bundle);

Which eventually goes down into this check:


struct ib_uobject *rdma_lookup_get_uobject(const struct uverbs_api_object *obj,
					   struct ib_uverbs_file *ufile, s64 id,
					   enum rdma_lookup_mode mode,
					   struct uverbs_attr_bundle *attrs)
{

		if (uobj->uapi_object != obj) {
			ret = -EINVAL;
			goto free;
		}

Which check the uobj the user provided is the same type as the schema
requires.

The legacy path is similar, we start here:

		pd = uobj_get_obj_read(pd, UVERBS_OBJECT_PD, cmd->pd_handle,
				       attrs);

Which also calls rdma_lookup_get_uobject() and does the same check.

Jason

RE: Question on PD validation

[Devale, Sindhu]

> -----Original Message-----
> From: Jason Gunthorpe <jgg@ziepe.ca>
> Sent: Tuesday, October 12, 2021 11:10 AM
> To: Devale, Sindhu <sindhu.devale@intel.com>
> Cc: linux-rdma@vger.kernel.org; Saleem, Shiraz <shiraz.saleem@intel.com>
> Subject: Re: Question on PD validation
> 
> On Mon, Oct 11, 2021 at 11:05:02PM +0000, Devale, Sindhu wrote:
> > Hi all,
> >
> > Currently, when an application creates a PD, the ib uverbs creates a PD
> uobj resource and tracks it through the xarray which is looked up using an
> uobj id/pd_handle.
> >
> > If a user application were to create a verb resource, example QP, with
> some random ibv_pd object  [i.e. one not allocated by user process], whose
> pd_handle happens to match the id of created PDs, the QP creation would
> succeed though one would expect it to fail For example:
> > During an alloc PD:
> > irdma_ualloc_pd, 122], pd_id: 44, ibv_pd: 0x8887c0, pd_handle: 0
> >
> > During create QP:
> > [irdma_ucreate_qp, 1480], ibv_pd: 0x8889f0, pd_handle: 0
> >
> >
> > Clearly, the ibv_pd that the application wants the QP to be associated
> > with is not the same as the ibv_pd created during the allocation of
> > PD. Yet, the creation of the QP is successful as the pd handle of 0
> > matches.
> 
> Most likely handle 0 is a PD, generally all uobj's require a PD to be created so
> PD is usually the thing in slot 0.
> 
> The validation that the index type matches is done here:
> 
> 	UVERBS_ATTR_IDR(UVERBS_ATTR_CREATE_QP_PD_HANDLE,
> 			UVERBS_OBJECT_PD,
> 			UVERBS_ACCESS_READ,
> 			UA_OPTIONAL),
> Which is passed into this:
> 
> static int uverbs_process_attr(struct bundle_priv *pbundle,
> 			       const struct uverbs_api_attr *attr_uapi,
> 			       struct ib_uverbs_attr *uattr, u32 attr_bkey) {
> 	case UVERBS_ATTR_TYPE_IDR:
> 		o_attr->uobject = uverbs_get_uobject_from_file(
> 			spec->u.obj.obj_type, spec->u.obj.access,
> 			uattr->data_s64, &pbundle->bundle);
> 
> Which eventually goes down into this check:
> 
> 
> struct ib_uobject *rdma_lookup_get_uobject(const struct uverbs_api_object
> *obj,
> 					   struct ib_uverbs_file *ufile, s64 id,
> 					   enum rdma_lookup_mode mode,
> 					   struct uverbs_attr_bundle *attrs) {
> 
> 		if (uobj->uapi_object != obj) {
> 			ret = -EINVAL;
> 			goto free;
> 		}
> 
> Which check the uobj the user provided is the same type as the schema
> requires.
> 
> The legacy path is similar, we start here:
> 
> 		pd = uobj_get_obj_read(pd, UVERBS_OBJECT_PD, cmd-
> >pd_handle,
> 				       attrs);
> 
> Which also calls rdma_lookup_get_uobject() and does the same check.
> 
> Jason

Hi Jason,

Thank you for responding. 

>struct ib_uobject *rdma_lookup_get_uobject(const struct uverbs_api_object *obj,
					   struct ib_uverbs_file *ufile, s64 id,
					   enum rdma_lookup_mode mode,
					   struct uverbs_attr_bundle *attrs) {
					
The lookup for a uobj in the above function happens based on the uobj id.

When an application creates a PD, ib_uverbs_alloc_pd() creates a uobj for the corresponding and assigns id of the uobj to the response pd_handle:

resp.pd_handle = uobj->id;

For example, I am creating two PDs:

ibv_pd: 0x21a4d00, pd_handle: 0
[ib_uverbs_alloc_pd, 458], allocated: 00000000d8facf77, uobject: 000000001584c2c2, pd_handle: 0

ibv_pd: 0x21b7a70, pd_handle: 1
[ib_uverbs_alloc_pd, 458], allocated: 0000000048001c84, uobject: 00000000a9cacf67, pd_handle: 1


Now, if a rogue application tries to create a QP using a different PD other than the above two but it's pd_handle "HAPPENS" to match one of the above:

What's going into create_qp:

ibv_pd: 0x21b3c90, pd_handle: 0
[create_qp, 1392], cmd->pd_handle: 0
[rdma_lookup_get_uobject, 381], id to lookup: 0
[rdma_lookup_get_uobject, 396], uobj retrieved: 000000001584c2c2
[create_qp, 1399], pd: 00000000d8facf77

It creates the QP with this invalid PD as the core retrieves the PD uobj based on pd_handle 0. 

Is this the correct behavior? 

Can we do the lookup of uobj based on the object itself instead of the uobj id?

Thank you,
Sindhu    

Re: Question on PD validation

[Jason Gunthorpe]

On Fri, Oct 15, 2021 at 11:09:37PM +0000, Devale, Sindhu wrote:
> 
> 
> > From: Jason Gunthorpe <jgg@ziepe.ca>
> > Sent: Tuesday, October 12, 2021 11:10 AM
> > To: Devale, Sindhu <sindhu.devale@intel.com>
> > Cc: linux-rdma@vger.kernel.org; Saleem, Shiraz <shiraz.saleem@intel.com>
> > Subject: Re: Question on PD validation
> > 
> > On Mon, Oct 11, 2021 at 11:05:02PM +0000, Devale, Sindhu wrote:
> > > Hi all,
> > >
> > > Currently, when an application creates a PD, the ib uverbs creates a PD
> > uobj resource and tracks it through the xarray which is looked up using an
> > uobj id/pd_handle.
> > >
> > > If a user application were to create a verb resource, example QP, with
> > some random ibv_pd object  [i.e. one not allocated by user process], whose
> > pd_handle happens to match the id of created PDs, the QP creation would
> > succeed though one would expect it to fail For example:
> > > During an alloc PD:
> > > irdma_ualloc_pd, 122], pd_id: 44, ibv_pd: 0x8887c0, pd_handle: 0
> > >
> > > During create QP:
> > > [irdma_ucreate_qp, 1480], ibv_pd: 0x8889f0, pd_handle: 0
> > >
> > >
> > > Clearly, the ibv_pd that the application wants the QP to be associated
> > > with is not the same as the ibv_pd created during the allocation of
> > > PD. Yet, the creation of the QP is successful as the pd handle of 0
> > > matches.
> > 
> > Most likely handle 0 is a PD, generally all uobj's require a PD to be created so
> > PD is usually the thing in slot 0.
> > 
> > The validation that the index type matches is done here:
> > 
> > 	UVERBS_ATTR_IDR(UVERBS_ATTR_CREATE_QP_PD_HANDLE,
> > 			UVERBS_OBJECT_PD,
> > 			UVERBS_ACCESS_READ,
> > 			UA_OPTIONAL),
> > Which is passed into this:
> > 
> > static int uverbs_process_attr(struct bundle_priv *pbundle,
> > 			       const struct uverbs_api_attr *attr_uapi,
> > 			       struct ib_uverbs_attr *uattr, u32 attr_bkey) {
> > 	case UVERBS_ATTR_TYPE_IDR:
> > 		o_attr->uobject = uverbs_get_uobject_from_file(
> > 			spec->u.obj.obj_type, spec->u.obj.access,
> > 			uattr->data_s64, &pbundle->bundle);
> > 
> > Which eventually goes down into this check:
> > 
> > 
> > struct ib_uobject *rdma_lookup_get_uobject(const struct uverbs_api_object
> > *obj,
> > 					   struct ib_uverbs_file *ufile, s64 id,
> > 					   enum rdma_lookup_mode mode,
> > 					   struct uverbs_attr_bundle *attrs) {
> > 
> > 		if (uobj->uapi_object != obj) {
> > 			ret = -EINVAL;
> > 			goto free;
> > 		}
> > 
> > Which check the uobj the user provided is the same type as the schema
> > requires.
> > 
> > The legacy path is similar, we start here:
> > 
> > 		pd = uobj_get_obj_read(pd, UVERBS_OBJECT_PD, cmd-
> > >pd_handle,
> > 				       attrs);
> > 
> > Which also calls rdma_lookup_get_uobject() and does the same check.
> > 
> > Jason
> 
> Hi Jason,
> 
> Thank you for responding. 
> 
> >struct ib_uobject *rdma_lookup_get_uobject(const struct uverbs_api_object *obj,
> 					   struct ib_uverbs_file *ufile, s64 id,
> 					   enum rdma_lookup_mode mode,
> 					   struct uverbs_attr_bundle *attrs) {
> 					
> The lookup for a uobj in the above function happens based on the uobj id.
> 
> When an application creates a PD, ib_uverbs_alloc_pd() creates a uobj for the corresponding and assigns id of the uobj to the response pd_handle:
> 
> resp.pd_handle = uobj->id;
> 
> For example, I am creating two PDs:
> 
> ibv_pd: 0x21a4d00, pd_handle: 0
> [ib_uverbs_alloc_pd, 458], allocated: 00000000d8facf77, uobject: 000000001584c2c2, pd_handle: 0
> 
> ibv_pd: 0x21b7a70, pd_handle: 1
> [ib_uverbs_alloc_pd, 458], allocated: 0000000048001c84, uobject: 00000000a9cacf67, pd_handle: 1
> 
> What's going into create_qp:
> 
> ibv_pd: 0x21b3c90, pd_handle: 0
> [create_qp, 1392], cmd->pd_handle: 0
> [rdma_lookup_get_uobject, 381], id to lookup: 0
> [rdma_lookup_get_uobject, 396], uobj retrieved: 000000001584c2c2
> [create_qp, 1399], pd: 00000000d8facf77

I don't know what you are trying to explain. You allocated a PD on
handle 0 and asked for a PD on handle 0 - and got back the same
pointer.

What is the problem?

> Now, if a rogue application tries to create a QP using a different
> PD other than the above two but it's pd_handle "HAPPENS" to match
> one of the above:

The xarray that holds the handles is scoped inside the ib_uverbs_file
- which is unique per file descriptor. A "rouge application" cannot
access handles outside its file descriptor.

Jason