| Message ID | 396b3fe2-6a84-efed-07de-3e6381009ad1@sandisk.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 11/21/2016 1:21 PM, Bart Van Assche wrote: > The array ib_mad_mgmt_class_table.method_table has MAX_MGMT_CLASS > (80) elements. Hence compare the array index with that value instead > of with IB_MGMT_MAX_METHODS (128). This patch avoids that Coverity > reports the following: > > Overrunning array class->method_table of 80 8-byte elements at element index 127 (byte offset 1016) using index convert_mgmt_class(mad_hdr->mgmt_class) (which evaluates to 127). > > Fixes: commit b7ab0b19a85f ("IB/mad: Verify mgmt class in received MADs") > Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> Reviewed-by: Hal Rosenstock <hal@mellanox.com> > Cc: Sean Hefty <sean.hefty@intel.com> > Cc: <stable@vger.kernel.org> > --- > drivers/infiniband/core/mad.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c > index 40cbd6b..2395fe2 100644 > --- a/drivers/infiniband/core/mad.c > +++ b/drivers/infiniband/core/mad.c > @@ -1746,7 +1746,7 @@ find_mad_agent(struct ib_mad_port_private *port_priv, > if (!class) > goto out; > if (convert_mgmt_class(mad_hdr->mgmt_class) >= > - IB_MGMT_MAX_METHODS) > + ARRAY_SIZE(class->method_table)) > goto out; > method = class->method_table[convert_mgmt_class( > mad_hdr->mgmt_class)]; > I think there is also similar thing in missing check in ib_register_mad_agent where: /* * Make sure MAD registration (if supplied) * is non overlapping with any existing ones */ if (mad_reg_req) { mgmt_class = convert_mgmt_class(mad_reg_req->mgmt_class); if (!is_vendor_class(mgmt_class)) { class = port_priv->version[mad_reg_req-> mgmt_class_version].class; if (class) { method = class->method_table[mgmt_class]; so here the class' method_table is also accessed without checking mgmt_class for range violation, right ? -- Hal -- 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 11/21/2016 12:17 PM, Hal Rosenstock wrote: > I think there is also similar thing in missing check in ib_register_mad_agent where: > > /* > * Make sure MAD registration (if supplied) > * is non overlapping with any existing ones > */ > if (mad_reg_req) { > mgmt_class = convert_mgmt_class(mad_reg_req->mgmt_class); > if (!is_vendor_class(mgmt_class)) { > class = port_priv->version[mad_reg_req-> > mgmt_class_version].class; > if (class) { > method = class->method_table[mgmt_class]; > > so here the class' method_table is also accessed without checking mgmt_class for range violation, right ? Hello Hal, I think such a check is already present in ib_register_mad_agent(): if (mad_reg_req->mgmt_class >= MAX_MGMT_CLASS) { /* * IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE is the only * one in this range currently allowed */ if (mad_reg_req->mgmt_class != IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE) { dev_notice(&device->dev, "%s: Invalid Mgmt Class 0x%x\n", __func__, mad_reg_req->mgmt_class); goto error1; } } [ ... ] Bart. -- 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
Hi Bart, On 11/21/2016 3:52 PM, Bart Van Assche wrote: > Hello Hal, > > I think such a check is already present in ib_register_mad_agent(): > > if (mad_reg_req->mgmt_class >= MAX_MGMT_CLASS) { You're right. -- Hal -- 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
PiANCj4gVGhlIGFycmF5IGliX21hZF9tZ210X2NsYXNzX3RhYmxlLm1ldGhvZF90YWJsZSBoYXMg TUFYX01HTVRfQ0xBU1MNCj4gKDgwKSBlbGVtZW50cy4gSGVuY2UgY29tcGFyZSB0aGUgYXJyYXkg aW5kZXggd2l0aCB0aGF0IHZhbHVlIGluc3RlYWQgb2Ygd2l0aA0KPiBJQl9NR01UX01BWF9NRVRI T0RTICgxMjgpLiBUaGlzIHBhdGNoIGF2b2lkcyB0aGF0IENvdmVyaXR5IHJlcG9ydHMgdGhlDQo+ IGZvbGxvd2luZzoNCj4gDQo+IE92ZXJydW5uaW5nIGFycmF5IGNsYXNzLT5tZXRob2RfdGFibGUg b2YgODAgOC1ieXRlIGVsZW1lbnRzIGF0IGVsZW1lbnQgaW5kZXgNCj4gMTI3IChieXRlIG9mZnNl dCAxMDE2KSB1c2luZyBpbmRleCBjb252ZXJ0X21nbXRfY2xhc3MobWFkX2hkci0NCj4gPm1nbXRf Y2xhc3MpICh3aGljaCBldmFsdWF0ZXMgdG8gMTI3KS4NCj4gDQo+IEZpeGVzOiBjb21taXQgYjdh YjBiMTlhODVmICgiSUIvbWFkOiBWZXJpZnkgbWdtdCBjbGFzcyBpbiByZWNlaXZlZCBNQURzIikN Cj4gU2lnbmVkLW9mZi1ieTogQmFydCBWYW4gQXNzY2hlIDxiYXJ0LnZhbmFzc2NoZUBzYW5kaXNr LmNvbT4NCj4gQ2M6IFNlYW4gSGVmdHkgPHNlYW4uaGVmdHlAaW50ZWwuY29tPg0KPiBDYzogPHN0 YWJsZUB2Z2VyLmtlcm5lbC5vcmc+DQoNClRoYW5rcyENCg0KUmV2aWV3ZWQtYnk6IElyYSBXZWlu eSA8aXJhLndlaW55QGludGVsLmNvbT4NCg0KPiAtLS0NCj4gIGRyaXZlcnMvaW5maW5pYmFuZC9j b3JlL21hZC5jIHwgMiArLQ0KPiAgMSBmaWxlIGNoYW5nZWQsIDEgaW5zZXJ0aW9uKCspLCAxIGRl bGV0aW9uKC0pDQo+IA0KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9pbmZpbmliYW5kL2NvcmUvbWFk LmMgYi9kcml2ZXJzL2luZmluaWJhbmQvY29yZS9tYWQuYyBpbmRleA0KPiA0MGNiZDZiLi4yMzk1 ZmUyIDEwMDY0NA0KPiAtLS0gYS9kcml2ZXJzL2luZmluaWJhbmQvY29yZS9tYWQuYw0KPiArKysg Yi9kcml2ZXJzL2luZmluaWJhbmQvY29yZS9tYWQuYw0KPiBAQCAtMTc0Niw3ICsxNzQ2LDcgQEAg ZmluZF9tYWRfYWdlbnQoc3RydWN0IGliX21hZF9wb3J0X3ByaXZhdGUNCj4gKnBvcnRfcHJpdiwN Cj4gIAkJCWlmICghY2xhc3MpDQo+ICAJCQkJZ290byBvdXQ7DQo+ICAJCQlpZiAoY29udmVydF9t Z210X2NsYXNzKG1hZF9oZHItPm1nbXRfY2xhc3MpID49DQo+IC0JCQkgICAgSUJfTUdNVF9NQVhf TUVUSE9EUykNCj4gKwkJCSAgICBBUlJBWV9TSVpFKGNsYXNzLT5tZXRob2RfdGFibGUpKQ0KPiAg CQkJCWdvdG8gb3V0Ow0KPiAgCQkJbWV0aG9kID0gY2xhc3MtPm1ldGhvZF90YWJsZVtjb252ZXJ0 X21nbXRfY2xhc3MoDQo+ICAJCQkJCQkJbWFkX2hkci0NCj4gPm1nbXRfY2xhc3MpXTsNCj4gLS0N Cj4gMi4xMC4yDQo+IA0KPiANCj4gLS0NCj4gVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6 IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LXJkbWEiIGluDQo+IHRoZSBib2R5IG9m IGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwub3JnDQo+IE1vcmUgbWFqb3Jkb21v IGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFqb3Jkb21vLWluZm8uaHRtbA0K -- 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
diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c index 40cbd6b..2395fe2 100644 --- a/drivers/infiniband/core/mad.c +++ b/drivers/infiniband/core/mad.c @@ -1746,7 +1746,7 @@ find_mad_agent(struct ib_mad_port_private *port_priv, if (!class) goto out; if (convert_mgmt_class(mad_hdr->mgmt_class) >= - IB_MGMT_MAX_METHODS) + ARRAY_SIZE(class->method_table)) goto out; method = class->method_table[convert_mgmt_class( mad_hdr->mgmt_class)];
The array ib_mad_mgmt_class_table.method_table has MAX_MGMT_CLASS (80) elements. Hence compare the array index with that value instead of with IB_MGMT_MAX_METHODS (128). This patch avoids that Coverity reports the following: Overrunning array class->method_table of 80 8-byte elements at element index 127 (byte offset 1016) using index convert_mgmt_class(mad_hdr->mgmt_class) (which evaluates to 127). Fixes: commit b7ab0b19a85f ("IB/mad: Verify mgmt class in received MADs") Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> Cc: Sean Hefty <sean.hefty@intel.com> Cc: <stable@vger.kernel.org> --- drivers/infiniband/core/mad.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)