On Sat, Jan 28, 2006 at 11:07:10AM -0500, Dan Williams wrote: > On Fri, 2006-01-27 at 17:01 -0800, Jouni Malinen wrote: > > On Thu, Jan 12, 2006 at 03:00:58PM -0500, Dan Williams wrote:
> 1) The patch preserves the null-termination of the ESSID, it just > doesn't return the expanded length to accommodate that null. So yes, > this may cause programs that simple copy the exact # of bytes, and then > do string operations on it, to crash. Programs that simply do string > operations on the returned essid regardless of length should be > unaffected. Boundary conditions are interesting here, since as the > ESSID approaches IW_MAX_ESSID (or whatever that is), which is 32, it was > unclear what the effect of padding the returned ESSID buffer was anyway. > If the ESSID from the AP actually was 32 bytes long, the padding null > would go into byte 33 and likely be ignored by the calling program if > it's badly coded. Agreed, I've mentioned that to Jean, but I don't remember anymore what was the expected way of handling 32-octet SSIDs. > 2) ipw and madwifi-ng at least don't do what airo/etc do. So wouldn't > those programs that have a problem with (1) also _already_ have a > problem with ipw and madwifi? Those drivers don't pad with NULL and > return len+1. They use the exact essid sent through SIOCSIWESSID. > madwifi-ng actually checks for trailing /0 and removes that before > setting the essid in hardware. Yes.. The "correct" behavior should be spelled out explicitly somewhere since there is still so much confusion about what exactly is the expected behavior. > 3) I posted a patch to correct the observed problem in wpa_supplicant > HEAD, which didn't work with atmel cards due to this issue. I was told > in no uncertain terms by Henrik Brix Andersen <[EMAIL PROTECTED]> to > "Please fix the offending drivers instead of breaking wpa_supplicant." > I actually have no idea if he has anything to do with wpa_supplicant, > but I heard no further discussion on the topic and therefore decided to > patch the drivers to do the right thing. This happened while I was travelling and did not have good access to my email for couple of weeks.. It's still on my to-do list, but going through all the email from past few weeks is going to take some time. I think that wpa_supplicant will have to try to cope with whatever the kernel drivers are doing. Unfortunately this means handling multiple different choices. Getting the SSID from the driver is somewhat easier, since wpa_supplicant can just determine what is the most likely interpretation of the returned. Setting SSID is more difficult, since some drivers may expect len+1 and some require len as SSID length.. Looking at your patch, it does indeed seem to be only for SIOCGIWESSID, so it is indeed something that will need to be merged into wpa_supplicant and I'll do that. > > Actually, wpa_supplicant started using real SSID length, but was changed > > in September 2004 to use len+1 after Jean explained what the > > SIOCSIWESSID was designed to do.. > > Are you sure this is really the case? The entire reason the patch was > posted to wpa_supplicant lists was because I had problems with drivers > that do len+1, which wpa_supplicant didn't accept. Yes, this change in 2004 was for SIOCSIWESSID. I had not looked at your patch when I wrote that, but you seem to be changing SIOCGIWESSID handling and that looks like a reasonable change. > In any case, I don't have a problem with reverting the patch to the > kernel drivers, but then we either need to: Before doing any more changes, I would really like to get consensus on what exactly WEXT requires so that we do not need to continue changing this in the future. Just to be sure that we understand all cases, I would like to see full list of cases and values for data and length field for SIOCGIWESSID and SIOCSIWESSID. My current assumption is that both ioctls were expected to to include extra nul termination in SSID (even though I don't see much point with this) and the data field for SSID is expected to have that nul termination. In other words, if a user space program uses SSID as a octet string (like wpa_supplicant is doing), setting SSID would need to use len+1 and extra '\0' added to the end of SSID data (e.g., by clearing the structure before copying SSID). In case of getting the SSID, kernel driver would do similar changes and len-1 would be used in user space. I would have preferred to see these ioctls use correct length and no expectation on the SSID data being a printable string. However, if the user space programs were to do this for SIOCSIWESSID, some drivers would configure incorrect SSID (the last octet missing). -- Jouni Malinen PGP id EFC895FA - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
