Re: [PATCH] eepro100.c

From: Dragan Stancevic (visitor@valinux.com)
Date: Fri Mar 31 2000 - 03:37:49 EST


On Fri, Mar 31, 2000, Jeff Garzik <jgarzik@mandrakesoft.com> wrote:
; Dragan Stancevic wrote:
; >
; > This patch is for 2.2 kernels against Andrey's "1.20.2.4" driver located at
; > ftp://ftp.sw.com.sg/pub/Linux/people/saw/kernel/v2.2/, this patch provides
; > a bit more detailed information about installed devices.
;
; hmmm.
;
; * kmalloc'ing the product name is completely unnecessary in this patch,
; it is never used outside of the routine in which the kmalloc occurs

Uhm, request_region doesn't make a copy of the name that you pass into it,
so the product_name is accessed anytime when you:

[root@dhcp-net200-69 /root]# cat /proc/ioports
[snip]
d800-d87f : sym53c8xx
df00-df3f : es1371
ffa0-ffa7 : ide0
ffa8-ffaf : ide1
d000b000-d000b01f : Intel EEPro 100 (82559) Rev:8

Otherwise the string is gone after the function exits....

; * you should be passing the ethernet device name to request_region. you
; can easily overflow a buffer (at least in 2.3.x, maybe in 2.2.x too) if
; you pass a too-long name to request_region. further, some userland
; software depends on the ethernet interface name being the value passed
; to request_region.

"Intel Speedo3 Ethernet" Was the string that I am replacing, comparing to this
"Intel EEPro 100 (82559) Rev:8" is 7 bytes longer or 5 comapring to
"Crystal audio controller"(which is another long one), and
"Intel Speedo3 Ethernet" isn't dev->name either so the apps that you are
mentioning wouldn't get dev->name either. On that note I though that
user-level apps should use NETDEVICE(7) to get information about the network
device...

What would be a reasonabe size for request_region so you don't overflow it?
What is the buffer size for request_region?

As far as I am concerned I agree with having in ioports any of the following:
1. ethX
2.Intel Speedo3 Ethernet
3.Intel EEPro 100 (82559) Rev:8
4.Intel EEPro 100 (82559)
        
So I am fine with whatever people prefer. Alan, Andrey should we use ethX in
all of the drivers?

; * can you get any of this info out of subsystem-device/vendor-id?

I don't have the specs here at home so, I'll get back to you on this one.
I think if I remember correctly it wasn't going to work but I might be
wrong, as I said I need to double check this when I get to work tomorrow.
--Sorry

; * instead of passing _yet another_ pci device structure member as an
; argument to speedo_found1, why not just convert the driver to use the
; new pci_dev structure. using pcibios_xxx is deprecated under 2.2.x, and
; requires use of the slow pci_find_slot function to compensate. Making
; this conversion also brings the 2.2.x and 2.3.x eepro100 drivers closer
; together.

I was just following the scheme that the driver was using.
I'll look in to converting the driver to the new pci_dev structure.

Thanks Jeff for all your feedback, I apreciate it alot.

-- 

-Dragan

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Fri Mar 31 2000 - 21:00:28 EST