2:46 p.m.
with this series we make
setscokopt(..., IPPROTO_TCP, TCP_ULP, "mptcp", sizeof("mptcp"));
return -1 and set errno to -EOPNOPSUPPORT.
this allows attaching MPTCP ULP only to a TCP socket that has been
created through sock_create_kern().
- Patch 1/2 lets subflow_init() fail with -EOPNOTSUPPORT in case users
try to setsockopt(..., TCP_ULP) on a TCP socket that has been created
by a userspace program. I suggest squashing this patch in "mptcp:
Associate MPTCP context with TCP socket".
- Patch 2/2 includes kselftest code. I suggest squashing it with "mptcp:
add basic kselftest for mptcp".
Changes since v1:
- don't redirect the selftest output to /dev/null
- test output depends on the value of 'errno', to avoid false negative
in case kernel is built without CONFIG_MPTCP
Davide Caratti (2):
mptcp: subflow: disallow setsockopt(... TCP_ULP, "mptcp")
selftests: test setsockopt(..., TCP_ULP, "mptcp", ...)
net/mptcp/subflow.c | 7 +++
.../selftests/net/mptcp/mptcp_connect.c | 52 +++++++++++++++++--
.../selftests/net/mptcp/mptcp_connect.sh | 19 +++++++
3 files changed, 75 insertions(+), 3 deletions(-)
--
2.21.0
2:46 p.m.
New subject: [PATCH v2 1/2] mptcp: subflow: disallow setsockopt(... TCP_ULP, "mptcp")
the MPTCP ULP can be attached to a TCP socket only if the socket has been
created with sock_create_kern(): ensure that the value of sk->sk_kern_sock
is 1 in the MPTCP subflow init() function, and let it return -EOPNOTSUPP
otherwise.
Signed-off-by: Davide Caratti <dcaratti(a)redhat.com>
---
net/mptcp/subflow.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index ebe93525b0aa..b40d927e71ca 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -435,6 +435,13 @@ static int subflow_ulp_init(struct sock *sk)
struct tcp_sock *tp = tcp_sk(sk);
int err = 0;
+ /* disallow attaching ULP to a socket unless it has been
+ * created with sock_create_kern()
+ */
+ if (!sk->sk_kern_sock) {
+ err = -EOPNOTSUPP;
+ goto out;
+ }
ctx = subflow_create_ctx(sk, sk->sk_socket, GFP_KERNEL);
if (!ctx) {
err = -ENOMEM;
--
2.21.0
2:46 p.m.
New subject: [PATCH v2 2/2] selftests: test setsockopt(..., TCP_ULP, "mptcp", ...)
add a selftest to verify that the kernel forbids attaching MPTCP subflows
to userspace TCP sockets with setsockopt(..., IPPROTO_TCP, TCP_ULP, ...).
Signed-off-by: Davide Caratti <dcaratti(a)redhat.com>
---
.../selftests/net/mptcp/mptcp_connect.c | 52 +++++++++++++++++--
.../selftests/net/mptcp/mptcp_connect.sh | 19 +++++++
2 files changed, 68 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c
b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index cac71f0ac8f8..a472e056fae6 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -18,7 +18,8 @@
#include <netdb.h>
#include <netinet/in.h>
-#include <netinet/tcp.h>
+
+#include <linux/tcp.h>
extern int optind;
@@ -32,11 +33,12 @@ static int poll_timeout;
static const char *cfg_host;
static const char *cfg_port = "12000";
static int cfg_sock_proto = IPPROTO_MPTCP;
+static bool tcpulp_audit;
static void die_usage(void)
{
- fprintf(stderr, "Usage: mptcp_connect [-s MPTCP|TCP] [-p port] "
+ fprintf(stderr, "Usage: mptcp_connect [-u] [-s MPTCP|TCP] [-p port] "
"[ -l ] [ -t timeout ] connect_address\n");
exit(1);
}
@@ -114,6 +116,44 @@ static int sock_listen_mptcp(const char * const listenaddr,
return sock;
}
+static bool sock_test_tcpulp(const char * const remoteaddr,
+ const char * const port)
+{
+
+ struct addrinfo hints = {
+ .ai_protocol = IPPROTO_TCP,
+ .ai_socktype = SOCK_STREAM,
+ };
+ struct addrinfo *a, *addr;
+ int sock = -1, ret = 0;
+ bool test_pass = false;
+
+ hints.ai_family = AF_INET;
+
+ xgetaddrinfo(remoteaddr, port, &hints, &addr);
+ for (a = addr; a; a = a->ai_next) {
+ sock = socket(a->ai_family, a->ai_socktype, IPPROTO_TCP);
+ if (sock < 0) {
+ perror("socket");
+ continue;
+ }
+ ret = setsockopt(sock, IPPROTO_TCP, TCP_ULP, "mptcp",
+ sizeof("mptcp"));
+ if (ret == -1 && errno == EOPNOTSUPP)
+ test_pass = true;
+ close(sock);
+
+ if (test_pass)
+ break;
+ if (!ret)
+ fprintf(stderr,
+ "setsockopt(TCP_ULP) returned 0\n");
+ else
+ perror("setsockopt(TCP_ULP)");
+ }
+ return test_pass;
+}
+
static int sock_connect_mptcp(const char * const remoteaddr,
const char * const port, int proto)
{
@@ -362,7 +402,7 @@ static void parse_opts(int argc, char **argv)
{
int c;
- while ((c = getopt(argc, argv, "lp:s:ht:")) != -1) {
+ while ((c = getopt(argc, argv, "lp:s:hut:")) != -1) {
switch (c) {
case 'l':
listen_mode = true;
@@ -376,6 +416,9 @@ static void parse_opts(int argc, char **argv)
case 'h':
die_usage();
break;
+ case 'u':
+ tcpulp_audit = true;
+ break;
case 't':
poll_timeout = atoi(optarg) * 1000;
if (poll_timeout <= 0)
@@ -395,6 +438,9 @@ int main(int argc, char *argv[])
parse_opts(argc, argv);
+ if (tcpulp_audit)
+ return sock_test_tcpulp(cfg_host, cfg_port) ? 0 : 1;
+
if (listen_mode) {
int fd = sock_listen_mptcp(cfg_host, cfg_port);
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 7d312bd9ac77..f0528df9ac31 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -134,6 +134,23 @@ check_mptcp_disabled()
return 0
}
+check_mptcp_ulp_setsockopt()
+{
+ local t="ns_ulp" retval=
+
+ ip netns add ${t} || exit $ksft_skip
+ if ! ip netns exec ${t} ./mptcp_connect -u -p 10000 -s TCP 127.0.0.1 2>&1; then
+ printf "setsockopt(..., TCP_ULP, \"mptcp\", ...) allowed\t[ FAIL
]\n"
+ retval=1
+ ret=$retval
+ else
+ printf "setsockopt(..., TCP_ULP, \"mptcp\", ...) blocked\t[ OK
]\n"
+ retval=0
+ fi
+ ip netns del ${t}
+ return $retval
+}
+
do_ping()
{
listener_ns="$1"
@@ -271,6 +288,8 @@ make_file "$sin" "server"
check_mptcp_disabled
+check_mptcp_ulp_setsockopt
+
for sender in 1 2 3 4;do
do_ping ns1 ns$sender 10.0.1.1
--
2.21.0
2:56 p.m.
On Mon, 2019-10-14 at 16:46 +0200, Davide Caratti wrote:
with this series we make
setscokopt(..., IPPROTO_TCP, TCP_ULP, "mptcp", sizeof("mptcp"));
return -1 and set errno to -EOPNOPSUPPORT.
this allows attaching MPTCP ULP only to a TCP socket that has been
created through sock_create_kern().
- Patch 1/2 lets subflow_init() fail with -EOPNOTSUPPORT in case users
try to setsockopt(..., TCP_ULP) on a TCP socket that has been created
by a userspace program. I suggest squashing this patch in "mptcp:
Associate MPTCP context with TCP socket".
- Patch 2/2 includes kselftest code. I suggest squashing it with "mptcp:
add basic kselftest for mptcp".
Changes since v1:
- don't redirect the selftest output to /dev/null
- test output depends on the value of 'errno', to avoid false negative
in case kernel is built without CONFIG_MPTCP
Davide Caratti (2):
mptcp: subflow: disallow setsockopt(... TCP_ULP, "mptcp")
selftests: test setsockopt(..., TCP_ULP, "mptcp", ...)
net/mptcp/subflow.c | 7 +++
.../selftests/net/mptcp/mptcp_connect.c | 52 +++++++++++++++++--
.../selftests/net/mptcp/mptcp_connect.sh | 19 +++++++
3 files changed, 75 insertions(+), 3 deletions(-)
The patches LGTM, thanks Davide!
/P
5:22 p.m.
Hi Davide, Paolo,
On 14/10/2019 16:56, Paolo Abeni wrote:
On Mon, 2019-10-14 at 16:46 +0200, Davide Caratti wrote:
> with this series we make
>
> setscokopt(..., IPPROTO_TCP, TCP_ULP, "mptcp", sizeof("mptcp"));
>
> return -1 and set errno to -EOPNOPSUPPORT.
>
> this allows attaching MPTCP ULP only to a TCP socket that has been
> created through sock_create_kern().
>
> - Patch 1/2 lets subflow_init() fail with -EOPNOTSUPPORT in case users
> try to setsockopt(..., TCP_ULP) on a TCP socket that has been created
> by a userspace program. I suggest squashing this patch in "mptcp:
> Associate MPTCP context with TCP socket".
> - Patch 2/2 includes kselftest code. I suggest squashing it with "mptcp:
> add basic kselftest for mptcp".
>
> Changes since v1:
> - don't redirect the selftest output to /dev/null
> - test output depends on the value of 'errno', to avoid false negative
> in case kernel is built without CONFIG_MPTCP
>
> Davide Caratti (2):
> mptcp: subflow: disallow setsockopt(... TCP_ULP, "mptcp")
> selftests: test setsockopt(..., TCP_ULP, "mptcp", ...)
>
> net/mptcp/subflow.c | 7 +++
> .../selftests/net/mptcp/mptcp_connect.c | 52 +++++++++++++++++--
> .../selftests/net/mptcp/mptcp_connect.sh | 19 +++++++
> 3 files changed, 75 insertions(+), 3 deletions(-)
The patches LGTM, thanks Davide!
Thank you for the patches and the review!
- 87df9026e7cc: "squashed" the first patch in "mptcp: Associate MPTCP
context with TCP socket"
- 8c907c48da5c: "Signed-off-by" + "Co-developed-by"
- a8a756ed4dbc: conflict in
t/mptcp-Create-SUBFLOW-socket-for-incoming-connections
- 2057bcf6bf5e: "squashed" the second patch in "mptcp: add basic
kselftest for mptcp"
- be20e32cf37d..c7557e1f213a: result
Tests are still OK!
Cheers,
Matt
/P
_______________________________________________
mptcp mailing list -- mptcp(a)lists.01.org
To unsubscribe send an email to mptcp-leave(a)lists.01.org
--
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium
952
days inactive
952
days old
4 comments
3 participants
participants (3)
-
Davide Caratti -
Matthieu Baerts -
Paolo Abeni