Ahrg. So I was complaining about NM not displaying any networks when
that umlaut-network I talked about earlier was present. But all that was
just wrong, the real problem seems to be that NM doesn't work on 64-bit
platforms. Is there any fix for it? It really should be going into a
0.6.5.1 version or something so distributions will ship it.
Same seems to go for wpa_supplicant, Debian is shipping 0.6.0 which only
triggers this in the kernel log:
[ 787.877417] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58418) on socket:[14569]
[ 787.877763] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58418) on socket:[14569]
[ 787.877817] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58418) on socket:[14569]
[ 795.264145] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58458) on socket:[14569]
[ 795.264243] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58458) on socket:[14569]
[ 795.264291] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58458) on socket:[14569]
And even the wpa_supplicant git version still seems to parse everything
itself without any 64-bit workarounds as far as I can tell. I keep
building wpa_supplicant for 64-bit just to test... Any chance to finally
get fixes into the stable versions distros are shipping?
Thanks,
johannes
On Thu, 2007-09-20 at 14:48 +0200, Johannes Berg wrote:
> Ahrg. So I was complaining about NM not displaying any networks when
> that umlaut-network I talked about earlier was present. But all that was
> just wrong, the real problem seems to be that NM doesn't work on 64-bit
> platforms. Is there any fix for it? It really should be going into a
> 0.6.5.1 version or something so distributions will ship it.
You mean it doesn't work correctly when handling ioctls on 64-bit
platforms? Or is just plain broken in non-ioctl ways?
Dan
> Same seems to go for wpa_supplicant, Debian is shipping 0.6.0 which only
> triggers this in the kernel log:
>
> [ 787.877417] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58418) on socket:[14569]
> [ 787.877763] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58418) on socket:[14569]
> [ 787.877817] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58418) on socket:[14569]
> [ 795.264145] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58458) on socket:[14569]
> [ 795.264243] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58458) on socket:[14569]
> [ 795.264291] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58458) on socket:[14569]
>
> And even the wpa_supplicant git version still seems to parse everything
> itself without any 64-bit workarounds as far as I can tell. I keep
> building wpa_supplicant for 64-bit just to test... Any chance to finally
> get fixes into the stable versions distros are shipping?
>
> Thanks,
> johannes
On Thu, 2007-09-20 at 13:56 -0700, Jean Tourrilhes wrote:
> > Does NM always use wpa_supplicant even in 0.6.5? I thought only later
> > versions deferred everything to it.
>
> That was my assumption as well, and the reason I spent time
> doing my patch. However, after your bug report, and before replying to
> you, I went back looking at the code, to verify.
> Well, as it happens, 0.6.5 uses wpa_supplicant for the scan if
> it's available (more below).
Hmm. ok.
> Ask Jouni ?
> For someone familiar with it, it's actually trivial, you just
> have to follow the patch for Wireless Tools. But for somebody
> unfamiliar, there is a huge learning curve.
> That's classical : all the eyes are on the kernel, and nobody
> care about userspace. We really should have more people familiar with
> the guts of wpa_supplicant.
Oh, I'm not too bad in wpa_supplicant. But I don't know how the
workaround you came up with works. I guess I'll have to dig into that.
You mentioned wpa_supplicant patches, do you still have those?
> > > Ok, I see what's happening. That would just prevent you to set
> > > authentication information, but that is not the root caause of your
> > > problems. I'll puch a fix ASAP to John.
> >
> > Not sure, this seems to make wpa_supplicant unhappy enough to not even
> > start doing anything. But then I configured it for WPA.
>
> Patch sent. Can't do harm anyway.
I'll give that a try next week.
> I'm currently stuck because I don't have a box handy to try
> NetworkManager on, most of my boxes are without all the Gnome
> overhead. I'll try to fix that, but it may take a few days.
> Meanwhile, I made a few patch just for you for NetworkManager
> 0.6.5. I could not even try to compile it (./configure dependancy), so
> beware.
>
> The first patch fix the event parsing code to use the libiw
> helpers. As you can see, this dramatically reduce the code
> complexity. However, as this code does not use the payload of events,
> it should not have been affected by the 32-64 bit issue.
>
> The second patch force NetworkManager to always use its own
> scanning code. This is a quick hack, I don't know what will be the
> interaction with wpa_supplicant and I don't know when this part of
> NetworkManager was last tested.
> In theory, with that change, you should start to see a list of
> networks in NetworkManager.
I may take a look, but my quad box isn't online and I usually don't have
the things fixed. In any case, my networks are all RSN-protected so if
wpa_supplicant can't talk to them that's little use to me. Hence, I'd
rather see it fixed in wpa_supplicant.
johannes
On Wed, 2007-09-26 at 16:51 -0700, Jean Tourrilhes wrote:
> Sorry, I was distracted by some Kismet bug.
> The patch I sent to Jouni was not a wpa_supplicant, but the
> Wireless Tools patch. The complete fix is across multiple Wireless
> Tools versions, and mixed with other stuff, this patch highlight
> clearly the changes.
Ok. Thanks Jean; I've been sick all week, will look at it next week and
let you know.
johannes
Hi,
> You know that I don't have any 64 bit box, so I can't really
> test it. I did extensive work with people of Gentoo and Debian to make
> sure that my fix in Wireless Extension works both with 32 bits
> userspace on 64 bits kernel and 64 bits userspace on 64 bits
> kernels. The versions that are fully fixed are 29-pre20 and later.
Ok. Sounds good.
> Then, I modified NetworkManaged to use libiw for scan
> parsing. The idea was to simplify NetworkManager and fix the 32-64 bit
> bug, plus a few other potential gotchas. The first version of
> NetworkManager to include that fix is 0.6.5. But, I've just realised
> that I did not convert event parsing, which could be an issue, I'll
> try to work on that.
> Note that the other big issue is that, if wpa_supplicant is
> present, NetworkManager will request the scan from it, and won't use
> its internal code, so all those fixes are useless. Maybe there should
> be a control to force NetworkManager to use its own scan code when
> needed.
Does NM always use wpa_supplicant even in 0.6.5? I thought only later
versions deferred everything to it.
> As far as I know, Debian testing (Lenny) has those
> packages. Of course, I would not mind if you could test all this,
> verify that the packages are the right version and that iwlist works
> properly. If iwlist does not work, the rest will never works.
iwlist works fine on my quad G5 box, but NM doesn't. I suppose then it
does use wpa_supplicant.
> With respect to wpa_supplicant. Well, I sent multiple e-mail
> to Jouni to inform him about this. My personaly inclination would be
> to rip the custom parsing code of wpa_supplicant and use libiw
> instead, but Jouni will never accept that. Maybe you should use
> xsupplicant instead.
Can't really use xsupplicant, since NM relies on it and we want the
kernel to rely more on wpa_supplicant (or another userspace MLME
implementation). Is it really hard to fix the parsing code in
wpa_supplicant? I'm not familiar with the workaround at all.
> Ok, I see what's happening. That would just prevent you to set
> authentication information, but that is not the root caause of your
> problems. I'll puch a fix ASAP to John.
Not sure, this seems to make wpa_supplicant unhappy enough to not even
start doing anything. But then I configured it for WPA.
johannes
On Thu, Sep 20, 2007 at 02:48:46PM +0200, Johannes Berg wrote:
> Ahrg. So I was complaining about NM not displaying any networks when
> that umlaut-network I talked about earlier was present. But all that was
> just wrong, the real problem seems to be that NM doesn't work on 64-bit
> platforms. Is there any fix for it? It really should be going into a
> 0.6.5.1 version or something so distributions will ship it.
>
> And even the wpa_supplicant git version still seems to parse everything
> itself without any 64-bit workarounds as far as I can tell. I keep
> building wpa_supplicant for 64-bit just to test... Any chance to finally
> get fixes into the stable versions distros are shipping?
>
> Thanks,
> johannes
You know that I don't have any 64 bit box, so I can't really
test it. I did extensive work with people of Gentoo and Debian to make
sure that my fix in Wireless Extension works both with 32 bits
userspace on 64 bits kernel and 64 bits userspace on 64 bits
kernels. The versions that are fully fixed are 29-pre20 and later.
Then, I modified NetworkManaged to use libiw for scan
parsing. The idea was to simplify NetworkManager and fix the 32-64 bit
bug, plus a few other potential gotchas. The first version of
NetworkManager to include that fix is 0.6.5. But, I've just realised
that I did not convert event parsing, which could be an issue, I'll
try to work on that.
Note that the other big issue is that, if wpa_supplicant is
present, NetworkManager will request the scan from it, and won't use
its internal code, so all those fixes are useless. Maybe there should
be a control to force NetworkManager to use its own scan code when
needed.
As far as I know, Debian testing (Lenny) has those
packages. Of course, I would not mind if you could test all this,
verify that the packages are the right version and that iwlist works
properly. If iwlist does not work, the rest will never works.
With respect to wpa_supplicant. Well, I sent multiple e-mail
to Jouni to inform him about this. My personaly inclination would be
to rip the custom parsing code of wpa_supplicant and use libiw
instead, but Jouni will never accept that. Maybe you should use
xsupplicant instead.
> Same seems to go for wpa_supplicant, Debian is shipping 0.6.0 which only
> triggers this in the kernel log:
>
> [ 787.877417] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58418) on socket:[14569]
> [ 787.877763] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58418) on socket:[14569]
> [ 787.877817] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58418) on socket:[14569]
> [ 795.264145] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58458) on socket:[14569]
> [ 795.264243] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58458) on socket:[14569]
> [ 795.264291] ioctl32(wpa_supplicant:5846): Unknown cmd fd(3) cmd(00008b32){t:8b;sz:0} arg(ffb58458) on socket:[14569]
>
Ok, I see what's happening. That would just prevent you to set
authentication information, but that is not the root caause of your
problems. I'll puch a fix ASAP to John.
Regards,
Jean
On Fri, Sep 21, 2007 at 04:36:54PM +0200, Johannes Berg wrote:
> On Thu, 2007-09-20 at 13:56 -0700, Jean Tourrilhes wrote:
>
> > For someone familiar with it, it's actually trivial, you just
> > have to follow the patch for Wireless Tools. But for somebody
> > unfamiliar, there is a huge learning curve.
> > That's classical : all the eyes are on the kernel, and nobody
> > care about userspace. We really should have more people familiar with
> > the guts of wpa_supplicant.
>
> Oh, I'm not too bad in wpa_supplicant. But I don't know how the
> workaround you came up with works. I guess I'll have to dig into that.
> You mentioned wpa_supplicant patches, do you still have those?
Sorry, I was distracted by some Kismet bug.
The patch I sent to Jouni was not a wpa_supplicant, but the
Wireless Tools patch. The complete fix is across multiple Wireless
Tools versions, and mixed with other stuff, this patch highlight
clearly the changes.
The patch probably does not have enough context to figure out
everything, but at least point in the right direction. It's
documentated, but feel free to ask questions. I also have tests
vectors that you can use on i386.
I personally don't like much the parser used in
wpa_supplicant, and it's somewhat different from my code. You will
find it in those two functions :
wpa_driver_wext_event_wireless()
wpa_driver_wext_get_scan_results()
Have fun...
Jean
-------------------------------------------------------------
diff -u -p wireless_tools.29.pre14/iwlib.c wireless_tools.29.pre20/iwlib.c
--- wireless_tools.29.pre14/iwlib.c 2007-02-16 17:17:15.000000000 -0800
+++ wireless_tools.29.pre20/iwlib.c 2007-04-19 16:33:57.000000000 -0700
@@ -2634,17 +2634,17 @@ static const unsigned int standard_event
/* Size (in bytes) of various events */
static const int event_type_size[] = {
- IW_EV_LCP_LEN, /* IW_HEADER_TYPE_NULL */
+ IW_EV_LCP_PK_LEN, /* IW_HEADER_TYPE_NULL */
0,
- IW_EV_CHAR_LEN, /* IW_HEADER_TYPE_CHAR */
+ IW_EV_CHAR_PK_LEN, /* IW_HEADER_TYPE_CHAR */
0,
- IW_EV_UINT_LEN, /* IW_HEADER_TYPE_UINT */
- IW_EV_FREQ_LEN, /* IW_HEADER_TYPE_FREQ */
- IW_EV_ADDR_LEN, /* IW_HEADER_TYPE_ADDR */
+ IW_EV_UINT_PK_LEN, /* IW_HEADER_TYPE_UINT */
+ IW_EV_FREQ_PK_LEN, /* IW_HEADER_TYPE_FREQ */
+ IW_EV_ADDR_PK_LEN, /* IW_HEADER_TYPE_ADDR */
0,
- IW_EV_POINT_LEN, /* Without variable payload */
- IW_EV_PARAM_LEN, /* IW_HEADER_TYPE_PARAM */
- IW_EV_QUAL_LEN, /* IW_HEADER_TYPE_QUAL */
+ IW_EV_POINT_PK_LEN, /* Without variable payload */
+ IW_EV_PARAM_PK_LEN, /* IW_HEADER_TYPE_PARAM */
+ IW_EV_QUAL_PK_LEN, /* IW_HEADER_TYPE_QUAL */
};
/*------------------------------------------------------------------*/
@@ -2681,29 +2681,26 @@ iw_extract_event_stream(struct stream_de
/* Don't "optimise" the following variable, it will crash */
unsigned cmd_index; /* *MUST* be unsigned */
- /* Unused for now. Will be later on... */
- we_version = we_version;
-
/* Check for end of stream */
- if((stream->current + IW_EV_LCP_LEN) > stream->end)
+ if((stream->current + IW_EV_LCP_PK_LEN) > stream->end)
return(0);
-#if DEBUG
+#ifdef DEBUG
printf("DBG - stream->current = %p, stream->value = %p, stream->end = %p\n",
stream->current, stream->value, stream->end);
#endif
/* Extract the event header (to get the event id).
* Note : the event may be unaligned, therefore copy... */
- memcpy((char *) iwe, stream->current, IW_EV_LCP_LEN);
+ memcpy((char *) iwe, stream->current, IW_EV_LCP_PK_LEN);
-#if DEBUG
+#ifdef DEBUG
printf("DBG - iwe->cmd = 0x%X, iwe->len = %d\n",
iwe->cmd, iwe->len);
#endif
/* Check invalid events */
- if(iwe->len <= IW_EV_LCP_LEN)
+ if(iwe->len <= IW_EV_LCP_PK_LEN)
return(-1);
/* Get the type and length of that event */
@@ -2721,28 +2718,28 @@ iw_extract_event_stream(struct stream_de
}
if(descr != NULL)
event_type = descr->header_type;
- /* Unknown events -> event_type=0 => IW_EV_LCP_LEN */
+ /* Unknown events -> event_type=0 => IW_EV_LCP_PK_LEN */
event_len = event_type_size[event_type];
/* Fixup for earlier version of WE */
if((we_version <= 18) && (event_type == IW_HEADER_TYPE_POINT))
event_len += IW_EV_POINT_OFF;
/* Check if we know about this event */
- if(event_len <= IW_EV_LCP_LEN)
+ if(event_len <= IW_EV_LCP_PK_LEN)
{
/* Skip to next event */
stream->current += iwe->len;
return(2);
}
- event_len -= IW_EV_LCP_LEN;
+ event_len -= IW_EV_LCP_PK_LEN;
/* Set pointer on data */
if(stream->value != NULL)
pointer = stream->value; /* Next value in event */
else
- pointer = stream->current + IW_EV_LCP_LEN; /* First value in event */
+ pointer = stream->current + IW_EV_LCP_PK_LEN; /* First value in event */
-#if DEBUG
+#ifdef DEBUG
printf("DBG - event_type = %d, event_len = %d, pointer = %p\n",
event_type, event_len, pointer);
#endif
@@ -2755,6 +2752,7 @@ iw_extract_event_stream(struct stream_de
return(-2);
}
/* Fixup for WE-19 and later : pointer no longer in the stream */
+ /* Beware of alignement. Dest has local alignement, not packed */
if((we_version > 18) && (event_type == IW_HEADER_TYPE_POINT))
memcpy((char *) iwe + IW_EV_LCP_LEN + IW_EV_POINT_OFF,
pointer, event_len);
@@ -2768,7 +2766,7 @@ iw_extract_event_stream(struct stream_de
if(event_type == IW_HEADER_TYPE_POINT)
{
/* Check the length of the payload */
- unsigned int extra_len = iwe->len - (event_len + IW_EV_LCP_LEN);
+ unsigned int extra_len = iwe->len - (event_len + IW_EV_LCP_PK_LEN);
if(extra_len > 0)
{
/* Set pointer on variable part (warning : non aligned) */
@@ -2783,9 +2781,35 @@ iw_extract_event_stream(struct stream_de
/* Those checks are actually pretty hard to trigger,
* because of the checks done in the kernel... */
+ unsigned int token_len = iwe->u.data.length * descr->token_size;
+
+ /* Ugly fixup for alignement issues.
+ * If the kernel is 64 bits and userspace 32 bits,
+ * we have an extra 4+4 bytes.
+ * Fixing that in the kernel would break 64 bits userspace. */
+ if((token_len != extra_len) && (extra_len >= 4))
+ {
+ __u16 alt_dlen = *((__u16 *) pointer);
+ unsigned int alt_token_len = alt_dlen * descr->token_size;
+ if((alt_token_len + 8) == extra_len)
+ {
+#ifdef DEBUG
+ printf("DBG - alt_token_len = %d\n", alt_token_len);
+#endif
+ /* Ok, let's redo everything */
+ pointer -= event_len;
+ pointer += 4;
+ /* Dest has local alignement, not packed */
+ memcpy((char *) iwe + IW_EV_LCP_LEN + IW_EV_POINT_OFF,
+ pointer, event_len);
+ pointer += event_len + 4;
+ iwe->u.data.pointer = pointer;
+ token_len = alt_token_len;
+ }
+ }
+
/* Discard bogus events which advertise more tokens than
* what they carry... */
- unsigned int token_len = iwe->u.data.length * descr->token_size;
if(token_len > extra_len)
iwe->u.data.pointer = NULL; /* Discard paylod */
/* Check that the advertised token size is not going to
@@ -2796,7 +2820,7 @@ iw_extract_event_stream(struct stream_de
/* Same for underflows... */
if(iwe->u.data.length < descr->min_tokens)
iwe->u.data.pointer = NULL; /* Discard paylod */
-#if DEBUG
+#ifdef DEBUG
printf("DBG - extra_len = %d, token_len = %d, token = %d, max = %d, min = %d\n",
extra_len, token_len, iwe->u.data.length, descr->max_tokens, descr->min_tokens);
#endif
@@ -2811,6 +2835,25 @@ iw_extract_event_stream(struct stream_de
}
else
{
+ /* Ugly fixup for alignement issues.
+ * If the kernel is 64 bits and userspace 32 bits,
+ * we have an extra 4 bytes.
+ * Fixing that in the kernel would break 64 bits userspace. */
+ if((stream->value == NULL)
+ && ((((iwe->len - IW_EV_LCP_PK_LEN) % event_len) == 4)
+ || ((iwe->len == 12) && ((event_type == IW_HEADER_TYPE_UINT) ||
+ (event_type == IW_HEADER_TYPE_QUAL))) ))
+ {
+#ifdef DEBUG
+ printf("DBG - alt iwe->len = %d\n", iwe->len - 4);
+#endif
+ pointer -= event_len;
+ pointer += 4;
+ /* Beware of alignement. Dest has local alignement, not packed */
+ memcpy((char *) iwe + IW_EV_LCP_LEN, pointer, event_len);
+ pointer += event_len;
+ }
+
/* Is there more value in the event ? */
if((pointer + event_len) <= (stream->current + iwe->len))
/* Go to next value */
diff -u -p wireless_tools.29.pre14/iwlib.h wireless_tools.29/iwlib.h
--- wireless_tools.29.pre14/iwlib.h 2007-02-16 17:17:26.000000000 -0800
+++ wireless_tools.29/iwlib.h 2007-06-22 11:01:04.000000000 -0700
@@ -153,6 +124,36 @@ extern "C" {
#define ARPHRD_IEEE80211 801 /* IEEE 802.11 */
#endif /* ARPHRD_IEEE80211 */
+#ifndef IW_EV_LCP_PK_LEN
+/* Size of the Event prefix when packed in stream */
+#define IW_EV_LCP_PK_LEN (4)
+/* Size of the various events when packed in stream */
+#define IW_EV_CHAR_PK_LEN (IW_EV_LCP_PK_LEN + IFNAMSIZ)
+#define IW_EV_UINT_PK_LEN (IW_EV_LCP_PK_LEN + sizeof(__u32))
+#define IW_EV_FREQ_PK_LEN (IW_EV_LCP_PK_LEN + sizeof(struct iw_freq))
+#define IW_EV_PARAM_PK_LEN (IW_EV_LCP_PK_LEN + sizeof(struct iw_param))
+#define IW_EV_ADDR_PK_LEN (IW_EV_LCP_PK_LEN + sizeof(struct sockaddr))
+#define IW_EV_QUAL_PK_LEN (IW_EV_LCP_PK_LEN + sizeof(struct iw_quality))
+#define IW_EV_POINT_PK_LEN (IW_EV_LCP_PK_LEN + 4)
+
+struct iw_pk_event
+{
+ __u16 len; /* Real lenght of this stuff */
+ __u16 cmd; /* Wireless IOCTL */
+ union iwreq_data u; /* IOCTL fixed payload */
+} __attribute__ ((packed));
+struct iw_pk_point
+{
+ void __user *pointer; /* Pointer to the data (in user space) */
+ __u16 length; /* number of fields or size in bytes */
+ __u16 flags; /* Optional params */
+} __attribute__ ((packed));
+
+#define IW_EV_LCP_PK2_LEN (sizeof(struct iw_pk_event) - sizeof(union iwreq_data))
+#define IW_EV_POINT_PK2_LEN (IW_EV_LCP_PK2_LEN + sizeof(struct iw_pk_point) - IW_EV_POINT_OFF)
+
+#endif /* IW_EV_LCP_PK_LEN */
+
/****************************** TYPES ******************************/
/* Shortcuts */
On Thu, 2007-09-20 at 11:50 -0400, Dan Williams wrote:
> You mean it doesn't work correctly when handling ioctls on 64-bit
> platforms? Or is just plain broken in non-ioctl ways?
The former. It starts fine but sees no networks, I guess because the
networks come in a 64-bit format it doesn't interpret. I haven't really
dug into it.
johannes
On Thu, Sep 20, 2007 at 07:01:09PM +0200, Johannes Berg wrote:
>
> > Then, I modified NetworkManaged to use libiw for scan
> > parsing. The idea was to simplify NetworkManager and fix the 32-64 bit
> > bug, plus a few other potential gotchas. The first version of
> > NetworkManager to include that fix is 0.6.5. But, I've just realised
> > that I did not convert event parsing, which could be an issue, I'll
> > try to work on that.
> > Note that the other big issue is that, if wpa_supplicant is
> > present, NetworkManager will request the scan from it, and won't use
> > its internal code, so all those fixes are useless. Maybe there should
> > be a control to force NetworkManager to use its own scan code when
> > needed.
>
> Does NM always use wpa_supplicant even in 0.6.5? I thought only later
> versions deferred everything to it.
That was my assumption as well, and the reason I spent time
doing my patch. However, after your bug report, and before replying to
you, I went back looking at the code, to verify.
Well, as it happens, 0.6.5 uses wpa_supplicant for the scan if
it's available (more below).
> > As far as I know, Debian testing (Lenny) has those
> > packages. Of course, I would not mind if you could test all this,
> > verify that the packages are the right version and that iwlist works
> > properly. If iwlist does not work, the rest will never works.
>
> iwlist works fine on my quad G5 box, but NM doesn't. I suppose then it
> does use wpa_supplicant.
Ok, we are making slow progress, but at least we are making
progress. We are highly dependant on good bug reports.
> > With respect to wpa_supplicant. Well, I sent multiple e-mail
> > to Jouni to inform him about this. My personaly inclination would be
> > to rip the custom parsing code of wpa_supplicant and use libiw
> > instead, but Jouni will never accept that. Maybe you should use
> > xsupplicant instead.
>
> Can't really use xsupplicant, since NM relies on it and we want the
> kernel to rely more on wpa_supplicant (or another userspace MLME
> implementation). Is it really hard to fix the parsing code in
> wpa_supplicant? I'm not familiar with the workaround at all.
Ask Jouni ?
For someone familiar with it, it's actually trivial, you just
have to follow the patch for Wireless Tools. But for somebody
unfamiliar, there is a huge learning curve.
That's classical : all the eyes are on the kernel, and nobody
care about userspace. We really should have more people familiar with
the guts of wpa_supplicant.
> > Ok, I see what's happening. That would just prevent you to set
> > authentication information, but that is not the root caause of your
> > problems. I'll puch a fix ASAP to John.
>
> Not sure, this seems to make wpa_supplicant unhappy enough to not even
> start doing anything. But then I configured it for WPA.
Patch sent. Can't do harm anyway.
> johannes
I'm currently stuck because I don't have a box handy to try
NetworkManager on, most of my boxes are without all the Gnome
overhead. I'll try to fix that, but it may take a few days.
Meanwhile, I made a few patch just for you for NetworkManager
0.6.5. I could not even try to compile it (./configure dependancy), so
beware.
The first patch fix the event parsing code to use the libiw
helpers. As you can see, this dramatically reduce the code
complexity. However, as this code does not use the payload of events,
it should not have been affected by the 32-64 bit issue.
The second patch force NetworkManager to always use its own
scanning code. This is a quick hack, I don't know what will be the
interaction with wpa_supplicant and I don't know when this part of
NetworkManager was last tested.
In theory, with that change, you should start to see a list of
networks in NetworkManager.
Have fun...
Jean
On Tue, 2007-10-09 at 19:20 -0700, Jouni Malinen wrote:
> Unfortunately, I do not have a device that would show the 64-bit issue.
> Could you please remind me which devices are affected and how common
> they are? So far, not very many people have complained about the issues
> with wpa_supplicant on 64-bit systems.
Every time you run a 32-bit wpa_supplicant on a 64-bit system.
> > Could you try wpa_supplicant with kernel patch below on your 32-bit
> > machine? I think this should make the problem show up on 32-bit
> > machines. Not having a 64-bit machine needn't stop you from fixing the
> > alignment problems :) I can then verify that it works and didn't break
> > 64/64 setups if you want.
>
> This sounds like an interesting way of resolving the lack of hardware
> part of the problem.. ;-) If this approach is confirmed to work, I
> would be quite a bit more likely to actually find time to take a closer
> look at the problem.
I think this should reproduce the problem since the actual problem is
that on a 64-bit kernel you have more padding that 32-bit userspace
needs to work around.
johannes
On Tue, Oct 09, 2007 at 07:20:41PM -0700, Jouni Malinen wrote:
> On Tue, Oct 09, 2007 at 11:15:16AM +0200, Johannes Berg wrote:
>
> > If I were to venture a guess I'd say that Jouni would accept a (sane)
> > replacement if you were to license it under dual BSD/GPL as the rest of
> > wpa_supplicant/hostapd.
>
> Maybe, maybe not. License is just one of the reasons (but a strong one;
> there's no way I would use GPL-only library here). The other part is in
> the likelyhood of target systems having the library available. If it is
> not expected to be available in more or less all targets, I do not
> really like to start requiring it. wpa_supplicant/hostapd are used in
> number of embedded designs and the less external libraries are required
> the better.
>
> Unfortunately, I do not have a device that would show the 64-bit issue.
> Could you please remind me which devices are affected and how common
> they are? So far, not very many people have complained about the issues
> with wpa_supplicant on 64-bit systems.
When I sent you the original patch for WT, I proposed you some
test vectors. I never heard back from you.
> > Could you try wpa_supplicant with kernel patch below on your 32-bit
> > machine? I think this should make the problem show up on 32-bit
> > machines. Not having a 64-bit machine needn't stop you from fixing the
> > alignment problems :) I can then verify that it works and didn't break
> > 64/64 setups if you want.
>
> This sounds like an interesting way of resolving the lack of hardware
> part of the problem.. ;-) If this approach is confirmed to work, I
> would be quite a bit more likely to actually find time to take a closer
> look at the problem.
You don't really need all that complex stuff. Here are a few
test vectors for i386 (see below). You stuff that in the buffer you
got from the kernel and attempt to parse it. That's what I used to fix
the bug and it worked for me.
Note : one of the thing to take care is to not screw up 64 bit
userspace on 64 bit kernel. That's why you should pay attention to the
way I did it in my patch.
Note2 : both the Scanning and the Event code need to be fixed.
> Jouni Malinen PGP id EFC895FA
Have fun...
Jean
--------------------------------------------------------------
//static unsigned char hack[] = { 0x19, 0x00, 0x1B, 0x8B, 0x50, 0x8A, 0x35, 0xE0, 0x09, 0x00, 0x01, 0x00, 0x50, 0x8A, 0x35, 0xF0, 0x47, 0x6F, 0x6C, 0x6F, 0x73, 0x4E, 0x65, 0x74, 0x7A };
//static unsigned char hack[] = { 0x18, 0x00, 0x15, 0x8B, 0x50, 0x8A, 0x35, 0xE0, 0x00, 0x01, 0x00, 0xC0, 0x02, 0xE1, 0xB8, 0x0E, 0xC0, 0x00, 0x00, 0x00, 0x50, 0x8A, 0x35, 0xF0 };
static unsigned char hack[] = { 0x18, 0x00, 0x01, 0x8B, 0x50, 0x8A, 0x35, 0xE0, 0x49, 0x45, 0x45, 0x45, 0x20, 0x38, 0x30, 0x32, 0x2E, 0x31, 0x31, 0x62, 0x67, 0x00, 0x35, 0xF0 };
//static unsigned char hack[] = { 0x0C, 0x00, 0x07, 0x8B, 0x50, 0x8A, 0x35, 0xE0, 0x03, 0x00, 0x00, 0x00 };
//static unsigned char hack[] = { 0x19, 0x00, 0x1B, 0x8B, 0x00, 0x00, 0x00, 0x00, 0x09, 0x00, 0x01, 0x00, 0x50, 0x8A, 0x35, 0xF0, 0x47, 0x6F, 0x6C, 0x6F, 0x73, 0x4E, 0x65, 0x74, 0x7A };
//static unsigned char hack[] = { 0x10, 0x00, 0x05, 0x8B, 0x50, 0x8A, 0x35, 0xE0, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x32 };
//static unsigned char hack[] = { 0x10, 0x00, 0x2B, 0x8B, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x67, 0x00, 0x35, 0xF0 };
//static unsigned char hack[] = { 0x70, 0x00, 0x21, 0x8B, 0x50, 0x8A, 0x35, 0xE0, 0x40, 0x42, 0x0F, 0x00, 0x00, 0x00, 0x00, 0x32, 0x80, 0x84, 0x1E, 0x00, 0x00, 0x00, 0x00, 0x32, 0x60, 0xEC, 0x53, 0x00, 0x00, 0x00, 0x00, 0x32, 0x80, 0x8D, 0x5B, 0x00, 0x00, 0x00, 0x00, 0x32, 0x40, 0x54, 0x89, 0x00, 0x00, 0x00, 0x00, 0x32, 0xC0, 0xD8, 0xA7, 0x00, 0x00, 0x00, 0x00, 0x00 };
static int hacksize = sizeof(hack);
On Tue, Oct 09, 2007 at 11:15:16AM +0200, Johannes Berg wrote:
> If I were to venture a guess I'd say that Jouni would accept a (sane)
> replacement if you were to license it under dual BSD/GPL as the rest of
> wpa_supplicant/hostapd.
Maybe, maybe not. License is just one of the reasons (but a strong one;
there's no way I would use GPL-only library here). The other part is in
the likelyhood of target systems having the library available. If it is
not expected to be available in more or less all targets, I do not
really like to start requiring it. wpa_supplicant/hostapd are used in
number of embedded designs and the less external libraries are required
the better.
Unfortunately, I do not have a device that would show the 64-bit issue.
Could you please remind me which devices are affected and how common
they are? So far, not very many people have complained about the issues
with wpa_supplicant on 64-bit systems.
> Could you try wpa_supplicant with kernel patch below on your 32-bit
> machine? I think this should make the problem show up on 32-bit
> machines. Not having a 64-bit machine needn't stop you from fixing the
> alignment problems :) I can then verify that it works and didn't break
> 64/64 setups if you want.
This sounds like an interesting way of resolving the lack of hardware
part of the problem.. ;-) If this approach is confirmed to work, I
would be quite a bit more likely to actually find time to take a closer
look at the problem.
--
Jouni Malinen PGP id EFC895FA
On Mon, Oct 01, 2007 at 10:18:10AM -0400, Dan Williams wrote:
> On Thu, 2007-09-20 at 13:56 -0700, Jean Tourrilhes wrote:
> >
> > Well, as it happens, 0.6.5 uses wpa_supplicant for the scan if
> > it's available (more below).
>
> This is mainly to ensure that wpa_supplicant and NM scan in a
> coordinated manner. Otherwise you get into a situation where
> wpa_supplicant scans, and NM's scan timer happens to fall right after
> wpa_supplicant's, and the driver gets two back-to-back scan requests
> (maybe the second one even overlaps the first). Makes drivers confused.
Yes, I fully agree with this decision, it look to me the right
way to do it.
> > I'm currently stuck because I don't have a box handy to try
> > NetworkManager on, most of my boxes are without all the Gnome
> > overhead. I'll try to fix that, but it may take a few days.
> > Meanwhile, I made a few patch just for you for NetworkManager
> > 0.6.5. I could not even try to compile it (./configure dependancy), so
> > beware.
>
> I think you already did the patch and sent it, and it was committed on
> March 2, 2007 to NM CVS. Unfortunately, I don't think we've cut a
> release of NM "stable" (which would be 0.6.5) for a long time, which is
> probably quite overdue. Most distros should be shipping a 0.6.5
> snapshot at least.
The patch last march was for the "scanning" code. The new
patch is for the "event" code. Yes, last march, I totally forgot about
the event code, sorry about that.
From my casual glance around, most distro ship what they call
"0.6.5". The fact that it's not a release means that all those "0.6.5"
are probably not the exact same version, as they are most likely at
different CVS time. This looks to me like the receipe for some
confusion. I personally don't know which "0.6.5" are good and which
are not, I assume most are good, but without looking at the actual
code, who knows...
In other words, if you eventually do a release, you may want
to call it "0.6.6".
> Dan
Have fun...
Jean
On Thu, 2007-09-20 at 13:56 -0700, Jean Tourrilhes wrote:
> On Thu, Sep 20, 2007 at 07:01:09PM +0200, Johannes Berg wrote:
> >
> > > Then, I modified NetworkManaged to use libiw for scan
> > > parsing. The idea was to simplify NetworkManager and fix the 32-64 bit
> > > bug, plus a few other potential gotchas. The first version of
> > > NetworkManager to include that fix is 0.6.5. But, I've just realised
> > > that I did not convert event parsing, which could be an issue, I'll
> > > try to work on that.
> > > Note that the other big issue is that, if wpa_supplicant is
> > > present, NetworkManager will request the scan from it, and won't use
> > > its internal code, so all those fixes are useless. Maybe there should
> > > be a control to force NetworkManager to use its own scan code when
> > > needed.
> >
> > Does NM always use wpa_supplicant even in 0.6.5? I thought only later
> > versions deferred everything to it.
>
> That was my assumption as well, and the reason I spent time
> doing my patch. However, after your bug report, and before replying to
> you, I went back looking at the code, to verify.
> Well, as it happens, 0.6.5 uses wpa_supplicant for the scan if
> it's available (more below).
This is mainly to ensure that wpa_supplicant and NM scan in a
coordinated manner. Otherwise you get into a situation where
wpa_supplicant scans, and NM's scan timer happens to fall right after
wpa_supplicant's, and the driver gets two back-to-back scan requests
(maybe the second one even overlaps the first). Makes drivers confused.
> > > As far as I know, Debian testing (Lenny) has those
> > > packages. Of course, I would not mind if you could test all this,
> > > verify that the packages are the right version and that iwlist works
> > > properly. If iwlist does not work, the rest will never works.
> >
> > iwlist works fine on my quad G5 box, but NM doesn't. I suppose then it
> > does use wpa_supplicant.
>
> Ok, we are making slow progress, but at least we are making
> progress. We are highly dependant on good bug reports.
>
> > > With respect to wpa_supplicant. Well, I sent multiple e-mail
> > > to Jouni to inform him about this. My personaly inclination would be
> > > to rip the custom parsing code of wpa_supplicant and use libiw
> > > instead, but Jouni will never accept that. Maybe you should use
> > > xsupplicant instead.
> >
> > Can't really use xsupplicant, since NM relies on it and we want the
> > kernel to rely more on wpa_supplicant (or another userspace MLME
> > implementation). Is it really hard to fix the parsing code in
> > wpa_supplicant? I'm not familiar with the workaround at all.
>
> Ask Jouni ?
> For someone familiar with it, it's actually trivial, you just
> have to follow the patch for Wireless Tools. But for somebody
> unfamiliar, there is a huge learning curve.
> That's classical : all the eyes are on the kernel, and nobody
> care about userspace. We really should have more people familiar with
> the guts of wpa_supplicant.
>
> > > Ok, I see what's happening. That would just prevent you to set
> > > authentication information, but that is not the root caause of your
> > > problems. I'll puch a fix ASAP to John.
> >
> > Not sure, this seems to make wpa_supplicant unhappy enough to not even
> > start doing anything. But then I configured it for WPA.
>
> Patch sent. Can't do harm anyway.
>
> > johannes
>
> I'm currently stuck because I don't have a box handy to try
> NetworkManager on, most of my boxes are without all the Gnome
> overhead. I'll try to fix that, but it may take a few days.
> Meanwhile, I made a few patch just for you for NetworkManager
> 0.6.5. I could not even try to compile it (./configure dependancy), so
> beware.
I think you already did the patch and sent it, and it was committed on
March 2, 2007 to NM CVS. Unfortunately, I don't think we've cut a
release of NM "stable" (which would be 0.6.5) for a long time, which is
probably quite overdue. Most distros should be shipping a 0.6.5
snapshot at least.
Dan
> The first patch fix the event parsing code to use the libiw
> helpers. As you can see, this dramatically reduce the code
> complexity. However, as this code does not use the payload of events,
> it should not have been affected by the 32-64 bit issue.
>
> The second patch force NetworkManager to always use its own
> scanning code. This is a quick hack, I don't know what will be the
> interaction with wpa_supplicant and I don't know when this part of
> NetworkManager was last tested.
> In theory, with that change, you should start to see a list of
> networks in NetworkManager.
>
> Have fun...
>
> Jean
On Wed, 2007-09-26 at 16:51 -0700, Jean Tourrilhes wrote:
> The patch I sent to Jouni was not a wpa_supplicant, but the
> Wireless Tools patch. The complete fix is across multiple Wireless
> Tools versions, and mixed with other stuff, this patch highlight
> clearly the changes.
> The patch probably does not have enough context to figure out
> everything, but at least point in the right direction. It's
> documentated, but feel free to ask questions. I also have tests
> vectors that you can use on i386.
I give up. I don't understand this structure packing at all nor do I
really want to. All the "cmd implies which event structure to use" and
"iw_point is defined as {pointer, length, flags} but used as {length,
flags}"... If there were at least proper structures defined :(
> I personally don't like much the parser used in
> wpa_supplicant, and it's somewhat different from my code. You will
> find it in those two functions :
If I were to venture a guess I'd say that Jouni would accept a (sane)
replacement if you were to license it under dual BSD/GPL as the rest of
wpa_supplicant/hostapd.
Could you try wpa_supplicant with kernel patch below on your 32-bit
machine? I think this should make the problem show up on 32-bit
machines. Not having a 64-bit machine needn't stop you from fixing the
alignment problems :) I can then verify that it works and didn't break
64/64 setups if you want.
johannes
---
include/linux/wireless.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
--- linux-2.6-git.orig/include/linux/wireless.h 2007-10-08 14:21:47.119641377 +0200
+++ linux-2.6-git/include/linux/wireless.h 2007-10-08 14:23:17.136641377 +0200
@@ -664,7 +664,10 @@ struct iw_param
*/
struct iw_point
{
- void __user *pointer; /* Pointer to the data (in user space) */
+ union {
+ void __user *pointer; /* Pointer to the data (in user space) */
+ u64 on_64_bit_machines_the_pointer_is_64bits;
+ };
__u16 length; /* number of fields or size in bytes */
__u16 flags; /* Optional params */
};