From abb972b03acf948255f1026442758adfba44baa1 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Tue, 18 Sep 2018 13:31:09 +0200 Subject: Fix endianness bug for CMSG parsing --- erts/emulator/drivers/common/inet_drv.c | 131 +++++++++++++++++++++----------- 1 file changed, 87 insertions(+), 44 deletions(-) diff --git a/erts/emulator/drivers/common/inet_drv.c b/erts/emulator/drivers/common/inet_drv.c index 3478ba7081..7f20477363 100644 --- a/erts/emulator/drivers/common/inet_drv.c +++ b/erts/emulator/drivers/common/inet_drv.c @@ -634,11 +634,10 @@ static size_t my_strnlen(const char *s, size_t maxlen) * the pointer difference from the cmsg start pointer * to the CMSG_DATA(cmsg) pointer. */ -#define LEN_CMSG_DATA(cmsg) ((char*)CMSG_DATA(cmsg) - (char*)(cmsg)) +#define LEN_CMSG_DATA(cmsg) \ + ((cmsg)->cmsg_len - ((char*)CMSG_DATA(cmsg) - (char*)(cmsg))) #define NXT_CMSG_HDR(cmsg) \ - ((struct cmsghdr*) \ - (((char*)(cmsg)) + \ - CMSG_SPACE((cmsg)->cmsg_len - LEN_CMSG_DATA(cmsg)))) + ((struct cmsghdr*)(((char*)(cmsg)) + CMSG_SPACE(LEN_CMSG_DATA(cmsg)))) #endif #if !defined(IPV6_PKTOPTIONS) && defined(IPV6_2292PKTOPTIONS) @@ -2796,39 +2795,61 @@ static int inet_async_data(inet_descriptor* desc, const char* buf, int len) } #ifndef __WIN32__ +static int load_cmsg_int(ErlDrvTermData *spec, int i, + struct cmsghdr *cmsg) { + union u { + byte uint8; + Uint16 uint16; + Uint32 uint32; + Uint64 uint64; + } *p; + p = (union u*) CMSG_DATA(cmsg); + switch (LEN_CMSG_DATA(cmsg) * CHAR_BIT) { + case 8: + return LOAD_INT(spec, i, p->uint8); + case 16: + return LOAD_INT(spec, i, p->uint16); + + case 32: + return LOAD_INT(spec, i, p->uint32); + + case 64: + return LOAD_INT(spec, i, p->uint64); + } + return LOAD_INT(spec, i, 0); +} + static int parse_ancillary_data_item(ErlDrvTermData *spec, int i, struct cmsghdr *cmsg, int *n) { -#define LOAD_CMSG(proto, type, vtype, am, load) \ - if (cmsg->cmsg_level == (proto) && \ - cmsg->cmsg_type == (type)) { \ - vtype *vp; \ - vp = (vtype *)CMSG_DATA(cmsg); \ - i = LOAD_ATOM(spec, i, (am)); \ - i = load(spec, i, *vp); \ - i = LOAD_TUPLE(spec, i, 2); \ - (*n)++; \ - return i; \ +#define LOAD_CMSG_INT(proto, type, am) \ + if (cmsg->cmsg_level == (proto) && \ + cmsg->cmsg_type == (type)) { \ + i = LOAD_ATOM(spec, i, (am)); \ + i = load_cmsg_int(spec, i, cmsg); \ + i = LOAD_TUPLE(spec, i, 2); \ + (*n)++; \ + return i; \ } #if defined(IPPROTO_IP) && defined(IP_TOS) - LOAD_CMSG(IPPROTO_IP, IP_TOS, unsigned char, am_tos, LOAD_INT); + LOAD_CMSG_INT(IPPROTO_IP, IP_TOS, am_tos); #endif #if defined(IPPROTO_IPV6) && defined(IPV6_TCLASS) - LOAD_CMSG(IPPROTO_IPV6, IPV6_TCLASS, unsigned char, am_tclass, LOAD_INT); + LOAD_CMSG_INT(IPPROTO_IPV6, IPV6_TCLASS, am_tclass); #endif #if defined(IPPROTO_IP) && defined(IP_TTL) - LOAD_CMSG(IPPROTO_IP, IP_TTL, unsigned char, am_ttl, LOAD_INT); + LOAD_CMSG_INT(IPPROTO_IP, IP_TTL, am_ttl); #endif /* BSD uses the RECV* names in CMSG fields */ #if defined(IPPROTO_IP) && defined(IP_RECVTOS) - LOAD_CMSG(IPPROTO_IP, IP_RECVTOS, unsigned char, am_tos, LOAD_INT); + LOAD_CMSG_INT(IPPROTO_IP, IP_RECVTOS, am_tos); #endif #if defined(IPPROTO_IPV6) && defined(IPV6_RECVTCLASS) - LOAD_CMSG(IPPROTO_IPV6, IPV6_RECVTCLASS, unsigned char, am_tclass, LOAD_INT); + LOAD_CMSG_INT(IPPROTO_IPV6, IPV6_RECVTCLASS, am_tclass); #endif #if defined(IPPROTO_IP) && defined(IP_RECVTTL) - LOAD_CMSG(IPPROTO_IP, IP_RECVTTL, unsigned char, am_ttl, LOAD_INT); + LOAD_CMSG_INT(IPPROTO_IP, IP_RECVTTL, am_ttl); #endif -#undef LOAD_CMSG +#undef LOAD_CMSG_INT return i; } #endif /* #ifndef __WIN32__ */ @@ -7330,6 +7351,35 @@ static int sctp_set_opts(inet_descriptor* desc, char* ptr, int len) } #endif /* HAVE_SCTP */ +#ifndef __WIN32__ +static void put_cmsg_int32(struct cmsghdr *cmsg, char *ptr) { + union u { + byte uint8; + Uint16 uint16; + Uint32 uint32; + Uint64 uint64; + } *p; + p = (union u*) CMSG_DATA(cmsg); + switch (LEN_CMSG_DATA(cmsg) * CHAR_BIT) { + case 8: + put_int32((Uint32) p->uint8, ptr); + break; + case 16: + put_int32((Uint32) p->uint16, ptr); + break; + case 32: + put_int32(p->uint32, ptr); + break; + case 64: + put_int32((Uint32) p->uint64, ptr); + break; + default: + put_int32(0, ptr); + } + return; +} +#endif + /* load all option values into the buf and reply ** return total length of reply filled into ptr ** ptr should point to a buffer with 9*len +1 to be safe!! @@ -7796,44 +7846,37 @@ static ErlDrvSSizeT inet_fill_opts(inet_descriptor* desc, cmsg = (struct cmsghdr*)cmsgbuf.buf; cmsg < cmsg_top; cmsg = NXT_CMSG_HDR(cmsg)) { -#define PUT_CMSG_DATA(CMSG_LEVEL, CMSG_TYPE, OPT, TYPE, SZ, PUT) \ - if ((cmsg->cmsg_level == CMSG_LEVEL) && \ - (cmsg->cmsg_type == CMSG_TYPE)) { \ - TYPE *cmsgp; \ - cmsgp = (TYPE *)CMSG_DATA(cmsg); \ - PLACE_FOR(1+SZ, ptr); \ - *ptr = OPT; \ - PUT(*cmsgp, ptr+1); \ - arg_sz += 1+SZ; \ - continue; \ +#define PUT_CMSG_INT32(CMSG_LEVEL, CMSG_TYPE, OPT) \ + if ((cmsg->cmsg_level == CMSG_LEVEL) && \ + (cmsg->cmsg_type == CMSG_TYPE)) { \ + PLACE_FOR(1+4, ptr); \ + *ptr++ = OPT; \ + put_cmsg_int32(cmsg, ptr); \ + ptr += 4; \ + arg_sz += 1+4; \ + continue; \ } #if defined(IPPROTO_IP) && defined(IP_TOS) - PUT_CMSG_DATA(IPPROTO_IP, IP_TOS, - INET_OPT_TOS, unsigned char, 4, put_int32); + PUT_CMSG_INT32(IPPROTO_IP, IP_TOS, INET_OPT_TOS); #endif #if defined(IPPROTO_IPV6) && defined(IPV6_TCLASS) - PUT_CMSG_DATA(IPPROTO_IPV6, IPV6_TCLASS, - INET_OPT_TCLASS, unsigned char, 4, put_int32); + PUT_CMSG_INT32(IPPROTO_IPV6, IPV6_TCLASS, INET_OPT_TCLASS); #endif #if defined(IPPROTO_IP) && defined(IP_TTL) - PUT_CMSG_DATA(IPPROTO_IP, IP_TTL, - INET_OPT_TTL, unsigned char, 4, put_int32); + PUT_CMSG_INT32(IPPROTO_IP, IP_TTL, INET_OPT_TTL); #endif /* BSD uses the RECV* names in CMSG fields */ } #if defined(IPPROTO_IP) && defined(IP_RECVTOS) - PUT_CMSG_DATA(IPPROTO_IP, IP_RECVTOS, - INET_OPT_TOS, unsigned char, 4, put_int32); + PUT_CMSG_INT32(IPPROTO_IP, IP_RECVTOS, INET_OPT_TOS); #endif #if defined(IPPROTO_IPV6) && defined(IPV6_RECVTCLASS) - PUT_CMSG_DATA(IPPROTO_IPV6, IPV6_RECVTCLASS, - INET_OPT_TCLASS, unsigned char, 4, put_int32); + PUT_CMSG_INT32(IPPROTO_IPV6, IPV6_RECVTCLASS, INET_OPT_TCLASS); #endif #if defined(IPPROTO_IP) && defined(IP_RECVTTL) - PUT_CMSG_DATA(IPPROTO_IP, IP_RECVTTL, - INET_OPT_TTL, unsigned char, 4, put_int32); + PUT_CMSG_INT32(IPPROTO_IP, IP_RECVTTL, INET_OPT_TTL); #endif -#undef PUT_CMSG_DATA +#undef PUT_CMSG_INT32 put_int32(arg_sz, arg_ptr); /* Put total length */ continue; } -- cgit v1.2.3 From 2ad4b8f65e07423dd3cf230cecae635189b6f437 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Tue, 18 Sep 2018 15:06:44 +0200 Subject: Improve platform filter --- lib/kernel/test/gen_tcp_misc_SUITE.erl | 43 ++++++++++++++++++++++++++-------- lib/kernel/test/gen_udp_SUITE.erl | 3 ++- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/lib/kernel/test/gen_tcp_misc_SUITE.erl b/lib/kernel/test/gen_tcp_misc_SUITE.erl index d532537eb9..358ca872f7 100644 --- a/lib/kernel/test/gen_tcp_misc_SUITE.erl +++ b/lib/kernel/test/gen_tcp_misc_SUITE.erl @@ -1919,7 +1919,17 @@ so_priority(Config) when is_list(Config) -> %% IP_RECVTOS and IP_RECVTCLASS for IP_PKTOPTIONS -%% does not seem to be implemented in Linux until kernel 3.0 +%% does not seem to be implemented in Linux until kernel 3.1 +%% +%% It seems pktoptions does not return valid values +%% for IPv4 connect sockets. On the accept socket +%% we get valid values, but on the connect socket we get +%% the default values for TOS and TTL. +%% +%% Therefore the argument CheckConnect that enables +%% checking the returned values for the connect socket. +%% It is only used for recvtclass that is an IPv6 option +%% and there we get valid values from both socket ends. recvtos(_Config) -> test_pktoptions( @@ -1965,25 +1975,38 @@ recvtclass(_Config) -> %% when machines with newer versions gets installed... %% If the test still fails for a plausible reason these %% version numbers simply should be increased. +%% Or maybe we should change to only test on known good +%% platforms - change {unix,_} to false? -%% Using the option returns einval, so it is not implemented. +%% pktoptions is not supported for IPv4 +recvtos_ok({unix,openbsd}, OSVer) -> not semver_lt(OSVer, {6,4,0}); recvtos_ok({unix,darwin}, OSVer) -> not semver_lt(OSVer, {17,6,0}); +recvtos_ok({unix,freebsd}, OSVer) -> not semver_lt(OSVer, {11,2,0}); +%% Using the option returns einval, so it is not implemented. +recvtos_ok({unix,sunos}, OSVer) -> not semver_lt(OSVer, {5,12,0}); %% Does not return any value - not implemented for pktoptions recvtos_ok({unix,linux}, OSVer) -> not semver_lt(OSVer, {3,1,0}); %% recvtos_ok({unix,_}, _) -> true; recvtos_ok(_, _) -> false. +%% pktoptions is not supported for IPv4 +recvttl_ok({unix,openbsd}, OSVer) -> not semver_lt(OSVer, {6,4,0}); +recvttl_ok({unix,darwin}, OSVer) -> not semver_lt(OSVer, {17,6,0}); +recvttl_ok({unix,freebsd}, OSVer) -> not semver_lt(OSVer, {11,2,0}); +%% Using the option returns einval, so it is not implemented. +recvttl_ok({unix,sunos}, OSVer) -> not semver_lt(OSVer, {5,12,0}); +%% recvttl_ok({unix,linux}, _) -> true; recvttl_ok({unix,_}, _) -> true; recvttl_ok(_, _) -> false. -%% Using the option returns einval, so it is not implemented. +%% pktoptions is not supported for IPv6 recvtclass_ok({unix,openbsd}, OSVer) -> not semver_lt(OSVer, {6,4,0}); -recvtclass_ok({unix,freebsd}, OSVer) -> not semver_lt(OSVer, {11,2,0}); -%% Using the option returns einval up to 0.9.0, so it is not implemented. -%% Does not return any value - not implemented for pktoptions recvtclass_ok({unix,darwin}, OSVer) -> not semver_lt(OSVer, {17,6,0}); +recvtclass_ok({unix,sunos}, OSVer) -> not semver_lt(OSVer, {5,12,0}); +%% Using the option returns einval, so it is not implemented. +recvtclass_ok({unix,freebsd}, OSVer) -> not semver_lt(OSVer, {11,2,0}); %% Does not return any value - not implemented for pktoptions recvtclass_ok({unix,linux}, OSVer) -> not semver_lt(OSVer, {3,1,0}); %% @@ -2002,18 +2025,18 @@ semver_lt({X1,Y1,Z1}, {X2,Y2,Z2}) -> end; semver_lt(_, {_,_,_}) -> false. -test_pktoptions(Family, Spec, OSFilter, CheckAccept) -> +test_pktoptions(Family, Spec, OSFilter, CheckConnect) -> OSType = os:type(), OSVer = os:version(), case OSFilter(OSType, OSVer) of true -> io:format("Os: ~p, ~p~n", [OSType,OSVer]), - test_pktoptions(Family, Spec, CheckAccept, OSType, OSVer); + test_pktoptions(Family, Spec, CheckConnect, OSType, OSVer); false -> {skip,{not_supported_for_os_version,{OSType,OSVer}}} end. %% -test_pktoptions(Family, Spec, CheckAccept, OSType, OSVer) -> +test_pktoptions(Family, Spec, CheckConnect, OSType, OSVer) -> Timeout = 5000, RecvOpts = [RecvOpt || {RecvOpt,_,_} <- Spec], TrueRecvOpts = [{RecvOpt,true} || {RecvOpt,_,_} <- Spec], @@ -2105,7 +2128,7 @@ test_pktoptions(Family, Spec, CheckAccept, OSType, OSVer) -> ok = gen_tcp:close(S4), ok = gen_tcp:close(S3), ok = gen_tcp:close(L), - (Result1 and ((not CheckAccept) or (Result2 and Result3))) + (Result1 and ((not CheckConnect) or (Result2 and Result3))) orelse exit({failed, [{OptsVals1,OptsVals4,OptsVals}, diff --git a/lib/kernel/test/gen_udp_SUITE.erl b/lib/kernel/test/gen_udp_SUITE.erl index 878e48786c..af9985de45 100644 --- a/lib/kernel/test/gen_udp_SUITE.erl +++ b/lib/kernel/test/gen_udp_SUITE.erl @@ -616,13 +616,14 @@ recvtclass(_Config) -> %% when machines with newer versions gets installed... %% If the test still fails for a plausible reason these %% version numbers simply should be increased. +%% Or maybe we should change to only test on known good platforms? %% Using the option returns einval, so it is not implemented. recvtos_ok({unix,darwin}, OSVer) -> not semver_lt(OSVer, {17,6,0}); %% Using the option returns einval, so it is not implemented. recvtos_ok({unix,openbsd}, OSVer) -> not semver_lt(OSVer, {6,4,0}); %% Using the option returns einval, so it is not implemented. -recvtos_ok({unix,sunos}, OSVer) -> not semver_lt(OSVer, {5,11,0}); +recvtos_ok({unix,sunos}, OSVer) -> not semver_lt(OSVer, {5,12,0}); %% recvtos_ok({unix,_}, _) -> true; recvtos_ok(_, _) -> false. -- cgit v1.2.3 From 9674ece7b2e57e265bd1dc55a623cc7888a1caa0 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Tue, 18 Sep 2018 15:46:31 +0200 Subject: Elaborate the disclaimer for 'pktoptions' --- lib/kernel/doc/src/gen_tcp.xml | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/kernel/doc/src/gen_tcp.xml b/lib/kernel/doc/src/gen_tcp.xml index cf649991d0..24d63693fd 100644 --- a/lib/kernel/doc/src/gen_tcp.xml +++ b/lib/kernel/doc/src/gen_tcp.xml @@ -74,13 +74,25 @@ do_recv(Sock, Bs) ->

If the platform implements the IPv4 option - IP_PKTOPTIONS (probably Linux specific), or the IPv6 option + IP_PKTOPTIONS, or the IPv6 option IPV6_PKTOPTIONS or IPV6_2292PKTOPTIONS for the socket this value is returned from inet:getopts/2 when called with the option name pktoptions.

+ +

+ This option appears to be VERY Linux specific, + and its existence in future Linux kernel versions + is also worrying since the option is part of RFC 2292 + which is since long (2003) obsoleted by RFC 3542 + that explicitly removes this possibility to get + packet information from a stream socket. + For comparision: it has existed in FreeBSD but is now removed, + at least since FreeBSD 10. +

+ -- cgit v1.2.3