mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
Subsystems (3)
PATCH 1/1 mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
The problem is that on 64bit systems then we don't clear the higher bits of the "pending" variable. So when we do:
ack = pending & ~BIT(STMFX_REG_IRQ_SRC_EN_GPIO);
if (ack) {the if (ack) condition relies on uninitialized data. The fix it that I've changed "pending" from an unsigned long to a u32. I changed "n" as well, because that's a number in the 0-10 range and it fits easily inside an int. We do need to add a cast to "pending" when we use it in the for_each_set_bit() loop, but that doesn't cause a proble, it's fine.
--- drivers/mfd/stmfx.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/mfd/stmfx.c +4 -4
@@ -204,12 +204,12 @@ static struct irq_chip stmfx_irq_chip = {static irqreturn_t stmfx_irq_handler (int irq,void *data) {struct stmfx *stmfx = data; -unsigned long n, pending; +u32 pending;u32 ack; +int n;int ret; - ret =regmap_read (stmfx->map,STMFX_REG_IRQ_PENDING , - (u32 *)&pending); + ret =regmap_read (stmfx->map,STMFX_REG_IRQ_PENDING , &pending);if (ret)return IRQ_NONE ;
@@ -224,7 +224,7 @@ static irqreturn_t stmfx_irq_handler(int irq, void *data)return IRQ_NONE ; } -for_each_set_bit (n, &pending,STMFX_REG_IRQ_SRC_MAX ) +for_each_set_bit (n, (unsigned long *)&pending,STMFX_REG_IRQ_SRC_MAX )handle_nested_irq (irq_find_mapping (stmfx->irq_domain, n));return IRQ_HANDLED ;
Signature · 1 line
2.20.1
Quoted 13 lines · Dan Carpenter
The problem is that on 64bit systems then we don't clear the higher bits of the "pending" variable. So when we do:
ack = pending & ~BIT(STMFX_REG_IRQ_SRC_EN_GPIO); if (ack) {the if (ack) condition relies on uninitialized data. The fix it that I've changed "pending" from an unsigned long to a u32. I changed "n" as well, because that's a number in the 0-10 range and it fits easily inside an int. We do need to add a cast to "pending" when we use it in the
for_each_set_bit()loop, but that doesn't cause a proble, it's fine.
Quoted 5 lines
Fixes 06252ade9156 ("mfd: Add ST Multi-Function eXpander (STMFX) core driver")Signed-off-by Dan Carpenter <[email protected]>--- drivers/mfd/stmfx.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Could do with an author's Ack here.
Signature · 4 lines
Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
SGkgRGFuLA0KDQpUaGFua3MgZm9yIHlvdXIgcGF0Y2guIE9uZSBtaW5vciBjb21tZW50Og0KDQpP biA1LzE1LzE5IDExOjMxIEFNLCBEYW4gQ2FycGVudGVyIHdyb3RlOg0KPiBUaGUgcHJvYmxlbSBp cyB0aGF0IG9uIDY0Yml0IHN5c3RlbXMgdGhlbiB3ZSBkb24ndCBjbGVhciB0aGUgaGlnaGVyDQo+ IGJpdHMgb2YgdGhlICJwZW5kaW5nIiB2YXJpYWJsZS4gIFNvIHdoZW4gd2UgZG86DQo+IA0KPiAJ YWNrID0gcGVuZGluZyAmIH5CSVQoU1RNRlhfUkVHX0lSUV9TUkNfRU5fR1BJTyk7DQo+IAlpZiAo YWNrKSB7DQo+IA0KPiB0aGUgaWYgKGFjaykgY29uZGl0aW9uIHJlbGllcyBvbiB1bmluaXRpYWxp emVkIGRhdGEuICBUaGUgZml4IGl0IHRoYXQNCj4gSSd2ZSBjaGFuZ2VkICJwZW5kaW5nIiBmcm9t IGFuIHVuc2lnbmVkIGxvbmcgdG8gYSB1MzIuICBJIGNoYW5nZWQgIm4iIGFzDQo+IHdlbGwsIGJl Y2F1c2UgdGhhdCdzIGEgbnVtYmVyIGluIHRoZSAwLTEwIHJhbmdlIGFuZCBpdCBmaXRzIGVhc2ls eQ0KPiBpbnNpZGUgYW4gaW50LiAgV2UgZG8gbmVlZCB0byBhZGQgYSBjYXN0IHRvICJwZW5kaW5n IiB3aGVuIHdlIHVzZSBpdCBpbg0KPiB0aGUgZm9yX2VhY2hfc2V0X2JpdCgpIGxvb3AsIGJ1dCB0 aGF0IGRvZXNuJ3QgY2F1c2UgYSBwcm9ibGUsIGl0J3MNCj4gZmluZS4NCj4gDQo+IEZpeGVzOiAw NjI1MmFkZTkxNTYgKCJtZmQ6IEFkZCBTVCBNdWx0aS1GdW5jdGlvbiBlWHBhbmRlciAoU1RNRlgp IGNvcmUgZHJpdmVyIikNCj4gU2lnbmVkLW9mZi1ieTogRGFuIENhcnBlbnRlciA8ZGFuLmNhcnBl bnRlckBvcmFjbGUuY29tPg0KPiAtLS0NCj4gICBkcml2ZXJzL21mZC9zdG1meC5jIHwgOCArKysr LS0tLQ0KPiAgIDEgZmlsZSBjaGFuZ2VkLCA0IGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25zKC0p DQo+IA0KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9tZmQvc3RtZnguYyBiL2RyaXZlcnMvbWZkL3N0 bWZ4LmMNCj4gaW5kZXggZmU4ZWZiYTJkNDVmLi5mZWU3NWI1ZDA5OGUgMTAwNjQ0DQo+IC0tLSBh L2RyaXZlcnMvbWZkL3N0bWZ4LmMNCj4gKysrIGIvZHJpdmVycy9tZmQvc3RtZnguYw0KPiBAQCAt MjA0LDEyICsyMDQsMTIgQEAgc3RhdGljIHN0cnVjdCBpcnFfY2hpcCBzdG1meF9pcnFfY2hpcCA9 IHsNCj4gICBzdGF0aWMgaXJxcmV0dXJuX3Qgc3RtZnhfaXJxX2hhbmRsZXIoaW50IGlycSwgdm9p ZCAqZGF0YSkNCj4gICB7DQo+ICAgCXN0cnVjdCBzdG1meCAqc3RtZnggPSBkYXRhOw0KPiAtCXVu c2lnbmVkIGxvbmcgbiwgcGVuZGluZzsNCj4gKwl1MzIgcGVuZGluZzsNCj4gICAJdTMyIGFjazsN Cj4gKwlpbnQgbjsNCj4gICAJaW50IHJldDsNCg0KQ291bGQgeW91IGdyb3VwOg0KDQp1MzIgcGVu ZGluZywgYWNrOw0KaW50IG4sIHJldDsNCg0KPiAgIA0KPiAtCXJldCA9IHJlZ21hcF9yZWFkKHN0 bWZ4LT5tYXAsIFNUTUZYX1JFR19JUlFfUEVORElORywNCj4gLQkJCSAgKHUzMiAqKSZwZW5kaW5n KTsNCj4gKwlyZXQgPSByZWdtYXBfcmVhZChzdG1meC0+bWFwLCBTVE1GWF9SRUdfSVJRX1BFTkRJ TkcsICZwZW5kaW5nKTsNCj4gICAJaWYgKHJldCkNCj4gICAJCXJldHVybiBJUlFfTk9ORTsNCj4g ICANCj4gQEAgLTIyNCw3ICsyMjQsNyBAQCBzdGF0aWMgaXJxcmV0dXJuX3Qgc3RtZnhfaXJxX2hh bmRsZXIoaW50IGlycSwgdm9pZCAqZGF0YSkNCj4gICAJCQlyZXR1cm4gSVJRX05PTkU7DQo+ICAg CX0NCj4gICANCj4gLQlmb3JfZWFjaF9zZXRfYml0KG4sICZwZW5kaW5nLCBTVE1GWF9SRUdfSVJR X1NSQ19NQVgpDQo+ICsJZm9yX2VhY2hfc2V0X2JpdChuLCAodW5zaWduZWQgbG9uZyAqKSZwZW5k aW5nLCBTVE1GWF9SRUdfSVJRX1NSQ19NQVgpDQo+ICAgCQloYW5kbGVfbmVzdGVkX2lycShpcnFf ZmluZF9tYXBwaW5nKHN0bWZ4LT5pcnFfZG9tYWluLCBuKSk7DQo+ICAgDQo+ICAgCXJldHVybiBJ UlFfSEFORExFRDsNCj4gDQoNCkkndmUgdGVzdGVkIGl0IG9uIHN0bTMybXAxNTdjLWV2MSwgc28g eW91IGNhbiBhZGQgbXkNClRlc3RlZC1ieTogQW1lbGllIERlbGF1bmF5IDxhbWVsaWUuZGVsYXVu YXlAc3QuY29tPg0KDQpSZWdhcmRzLA0KQW1lbGll
PATCH v2 1/1 mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
The problem is that on 64bit systems then we don't clear the higher bits of the "pending" variable. So when we do:
ack = pending & ~BIT(STMFX_REG_IRQ_SRC_EN_GPIO);
if (ack) {the if (ack) condition relies on uninitialized data. The fix it that I've changed "pending" from an unsigned long to a u32. I changed "n" as well, because that's a number in the 0-10 range and it fits easily inside an int. We do need to add a cast to "pending" when we use it in the for_each_set_bit() loop, but that doesn't cause a proble, it's fine.
---
v2: white space changes
drivers/mfd/stmfx.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/mfd/stmfx.c +4 -6
@@ -204,12 +204,10 @@ static struct irq_chip stmfx_irq_chip = {static irqreturn_t stmfx_irq_handler (int irq,void *data) {struct stmfx *stmfx = data; -unsigned long n, pending; -u32 ack; -int ret; +u32 pending, ack; +int n, ret; - ret =regmap_read (stmfx->map,STMFX_REG_IRQ_PENDING , - (u32 *)&pending); + ret =regmap_read (stmfx->map,STMFX_REG_IRQ_PENDING , &pending);if (ret)return IRQ_NONE ;
@@ -224,7 +222,7 @@ static irqreturn_t stmfx_irq_handler(int irq, void *data)return IRQ_NONE ; } -for_each_set_bit (n, &pending,STMFX_REG_IRQ_SRC_MAX ) +for_each_set_bit (n, (unsigned long *)&pending,STMFX_REG_IRQ_SRC_MAX )handle_nested_irq (irq_find_mapping (stmfx->irq_domain, n));return IRQ_HANDLED ;
Signature · 1 line
2.20.1
T24gNi82LzE5IDI6NDEgUE0sIERhbiBDYXJwZW50ZXIgd3JvdGU6DQo+IFRoZSBwcm9ibGVtIGlz IHRoYXQgb24gNjRiaXQgc3lzdGVtcyB0aGVuIHdlIGRvbid0IGNsZWFyIHRoZSBoaWdoZXINCj4g Yml0cyBvZiB0aGUgInBlbmRpbmciIHZhcmlhYmxlLiAgU28gd2hlbiB3ZSBkbzoNCj4gDQo+ICAg ICAgICAgIGFjayA9IHBlbmRpbmcgJiB+QklUKFNUTUZYX1JFR19JUlFfU1JDX0VOX0dQSU8pOw0K PiAgICAgICAgICBpZiAoYWNrKSB7DQo+IA0KPiB0aGUgaWYgKGFjaykgY29uZGl0aW9uIHJlbGll cyBvbiB1bmluaXRpYWxpemVkIGRhdGEuICBUaGUgZml4IGl0IHRoYXQNCj4gSSd2ZSBjaGFuZ2Vk ICJwZW5kaW5nIiBmcm9tIGFuIHVuc2lnbmVkIGxvbmcgdG8gYSB1MzIuICBJIGNoYW5nZWQgIm4i IGFzDQo+IHdlbGwsIGJlY2F1c2UgdGhhdCdzIGEgbnVtYmVyIGluIHRoZSAwLTEwIHJhbmdlIGFu ZCBpdCBmaXRzIGVhc2lseQ0KPiBpbnNpZGUgYW4gaW50LiAgV2UgZG8gbmVlZCB0byBhZGQgYSBj YXN0IHRvICJwZW5kaW5nIiB3aGVuIHdlIHVzZSBpdCBpbg0KPiB0aGUgZm9yX2VhY2hfc2V0X2Jp dCgpIGxvb3AsIGJ1dCB0aGF0IGRvZXNuJ3QgY2F1c2UgYSBwcm9ibGUsIGl0J3MNCj4gZmluZS4N Cj4gDQo+IEZpeGVzOiAwNjI1MmFkZTkxNTYgKCJtZmQ6IEFkZCBTVCBNdWx0aS1GdW5jdGlvbiBl WHBhbmRlciAoU1RNRlgpIGNvcmUgZHJpdmVyIikNCj4gU2lnbmVkLW9mZi1ieTogRGFuIENhcnBl bnRlciA8ZGFuLmNhcnBlbnRlckBvcmFjbGUuY29tPg0KDQpBY2tlZC1ieTogQW1lbGllIERlbGF1 bmF5IDxhbWVsaWUuZGVsYXVuYXlAc3QuY29tPg0KDQo+IC0tLQ0KPiB2Mjogd2hpdGUgc3BhY2Ug Y2hhbmdlcw0KPiANCj4gICBkcml2ZXJzL21mZC9zdG1meC5jIHwgMTAgKysrKy0tLS0tLQ0KPiAg IDEgZmlsZSBjaGFuZ2VkLCA0IGluc2VydGlvbnMoKyksIDYgZGVsZXRpb25zKC0pDQo+IA0KPiBk aWZmIC0tZ2l0IGEvZHJpdmVycy9tZmQvc3RtZnguYyBiL2RyaXZlcnMvbWZkL3N0bWZ4LmMNCj4g aW5kZXggZmU4ZWZiYTJkNDVmLi43YzQxOWMwNzg2ODggMTAwNjQ0DQo+IC0tLSBhL2RyaXZlcnMv bWZkL3N0bWZ4LmMNCj4gKysrIGIvZHJpdmVycy9tZmQvc3RtZnguYw0KPiBAQCAtMjA0LDEyICsy MDQsMTAgQEAgc3RhdGljIHN0cnVjdCBpcnFfY2hpcCBzdG1meF9pcnFfY2hpcCA9IHsNCj4gICBz dGF0aWMgaXJxcmV0dXJuX3Qgc3RtZnhfaXJxX2hhbmRsZXIoaW50IGlycSwgdm9pZCAqZGF0YSkN Cj4gICB7DQo+ICAgCXN0cnVjdCBzdG1meCAqc3RtZnggPSBkYXRhOw0KPiAtCXVuc2lnbmVkIGxv bmcgbiwgcGVuZGluZzsNCj4gLQl1MzIgYWNrOw0KPiAtCWludCByZXQ7DQo+ICsJdTMyIHBlbmRp bmcsIGFjazsNCj4gKwlpbnQgbiwgcmV0Ow0KPiAgIA0KPiAtCXJldCA9IHJlZ21hcF9yZWFkKHN0 bWZ4LT5tYXAsIFNUTUZYX1JFR19JUlFfUEVORElORywNCj4gLQkJCSAgKHUzMiAqKSZwZW5kaW5n KTsNCj4gKwlyZXQgPSByZWdtYXBfcmVhZChzdG1meC0+bWFwLCBTVE1GWF9SRUdfSVJRX1BFTkRJ TkcsICZwZW5kaW5nKTsNCj4gICAJaWYgKHJldCkNCj4gICAJCXJldHVybiBJUlFfTk9ORTsNCj4g ICANCj4gQEAgLTIyNCw3ICsyMjIsNyBAQCBzdGF0aWMgaXJxcmV0dXJuX3Qgc3RtZnhfaXJxX2hh bmRsZXIoaW50IGlycSwgdm9pZCAqZGF0YSkNCj4gICAJCQlyZXR1cm4gSVJRX05PTkU7DQo+ICAg CX0NCj4gICANCj4gLQlmb3JfZWFjaF9zZXRfYml0KG4sICZwZW5kaW5nLCBTVE1GWF9SRUdfSVJR X1NSQ19NQVgpDQo+ICsJZm9yX2VhY2hfc2V0X2JpdChuLCAodW5zaWduZWQgbG9uZyAqKSZwZW5k aW5nLCBTVE1GWF9SRUdfSVJRX1NSQ19NQVgpDQo+ICAgCQloYW5kbGVfbmVzdGVkX2lycShpcnFf ZmluZF9tYXBwaW5nKHN0bWZ4LT5pcnFfZG9tYWluLCBuKSk7DQo+ICAgDQo+ICAgCXJldHVybiBJ UlFfSEFORExFRDsNCj4g
Lee Jones
AppliedQuoted 13 lines · Dan Carpenter
The problem is that on 64bit systems then we don't clear the higher bits of the "pending" variable. So when we do:
ack = pending & ~BIT(STMFX_REG_IRQ_SRC_EN_GPIO); if (ack) {the if (ack) condition relies on uninitialized data. The fix it that I've changed "pending" from an unsigned long to a u32. I changed "n" as well, because that's a number in the 0-10 range and it fits easily inside an int. We do need to add a cast to "pending" when we use it in the
for_each_set_bit()loop, but that doesn't cause a proble, it's fine.
Quoted 7 lines
Fixes 06252ade9156 ("mfd: Add ST Multi-Function eXpander (STMFX) core driver")Signed-off-by Dan Carpenter <[email protected]>---
v2: white space changesdrivers/mfd/stmfx.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
Applied, thanks.
Signature · 4 lines
Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog