Commit 10d8dad8 authored by Gerrit Renker's avatar Gerrit Renker Committed by John W. Linville

wext: fix alignment problem in serializing 'struct iw_point'

wext: fix alignment problem in serializing 'struct iw_point'

This fixes a typo in the definition of the serialized length of struct iw_point:
 a) wireless.h is exported to userspace, the typo causes IW_EV_POINT_PK_LEN
    to be 12 on 64-bit, and 8 on 32-bit systems (causing misalignment);
 b) in compat-64 mode iwe_stream_add_point() memcpys overlap (see below).

The second case in  in compat-64 mode looks like (variable names are as in
include/net/iw_handler.h:iwe_stream_add_point()):

 point_len = IW_EV_COMPAT_POINT_LEN = 8
 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
 <-------------->                *---> 'extra' data area
 +-------+-------+-------+-------+---------------+------- ...-+
 | len   | cmd   |length | flags |  (empty) -> extra      ... |
 +-------+-------+-------+-------+---------------+------- ...-+
    2       2       2       2          4

     lcp_len
 <-------------->                <-!! OVERLAP !!>
 <--1st memcpy--><------- 2nd memcpy ----------->
                                 <---- 3rd memcpy ------- ... >
 <--------- point_len ---------->

This case could cause overrun whenever iw_point.length < 4.
The other two cases are -
 * 32-bit systems: IW_EV_POINT_PK_LEN - IW_EV_LCP_PK_LEN =  8 - 4 = 4,
   the second memcpy copies exactly the 4 required bytes;
 * 64-bit systems: IW_EV_POINT_PK_LEN - IW_EV_LCP_PK_LEN = 12 - 4 = 8,
   the second memcpy copies a superfluous (but non overlapping) 4 bytes.

The patch changes IW_EV_POINT_PK_LEN to be 8, so that in all 3 cases always only
the requested iw_point.{length,flags} (both __u16) are copied, avoiding overrrun
(compat-64) and superfluous copy (64-bit). In addition, the userspace header is
sanitized (in agreement with version 30 of the wireless tools).

Many thanks to Johannes Berg for help and review with this patch.
Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
Signed-off-by: default avatarJohn W. Linville <linville@tuxdriver.com>
parent 908ebfb9
......@@ -1157,6 +1157,6 @@ struct __compat_iw_event {
#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_LEN + 4)
#define IW_EV_POINT_PK_LEN (IW_EV_LCP_PK_LEN + 4)
#endif /* _LINUX_WIRELESS_H */
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment