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: > > > ESSIDs can technically include NULL characters. Drivers should not be > > adjusting the length of the ESSID before reporting it in their > > SIOCGIWESSID handlers. Breaks stuff like wpa_supplicant. Note that ipw > > drivers, which seem to currently be the "most correct", don't have this > > problem. > > I'm still trying to go through the huge amount of email I've received > during two-week time travelling and noticed this one just now. I did not > see any discussion about this on netdev list, but I believe the change > here is changes the WE user space interface and this has not been done > previously in order to not break user space programs. > > I agree that the choice of SSID len+1 is unfortunate for a data > structure that is not really a string but array of octets. However, that > was the way SIOCGIWESSID/SIOCSIWESSID was designed if I have understood > correctly (Jean cc'ed just in case). > > Is this change, if applied (or was it already applied?), an indication > that we are now changing the WE interface in a way that is not > backwards compatible and we are willing to break an existing kernel-user > space interface?
The kernel patch against drivers is in netdev-2.6. A few observations: 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. 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. 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. > 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. In any case, I don't have a problem with reverting the patch to the kernel drivers, but then we either need to: a) fix up other drivers like ipw and madwifi-ng (and possibly others) to conform the current WEXT (ie, allowing len+1), and add this to documentation somewhere as a limitation of the API, and fix wpa_supplicant or; b) Let the change go through and document it as a WEXT api change, and fix up user-space programs. If there are good reasons for keeping len+1 (and it sounds like there are), then I'll post a patch reverting my previous patch and we can then work on (a). Dan - 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
