[PATCH] [RFC] Potential bug in defining 'irq field' of 'ifmap structure'

[PATCH] [RFC] Potential bug in defining 'irq field' of 'ifmap structure'

[Sonasath, Moiz]

From: Moiz Sonasath <m-sonasath@ti.com>

There seems to be a bug in the ioctl implementation in /kernel/net/core/dev.c 
 
dev_ifsioc_locked()
	case SIOCGIFMAP:
			ifr->ifr_map.irq = dev->irq;   // ?? type mismatch

Here
ifr->ifr_map.irq) is of type unsigned char
dev-irq is of type unsigned int

So ifconfig reports a wrong irq number when the dev->irq number is > 255.

I am confused to see the same typedefs in file: net/if.h 
Not sure how to make changes for the user side net/if.h file?

Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
	
---
 include/linux/if.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/if.h b/include/linux/if.h
index 2d89c96..1ac6559 100644
--- a/include/linux/if.h
+++ b/include/linux/if.h
@@ -126,7 +126,7 @@ struct ifmap
 	unsigned long mem_start;
 	unsigned long mem_end;
 	unsigned short base_addr;
-	unsigned char irq;
+	unsigned int irq;
 	unsigned char dma;
 	unsigned char port;
 	/* 3 bytes spare */
-- 
1.5.6.3

Re: [PATCH] [RFC] Potential bug in defining 'irq field' of 'ifmap structure'

[David Miller]

From: "Sonasath, Moiz" <m-sonasath@ti.com>
Date: Mon, 6 Jul 2009 17:23:36 -0500

> There seems to be a bug in the ioctl implementation in /kernel/net/core/dev.c 
>  
> dev_ifsioc_locked()
> 	case SIOCGIFMAP:
> 			ifr->ifr_map.irq = dev->irq;   // ?? type mismatch
> 
> Here
> ifr->ifr_map.irq) is of type unsigned char
> dev-irq is of type unsigned int
> 
> So ifconfig reports a wrong irq number when the dev->irq number is > 255.

This is a known and unavoidable limitation of this interface.
It's only real use is to control ISA style IRQs which are
< 255.

> I am confused to see the same typedefs in file: net/if.h 
> Not sure how to make changes for the user side net/if.h file?
> 
> Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>

You can't make these kinds of changes, every userland binary out there
using this structure would break.

RE: [PATCH] [RFC] Potential bug in defining 'irq field' of 'ifmap structure'

[Pandita, Vikram]

David

>-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net]
>> dev_ifsioc_locked()
>> 	case SIOCGIFMAP:
>> 			ifr->ifr_map.irq = dev->irq;   // ?? type mismatch
>>
>> Here
>> ifr->ifr_map.irq) is of type unsigned char
>> dev-irq is of type unsigned int
>>
>> So ifconfig reports a wrong irq number when the dev->irq number is > 255.
>
>This is a known and unavoidable limitation of this interface.
>It's only real use is to control ISA style IRQs which are < 255.

On Zoom2 TI OMAP3 based board, the IRQ we are requesting is value=381 and hence the problem reported.
We do understand that this would break all user level code.

>
>> I am confused to see the same typedefs in file: net/if.h
>> Not sure how to make changes for the user side net/if.h file?

Any idea why net/if.h user level file does not have same definitions as linux/if.h?
In other words, what is the origin of net/if.h file?

>>
>> Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
>> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
>
>You can't make these kinds of changes, every userland binary out there
>using this structure would break.

Re: [PATCH] [RFC] Potential bug in defining 'irq field' of 'ifmap structure'

[David Miller]

From: "Pandita, Vikram" <vikram.pandita@ti.com>
Date: Wed, 8 Jul 2009 02:27:51 +0530

> On Zoom2 TI OMAP3 based board, the IRQ we are requesting is
> value=381 and hence the problem reported.

The IRQ reported by this user call is only reliable for
ISA devices.

Or, ARM platforms could opt to use a virtual IRQ scheme like
PowerPC and Sparc use, which keeps all device IRQs under
256.