Replace unaligned 16bit access to TCP MSS value with bytewise access
PATCH 1/1 Replace unaligned 16bit access to TCP MSS value with bytewise access
TCP options are not always word-aligned, and accessing a 16bit value at an odd memory address will cause a "bus error" crash on some architectures, e.g. Linux/Sparc(64)
Trac #497
--- src/openvpn/mss.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)
src/openvpn/mss.c +8 -9
@@ -129,7 +129,7 @@ mss_fixup_dowork (struct buffer *buf, uint16_t maxmss) {int hlen, olen, optlen;uint8_t *opt; -uint16_t *mss; +uint16_t mssval;int accumulate;struct openvpn_tcphdr *tc;
@@ -159,14 +159,13 @@ mss_fixup_dowork (struct buffer *buf, uint16_t maxmss)if (*opt ==OPENVPN_TCPOPT_MAXSEG ) {if (optlen !=OPENVPN_TCPOLEN_MAXSEG )continue ; - mss = (uint16_t *)(opt +2 ); -if (ntohs (*mss) > maxmss) { -dmsg (D_MSS ,"MSS: %d -> %d" , - (int )ntohs (*mss), - (int ) maxmss); - accumulate = *mss; - *mss =htons (maxmss); - accumulate -= *mss; + mssval = (opt[2 ]<<8 )+opt[3 ]; +if (mssval > maxmss) { +dmsg (D_MSS ,"MSS: %d -> %d" , (int ) mssval, (int ) maxmss); + accumulate =htons (mssval); + opt[2 ] = (maxmss>>8 )&0xff ; + opt[3 ] = maxmss&0xff ; + accumulate -=htons (maxmss);ADJUST_CHECKSUM (accumulate, tc->check); } }
Signature · 1 line
2.3.6
Quoted 37 lines · Gert Doering
TCP options are not always word-aligned, and accessing a 16bit value at an odd memory address will cause a "bus error" crash on some architectures, e.g. Linux/Sparc(64)
Trac #497
Signed-off-by Gert Doering <[email protected]...>--- src/openvpn/mss.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)src/openvpn/mss.c +1
-9@@ -129,7 +129,7 @@ mss_fixup_dowork (struct buffer *buf, uint16_t maxmss) {int hlen, olen, optlen;uint8_t *opt; -uint16_t *mss; +uint16_t mssval;int accumulate;struct openvpn_tcphdr *tc;@@ -159,14 +159,13 @@ mss_fixup_dowork (struct buffer *buf, uint16_t maxmss)if (*opt ==OPENVPN_TCPOPT_MAXSEG ) {if (optlen !=OPENVPN_TCPOLEN_MAXSEG )continue ; - mss = (uint16_t *)(opt +2 ); -if (ntohs (*mss) > maxmss) { -dmsg (D_MSS ,"MSS: %d -> %d" , - (int )ntohs (*mss), - (int ) maxmss); - accumulate = *mss; - *mss =htons (maxmss); - accumulate -= *mss;
src/openvpn/mss.c mss_fixup_dowork (struct buffer *buf, uint16_t maxmss) · line 171
Quoted 10 lines
src/openvpn/mss.c +7
-0+ mssval = (opt[2 ]<<8 )+opt[3 ]; +if (mssval > maxmss) { +dmsg (D_MSS ,"MSS: %d -> %d" , (int ) mssval, (int ) maxmss); + accumulate =htons (mssval); + opt[2 ] = (maxmss>>8 )&0xff ; + opt[3 ] = maxmss&0xff ; + accumulate -=htons (maxmss);ADJUST_CHECKSUM (accumulate, tc->check); } }
ACK. Change looks fine. Though I don't have a platform to experimentally verify the fix.
-Steffan
Patch has been applied to the master and release/2.3 branch.
commit 2e2a34181962b33d70c34c28dcb1e1977c2fd54e (master) commit 9a2ab2fbebb1c6712e23c98cb06a93d40d378fcd (release/2.3)
Author: Gert Doering Date: Thu Aug 27 15:00:02 2015 +0200
Replace unaligned 16bit access to TCP MSS value with bytewise access
Signed-off-by: Gert Doering <[email protected]...>
Acked-by: Steffan Karger <[email protected]...>
Message-Id: <[email protected]...>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10056--
kind regards,
Gert Doering