* [PATCH] mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
@ 2019-05-15 9:31 ` Dan Carpenter
0 siblings, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2019-05-15 9:31 UTC (permalink / raw)
To: Lee Jones, Amelie Delaunay
Cc: Maxime Coquelin, Alexandre Torgue, linux-stm32, linux-kernel,
kernel-janitors
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.
Fixes: 06252ade9156 ("mfd: Add ST Multi-Function eXpander (STMFX) core driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/mfd/stmfx.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mfd/stmfx.c b/drivers/mfd/stmfx.c
index fe8efba2d45f..fee75b5d098e 100644
--- a/drivers/mfd/stmfx.c
+++ b/drivers/mfd/stmfx.c
@@ -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;
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
@ 2019-05-15 9:31 ` Dan Carpenter
0 siblings, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2019-05-15 9:31 UTC (permalink / raw)
To: Lee Jones, Amelie Delaunay
Cc: Maxime Coquelin, Alexandre Torgue, linux-stm32, linux-kernel,
kernel-janitors
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.
Fixes: 06252ade9156 ("mfd: Add ST Multi-Function eXpander (STMFX) core driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/mfd/stmfx.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mfd/stmfx.c b/drivers/mfd/stmfx.c
index fe8efba2d45f..fee75b5d098e 100644
--- a/drivers/mfd/stmfx.c
+++ b/drivers/mfd/stmfx.c
@@ -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;
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
2019-05-15 9:31 ` Dan Carpenter
@ 2019-06-03 8:12 ` Lee Jones
-1 siblings, 0 replies; 12+ messages in thread
From: Lee Jones @ 2019-06-03 8:12 UTC (permalink / raw)
To: Dan Carpenter
Cc: Amelie Delaunay, Maxime Coquelin, Alexandre Torgue, linux-stm32,
linux-kernel, kernel-janitors
On Wed, 15 May 2019, Dan Carpenter wrote:
> 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.
>
> Fixes: 06252ade9156 ("mfd: Add ST Multi-Function eXpander (STMFX) core driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/mfd/stmfx.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
Could do with an author's Ack here.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
@ 2019-06-03 8:12 ` Lee Jones
0 siblings, 0 replies; 12+ messages in thread
From: Lee Jones @ 2019-06-03 8:12 UTC (permalink / raw)
To: Dan Carpenter
Cc: Amelie Delaunay, Maxime Coquelin, Alexandre Torgue, linux-stm32,
linux-kernel, kernel-janitors
On Wed, 15 May 2019, Dan Carpenter wrote:
> 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.
>
> Fixes: 06252ade9156 ("mfd: Add ST Multi-Function eXpander (STMFX) core driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/mfd/stmfx.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
Could do with an author's Ack here.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
2019-05-15 9:31 ` Dan Carpenter
@ 2019-06-03 9:20 ` Amelie DELAUNAY
-1 siblings, 0 replies; 12+ messages in thread
From: Amelie DELAUNAY @ 2019-06-03 9:20 UTC (permalink / raw)
To: Dan Carpenter, Lee Jones
Cc: Maxime Coquelin, Alexandre TORGUE, linux-stm32, linux-kernel,
kernel-janitors
Hi Dan,
Thanks for your patch. One minor comment:
On 5/15/19 11:31 AM, Dan Carpenter wrote:
> 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.
>
> Fixes: 06252ade9156 ("mfd: Add ST Multi-Function eXpander (STMFX) core driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/mfd/stmfx.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mfd/stmfx.c b/drivers/mfd/stmfx.c
> index fe8efba2d45f..fee75b5d098e 100644
> --- a/drivers/mfd/stmfx.c
> +++ b/drivers/mfd/stmfx.c
> @@ -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;
Could you group:
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 +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;
>
I've tested it on stm32mp157c-ev1, so you can add my
Tested-by: Amelie Delaunay <amelie.delaunay@st.com>
Regards,
Amelie
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
@ 2019-06-03 9:20 ` Amelie DELAUNAY
0 siblings, 0 replies; 12+ messages in thread
From: Amelie DELAUNAY @ 2019-06-03 9:20 UTC (permalink / raw)
To: Dan Carpenter, Lee Jones
Cc: Maxime Coquelin, Alexandre TORGUE, linux-stm32, linux-kernel,
kernel-janitors
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
2019-06-03 9:20 ` Amelie DELAUNAY
@ 2019-06-06 12:41 ` Dan Carpenter
-1 siblings, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2019-06-06 12:41 UTC (permalink / raw)
To: Lee Jones, Amelie DELAUNAY
Cc: Maxime Coquelin, Alexandre Torgue, linux-stm32, linux-kernel,
kernel-janitors
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.
Fixes: 06252ade9156 ("mfd: Add ST Multi-Function eXpander (STMFX) core driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: white space changes
drivers/mfd/stmfx.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/mfd/stmfx.c b/drivers/mfd/stmfx.c
index fe8efba2d45f..7c419c078688 100644
--- a/drivers/mfd/stmfx.c
+++ b/drivers/mfd/stmfx.c
@@ -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;
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2] mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
@ 2019-06-06 12:41 ` Dan Carpenter
0 siblings, 0 replies; 12+ messages in thread
From: Dan Carpenter @ 2019-06-06 12:41 UTC (permalink / raw)
To: Lee Jones, Amelie DELAUNAY
Cc: Maxime Coquelin, Alexandre Torgue, linux-stm32, linux-kernel,
kernel-janitors
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.
Fixes: 06252ade9156 ("mfd: Add ST Multi-Function eXpander (STMFX) core driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: white space changes
drivers/mfd/stmfx.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/mfd/stmfx.c b/drivers/mfd/stmfx.c
index fe8efba2d45f..7c419c078688 100644
--- a/drivers/mfd/stmfx.c
+++ b/drivers/mfd/stmfx.c
@@ -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;
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
2019-06-06 12:41 ` Dan Carpenter
@ 2019-06-06 12:56 ` Amelie DELAUNAY
-1 siblings, 0 replies; 12+ messages in thread
From: Amelie DELAUNAY @ 2019-06-06 12:56 UTC (permalink / raw)
To: Dan Carpenter, Lee Jones
Cc: Maxime Coquelin, Alexandre TORGUE, linux-stm32, linux-kernel,
kernel-janitors
On 6/6/19 2:41 PM, Dan Carpenter wrote:
> 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.
>
> Fixes: 06252ade9156 ("mfd: Add ST Multi-Function eXpander (STMFX) core driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Amelie Delaunay <amelie.delaunay@st.com>
> ---
> v2: white space changes
>
> drivers/mfd/stmfx.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mfd/stmfx.c b/drivers/mfd/stmfx.c
> index fe8efba2d45f..7c419c078688 100644
> --- a/drivers/mfd/stmfx.c
> +++ b/drivers/mfd/stmfx.c
> @@ -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;
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
@ 2019-06-06 12:56 ` Amelie DELAUNAY
0 siblings, 0 replies; 12+ messages in thread
From: Amelie DELAUNAY @ 2019-06-06 12:56 UTC (permalink / raw)
To: Dan Carpenter, Lee Jones
Cc: Maxime Coquelin, Alexandre TORGUE, linux-stm32, linux-kernel,
kernel-janitors
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
2019-06-06 12:41 ` Dan Carpenter
@ 2019-06-12 9:37 ` Lee Jones
-1 siblings, 0 replies; 12+ messages in thread
From: Lee Jones @ 2019-06-12 9:37 UTC (permalink / raw)
To: Dan Carpenter
Cc: Amelie DELAUNAY, Maxime Coquelin, Alexandre Torgue, linux-stm32,
linux-kernel, kernel-janitors
On Thu, 06 Jun 2019, Dan Carpenter wrote:
> 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.
>
> Fixes: 06252ade9156 ("mfd: Add ST Multi-Function eXpander (STMFX) core driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: white space changes
>
> drivers/mfd/stmfx.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
Applied, thanks.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] mfd: stmfx: Uninitialized variable in stmfx_irq_handler()
@ 2019-06-12 9:37 ` Lee Jones
0 siblings, 0 replies; 12+ messages in thread
From: Lee Jones @ 2019-06-12 9:37 UTC (permalink / raw)
To: Dan Carpenter
Cc: Amelie DELAUNAY, Maxime Coquelin, Alexandre Torgue, linux-stm32,
linux-kernel, kernel-janitors
On Thu, 06 Jun 2019, Dan Carpenter wrote:
> 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.
>
> Fixes: 06252ade9156 ("mfd: Add ST Multi-Function eXpander (STMFX) core driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: white space changes
>
> drivers/mfd/stmfx.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
Applied, thanks.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-06-12 9:37 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-15 9:31 [PATCH] mfd: stmfx: Uninitialized variable in stmfx_irq_handler() Dan Carpenter
2019-05-15 9:31 ` Dan Carpenter
2019-06-03 8:12 ` Lee Jones
2019-06-03 8:12 ` Lee Jones
2019-06-03 9:20 ` Amelie DELAUNAY
2019-06-03 9:20 ` Amelie DELAUNAY
2019-06-06 12:41 ` [PATCH v2] " Dan Carpenter
2019-06-06 12:41 ` Dan Carpenter
2019-06-06 12:56 ` Amelie DELAUNAY
2019-06-06 12:56 ` Amelie DELAUNAY
2019-06-12 9:37 ` Lee Jones
2019-06-12 9:37 ` Lee Jones
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.