Skip to contentSkip to conversation

IXP4xx: Ensure index is positive

PATCH In review Roel Kluin 3 participants 5 messages Nov 3, 2009 – Nov 5, 2009

Subsystems (3)
Arm (32-bit) Arm sub-architectures Arm/Intel IXP4XX Arm architecture
lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 IXP4xx: Ensure index is positive

The indexes are signed, make sure they are not negative when we read the array elements.

Signed-off-by Roel Kluin <[email protected]>
---
 arch/arm/mach-ixp4xx/common.c     |    2 +-
 arch/arm/mach-ixp4xx/ixp4xx_npe.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-ixp4xx/common.c +1 -1
index cfd52fb..2df77bc 100644
@@ -119,7 +119,7 @@ EXPORT_SYMBOL(gpio_to_irq);
 
 int irq_to_gpio(int irq)
 {
-	int gpio = (irq < 32) ? irq2gpio[irq] : -EINVAL;
+	int gpio = (irq < 32 && irq >= 0) ? irq2gpio[irq] : -EINVAL;
 
 	if (gpio == -1)
 		return -EINVAL;
arch/arm/mach-ixp4xx/ixp4xx_npe.c +1 -1
index 47ac69c..30e1456 100644
@@ -667,7 +667,7 @@ err:
 
 struct npe *npe_request(int id)
 {
-	if (id < NPE_COUNT)
+	if (id >= 0 && id < NPE_COUNT)
 		if (npe_tab[id].valid)
 			if (try_module_get(THIS_MODULE))
 				return &npe_tab[id];
#1 lore raw
Quoted 29 lines · Roel Kluin

The indexes are signed, make sure they are not negative when we read the array elements.

Signed-off-by Roel Kluin <[email protected]>
---
 arch/arm/mach-ixp4xx/common.c     |    2 +-
 arch/arm/mach-ixp4xx/ixp4xx_npe.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-ixp4xx/common.c +1 -1
index cfd52fb..2df77bc 100644
@@ -119,7 +119,7 @@ EXPORT_SYMBOL(gpio_to_irq);
 
 int irq_to_gpio(int irq)
 {
-	int gpio = (irq < 32) ? irq2gpio[irq] : -EINVAL;
+	int gpio = (irq < 32 && irq >= 0) ? irq2gpio[irq] : -EINVAL;
 
 	if (gpio == -1)
 		return -EINVAL;
arch/arm/mach-ixp4xx/ixp4xx_npe.c +0 -0
index 47ac69c..30e1456 100644

arch/arm/mach-ixp4xx/ixp4xx_npe.c err: · line 673

Quoted 8 lines
arch/arm/mach-ixp4xx/ixp4xx_npe.c +1 -1
@@ -667,7 +667,7 @@ err:
 
 struct npe *npe_request(int id)
 {
-	if (id < NPE_COUNT)
+	if (id >= 0 && id < NPE_COUNT)
 		if (npe_tab[id].valid)
 			if (try_module_get(THIS_MODULE))
 				return &npe_tab[id];

changing npe_request() to unsigned would probably be better and not add
to bloat. If your calling these functions with negative arguments,
your code is buggy then.

#2 lore raw
Quoted 9 lines · Karl Hiramoto
Quoted 8 lines

+++ b/arch/arm/mach-ixp4xx/common.c

arch/arm/mach-ixp4xx/common.c +1 -1
@@ -119,7 +119,7 @@ EXPORT_SYMBOL(gpio_to_irq);
  int irq_to_gpio(int irq)
 {
-	int gpio = (irq < 32) ? irq2gpio[irq] : -EINVAL;
+	int gpio = (irq < 32 && irq >= 0) ? irq2gpio[irq] : -EINVAL;
  	if (gpio == -1)
 		return -EINVAL;
Quoted 10 lines

arch/arm/mach-ixp4xx/ixp4xx_npe.c err: · line 669

Quoted 6 lines
arch/arm/mach-ixp4xx/ixp4xx_npe.c +2 -1
+++ b/arch/arm/mach-ixp4xx/ixp4xx_npe.c

@@ -667,7 +667,7 @@ err:
  struct npe *npe_request(int id)
 {
-	if (id < NPE_COUNT)
+	if (id >= 0 && id < NPE_COUNT)

changing npe_request() to unsigned would probably be better and not
add to bloat. If your calling these functions with negative
arguments, your code is buggy then.

Right. Both files in fact.

Even the id < NPE_COUNT test is probably not needed but I can imagine someone lowering NPE_COUNT. Negative values are unreasonable (though unsigned type make this unrelevant, of course).

Signature · 1 line
Krzysztof Halasa
author
#3 lore raw

The indexes were signed, so negatives were possible.

Signed-off-by Roel Kluin <[email protected]>

---
Op 03-11-09 22:23, Krzysztof Halasa schreef:

Quoted 9 lines
Quoted 1 line · Karl Hiramoto

changing npe_request() to unsigned would probably be better

Right. Both files in fact.

Even the id < NPE_COUNT test is probably not needed but I can imagine someone lowering NPE_COUNT. Negative values are unreasonable (though unsigned type make this unrelevant, of course).

 arch/arm/mach-ixp4xx/common.c            |    2 +-
 arch/arm/mach-ixp4xx/include/mach/gpio.h |    2 +-
 arch/arm/mach-ixp4xx/include/mach/npe.h  |    2 +-
 arch/arm/mach-ixp4xx/ixp4xx_npe.c        |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

Is this ok?

arch/arm/mach-ixp4xx/common.c +1 -1
index cfd52fb..3bbf40f 100644
@@ -117,7 +117,7 @@ int gpio_to_irq(int gpio)
 }
 EXPORT_SYMBOL(gpio_to_irq);
 
-int irq_to_gpio(int irq)
+int irq_to_gpio(unsigned int irq)
 {
 	int gpio = (irq < 32) ? irq2gpio[irq] : -EINVAL;
arch/arm/mach-ixp4xx/include/mach/gpio.h +1 -1
index cd5aec2..a5f87de 100644
@@ -70,7 +70,7 @@ static inline void gpio_set_value(unsigned gpio, int value)
 #include <asm-generic/gpio.h>			/* cansleep wrappers */
 
 extern int gpio_to_irq(int gpio);
-extern int irq_to_gpio(int gpio);
+extern int irq_to_gpio(unsigned int irq);
 
 #endif
arch/arm/mach-ixp4xx/include/mach/npe.h +1 -1
index 37d0511..e320db2 100644
@@ -33,7 +33,7 @@ int npe_send_message(struct npe *npe, const void *msg, const char *what);
 int npe_recv_message(struct npe *npe, void *msg, const char *what);
 int npe_send_recv_message(struct npe *npe, void *msg, const char *what);
 int npe_load_firmware(struct npe *npe, const char *name, struct device *dev);
-struct npe *npe_request(int id);
+struct npe *npe_request(unsigned id);
 void npe_release(struct npe *npe);
 
 #endif /* __IXP4XX_NPE_H */
arch/arm/mach-ixp4xx/ixp4xx_npe.c +1 -1
index 47ac69c..e8bb257 100644
@@ -665,7 +665,7 @@ err:
 }
 
 
-struct npe *npe_request(int id)
+struct npe *npe_request(unsigned id)
 {
 	if (id < NPE_COUNT)
 		if (npe_tab[id].valid)
#4 lore raw
Quoted 41 lines · Roel Kluin

+++ b/arch/arm/mach-ixp4xx/common.c

arch/arm/mach-ixp4xx/common.c +1 -1
@@ -117,7 +117,7 @@ int gpio_to_irq(int gpio)
 }
 EXPORT_SYMBOL(gpio_to_irq);
 
-int irq_to_gpio(int irq)
+int irq_to_gpio(unsigned int irq)
 {
 	int gpio = (irq < 32) ? irq2gpio[irq] : -EINVAL;
arch/arm/mach-ixp4xx/include/mach/gpio.h +1 -1
index cd5aec2..a5f87de 100644
@@ -70,7 +70,7 @@ static inline void gpio_set_value(unsigned gpio, int value)
 #include <asm-generic/gpio.h>			/* cansleep wrappers */
 
 extern int gpio_to_irq(int gpio);
-extern int irq_to_gpio(int gpio);
+extern int irq_to_gpio(unsigned int irq);
 
 #endif
arch/arm/mach-ixp4xx/include/mach/npe.h +1 -1
index 37d0511..e320db2 100644
@@ -33,7 +33,7 @@ int npe_send_message(struct npe *npe, const void *msg, const char *what);
 int npe_recv_message(struct npe *npe, void *msg, const char *what);
 int npe_send_recv_message(struct npe *npe, void *msg, const char *what);
 int npe_load_firmware(struct npe *npe, const char *name, struct device *dev);
-struct npe *npe_request(int id);
+struct npe *npe_request(unsigned id);
 void npe_release(struct npe *npe);
 
 #endif /* __IXP4XX_NPE_H */
arch/arm/mach-ixp4xx/ixp4xx_npe.c +0 -0
index 47ac69c..e8bb257 100644

arch/arm/mach-ixp4xx/ixp4xx_npe.c err: · line 668

Quoted 5 lines
arch/arm/mach-ixp4xx/ixp4xx_npe.c +1 -1
@@ -665,7 +665,7 @@ err:
 }
 
 
-struct npe *npe_request(int id)
+struct npe *npe_request(unsigned id)

It looks good, thanks a lot.

Signature · 1 line
Krzysztof Halasa