| Ah, so userspace reading this was the issue -- yes, I can see that, and
| I checked the "canonical userspace" (wireless-tools), and it essentially
| contains this patch.
|
The change should really be confined only to userspace, please see below.
There are three different alignments for three different cases, which are
caught by iwlib in userspace. Users of wireless.h can be encouraged to use the
latest iwlib, which catches the alignment issues transparently (I have been
given this advice myself).
In the drawings below I have traced with an asterisk where in userspace the data is
read out - as far as I can see this is correct. The only worrying case is number (3).
The cases are:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1) 32 bit kernel
~~~~~~~~~~~~~~~~
point_len = IW_EV_POINT_LEN = 8
iwe->len = event_len = point_len + u.data.length = 8 + u.data.length
lcp_len = IW_EV_LCP_LEN = 4
2nd memcpy: IW_EV_POINT_PK_LEN - IW_EV_LCP_PK_LEN = 8 - 4 = 4
IW_EV_LCP_PK_LEN
<--------------> *
+-------+-------+-------+-------+------------ ...-+
| len | cmd |length | flags | extra ... |
+-------+-------+-------+-------+------------ ...-+
2 2 2 2
lcp_len
<-------------->
<--1st memcpy--><- 2nd memcpy -><- 3rd memcpy ... >
<--------- point_len ---------->
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2) 64-bit kernel
~~~~~~~~~~~~~~~~
point_len = IW_EV_POINT_LEN = 16
iwe->len = event_len = point_len + u.data.length = 16 + u.data.length
lcp_len = IW_EV_LCP_LEN = 8
2nd memcpy: IW_EV_POINT_PK_LEN - IW_EV_LCP_PK_LEN = 12 - 4 = 8
IW_EV_LCP_PK_LEN
<--------------> * *
+-------+-------+---------------+-------+-------+---------------+------------ ...-+
| len | cmd | (empty) |length | flags | (empty) | extra ... |
+-------+-------+---------------+-------+-------+---------------+------------ ...-+
2 2 4 2 2 4
lcp_len
<------------------------------>
<------- 1st memcpy -----------><--------- 2nd memcpy ---------><- 3rd memcpy ... >
<--------- point_len ------------------------------------------>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3) 64-bit compat
~~~~~~~~~~~~~~~~
point_len = IW_EV_COMPAT_POINT_LEN = 8
iwe->len = event_len = point_len + u.data.length = 8 + u.data.length
lcp_len = IW_EV_COMPAT_LCP_LEN = 4
2nd memcpy: IW_EV_POINT_PK_LEN - IW_EV_LCP_PK_LEN = 12 - 4 = 8
IW_EV_LCP_PK_LEN
<--------------> *
+-------+-------+-------+-------+---------------+------- ...-+
| len | cmd |length | flags | (empty) -> extra ... |
+-------+-------+-------+-------+---------------+------- ...-+
2 2 2 2 4
lcp_len
<--------------> <-!! OVERLAP !!>
<--1st memcpy--><------- 2nd memcpy ----------->
<---- 3rd memcpy ------- ... >
<--------- point_len ---------->
Here it is as you point out: the second and third memcpy()'s overlap by 4 bytes,
which would be a problem if the length == 0 and no extra data were to be copied.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I attach a patch to fix the last case -- could you please have a look.
wext: avoid overlapping memcpy in compat-64 mode
The second memcpy in iwe_stream_add_point() copies:
* 4 bytes in 32-bit mode (sufficient for length/flags);
* 8 bytes in 64-bit mode (only first 4 bytes used);
* 8 bytes in 64-bit compat (overlaps with the third mempcy).
To avoid problems with the third memcpy, this patch reduces the copy length
to always 4 bytes = sizeof(iw_point.length) + sizeof(iw_point.flags)/
Signed-off-by: Gerrit Renker <gerrit@xxxxxxxxxxxxxx>
---
include/net/iw_handler.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/include/net/iw_handler.h
+++ b/include/net/iw_handler.h
@@ -548,9 +548,9 @@ iwe_stream_add_point(struct iw_request_info *info, char *stream, char *ends,
if(likely((stream + event_len) < ends)) {
iwe->len = event_len;
memcpy(stream, (char *) iwe, IW_EV_LCP_PK_LEN);
+ /* copy length and flags of iw_point */
memcpy(stream + lcp_len,
- ((char *) &iwe->u) + IW_EV_POINT_OFF,
- IW_EV_POINT_PK_LEN - IW_EV_LCP_PK_LEN);
+ ((char *) &iwe->u) + IW_EV_POINT_OFF, 4);
memcpy(stream + point_len, extra, iwe->u.data.length);
stream += event_len;
}
