| Message ID | 1471907797-81772-1-git-send-email-shiraz.saleem@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 8/22/2016 7:16 PM, Shiraz Saleem wrote: > Add NULL check for pdata and pdata->addr before the memcpy in > i40iw_form_cm_frame(). This fixes a NULL pointer de-reference > which occurs when the MPA private data pointer is NULL. Also > only copy pdata->size bytes in the memcpy to prevent reading > past the length of the private data buffer provided by upper layer. > > Fixes: f27b4746f378 ("i40iw: add connection management code") > > Reported-by: Stefan Assmann <sassmann@redhat.com> > Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com> > Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com> > --- > drivers/infiniband/hw/i40iw/i40iw_cm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c > index 5026dc7..6434398 100644 > --- a/drivers/infiniband/hw/i40iw/i40iw_cm.c > +++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c > @@ -535,8 +535,8 @@ static struct i40iw_puda_buf *i40iw_form_cm_frame(struct i40iw_cm_node *cm_node, > buf += hdr_len; > } > > - if (pd_len) > - memcpy(buf, pdata->addr, pd_len); > + if (pdata && pdata->addr) > + memcpy(buf, pdata->addr, pdata->size); Is there a guarantee that pdata->size is always less than pd_len? Do you need a check here?
On Tue, Aug 23, 2016 at 12:47:35PM -0400, Doug Ledford wrote: > On 8/22/2016 7:16 PM, Shiraz Saleem wrote: > > Add NULL check for pdata and pdata->addr before the memcpy in > > i40iw_form_cm_frame(). This fixes a NULL pointer de-reference > > which occurs when the MPA private data pointer is NULL. Also > > only copy pdata->size bytes in the memcpy to prevent reading > > past the length of the private data buffer provided by upper layer. > > > > Fixes: f27b4746f378 ("i40iw: add connection management code") > > > > Reported-by: Stefan Assmann <sassmann@redhat.com> > > Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com> > > Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com> > > --- > > drivers/infiniband/hw/i40iw/i40iw_cm.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c > > index 5026dc7..6434398 100644 > > --- a/drivers/infiniband/hw/i40iw/i40iw_cm.c > > +++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c > > @@ -535,8 +535,8 @@ static struct i40iw_puda_buf *i40iw_form_cm_frame(struct i40iw_cm_node *cm_node, > > buf += hdr_len; > > } > > > > - if (pd_len) > > - memcpy(buf, pdata->addr, pd_len); > > + if (pdata && pdata->addr) > > + memcpy(buf, pdata->addr, pdata->size); > > Is there a guarantee that pdata->size is always less than pd_len? Do > you need a check here? > pd_len is 'always' greater than or equal to pdata->size. In i40iw_form_cm_frame(), pd_len starts out as pdata->size and then may be incremented by 4. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/23/2016 3:10 PM, Shiraz Saleem wrote: > On Tue, Aug 23, 2016 at 12:47:35PM -0400, Doug Ledford wrote: >> On 8/22/2016 7:16 PM, Shiraz Saleem wrote: >>> Add NULL check for pdata and pdata->addr before the memcpy in >>> i40iw_form_cm_frame(). This fixes a NULL pointer de-reference >>> which occurs when the MPA private data pointer is NULL. Also >>> only copy pdata->size bytes in the memcpy to prevent reading >>> past the length of the private data buffer provided by upper layer. >>> >>> Fixes: f27b4746f378 ("i40iw: add connection management code") >>> >>> Reported-by: Stefan Assmann <sassmann@redhat.com> >>> Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com> >>> Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com> >>> --- >>> drivers/infiniband/hw/i40iw/i40iw_cm.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c >>> index 5026dc7..6434398 100644 >>> --- a/drivers/infiniband/hw/i40iw/i40iw_cm.c >>> +++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c >>> @@ -535,8 +535,8 @@ static struct i40iw_puda_buf *i40iw_form_cm_frame(struct i40iw_cm_node *cm_node, >>> buf += hdr_len; >>> } >>> >>> - if (pd_len) >>> - memcpy(buf, pdata->addr, pd_len); >>> + if (pdata && pdata->addr) >>> + memcpy(buf, pdata->addr, pdata->size); >> >> Is there a guarantee that pdata->size is always less than pd_len? Do >> you need a check here? >> > > pd_len is 'always' greater than or equal to pdata->size. In i40iw_form_cm_frame(), > pd_len starts out as pdata->size and then may be incremented by 4. > Thanks, I'll pull this in then.
On 8/23/2016 4:16 PM, Doug Ledford wrote: > On 8/23/2016 3:10 PM, Shiraz Saleem wrote: >> On Tue, Aug 23, 2016 at 12:47:35PM -0400, Doug Ledford wrote: >>> On 8/22/2016 7:16 PM, Shiraz Saleem wrote: >>>> Add NULL check for pdata and pdata->addr before the memcpy in >>>> i40iw_form_cm_frame(). This fixes a NULL pointer de-reference >>>> which occurs when the MPA private data pointer is NULL. Also >>>> only copy pdata->size bytes in the memcpy to prevent reading >>>> past the length of the private data buffer provided by upper layer. >>>> >>>> Fixes: f27b4746f378 ("i40iw: add connection management code") >>>> >>>> Reported-by: Stefan Assmann <sassmann@redhat.com> >>>> Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com> >>>> Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com> >>>> --- >>>> drivers/infiniband/hw/i40iw/i40iw_cm.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c >>>> index 5026dc7..6434398 100644 >>>> --- a/drivers/infiniband/hw/i40iw/i40iw_cm.c >>>> +++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c >>>> @@ -535,8 +535,8 @@ static struct i40iw_puda_buf *i40iw_form_cm_frame(struct i40iw_cm_node *cm_node, >>>> buf += hdr_len; >>>> } >>>> >>>> - if (pd_len) >>>> - memcpy(buf, pdata->addr, pd_len); >>>> + if (pdata && pdata->addr) >>>> + memcpy(buf, pdata->addr, pdata->size); >>> >>> Is there a guarantee that pdata->size is always less than pd_len? Do >>> you need a check here? >>> >> >> pd_len is 'always' greater than or equal to pdata->size. In i40iw_form_cm_frame(), >> pd_len starts out as pdata->size and then may be incremented by 4. >> > > Thanks, I'll pull this in then. > Applied.
diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c index 5026dc7..6434398 100644 --- a/drivers/infiniband/hw/i40iw/i40iw_cm.c +++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c @@ -535,8 +535,8 @@ static struct i40iw_puda_buf *i40iw_form_cm_frame(struct i40iw_cm_node *cm_node, buf += hdr_len; } - if (pd_len) - memcpy(buf, pdata->addr, pd_len); + if (pdata && pdata->addr) + memcpy(buf, pdata->addr, pdata->size); atomic_set(&sqbuf->refcount, 1);
Add NULL check for pdata and pdata->addr before the memcpy in i40iw_form_cm_frame(). This fixes a NULL pointer de-reference which occurs when the MPA private data pointer is NULL. Also only copy pdata->size bytes in the memcpy to prevent reading past the length of the private data buffer provided by upper layer. Fixes: f27b4746f378 ("i40iw: add connection management code") Reported-by: Stefan Assmann <sassmann@redhat.com> Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com> Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com> --- drivers/infiniband/hw/i40iw/i40iw_cm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)