Re: [PATCH rdma-next 6/6] RDMA/srpt: Use private_data_len instead of hardcoded value

[Jason Gunthorpe <jgg@xxxxxxxx>]

On Sun, Oct 20, 2019 at 10:15:59AM +0300, Leon Romanovsky wrote:
> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> index daf811abf40a..e66366de11e9 100644
> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> @@ -2609,7 +2609,7 @@ static int srpt_cm_handler(struct ib_cm_id *cm_id,
>  	case IB_CM_REJ_RECEIVED:
>  		srpt_cm_rej_recv(ch, event->param.rej_rcvd.reason,
>  				 event->private_data,
> -				 IB_CM_REJ_PRIVATE_DATA_SIZE);
> +				 event->private_data_len);

So, I took a look and found a heck of a lot more places assuming the
size of private data that really should be checked if we are going to
introduce a buffer length here.

This is the first couple I noticed, but there were many many more and
they all should be handled..

--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -2677,9 +2677,14 @@ static void srp_ib_cm_rej_handler(struct ib_cm_id *cm_id,
                break;
 
        case IB_CM_REJ_CONSUMER_DEFINED:
+               if (event->private_data_len < sizeof(struct srp_login_rej)) {
+                       ch->status = -ECONNRESET;
+                       break;
+               }
+

--- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
@@ -985,12 +985,15 @@ static int ipoib_cm_rep_handler(struct ib_cm_id *cm_id,
 {
        struct ipoib_cm_tx *p = cm_id->context;
        struct ipoib_dev_priv *priv = ipoib_priv(p->dev);
        struct ipoib_cm_data *data = event->private_data;
        struct sk_buff_head skqueue;
        struct ib_qp_attr qp_attr;
        int qp_attr_mask, ret;
        struct sk_buff *skb;
 
+       if (event->private_data_len < sizeof(*data))
+               return -EINVAL;
+


Thanks,
Jason