Hi,
When I was working on CVE-2023-0461, I found the below backport commit in stable-4.19.x maybe something wrong?
755193f2523c ("net/ulp: prevent ULP without clone op from entering the LISTEN status")
1. err = -EADDRINUSE in inet_csk_listen_start() was removed. But it is the error code when get_port() fails. 2. The change in __tcp_set_ulp() should not be discarded?
Can I modify the patch like below?
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c index 0a69f92da71b..3ed2f753628e 100644 --- a/net/ipv4/inet_connection_sock.c +++ b/net/ipv4/inet_connection_sock.c @@ -903,11 +903,25 @@ void inet_csk_prepare_forced_close(struct sock *sk) } EXPORT_SYMBOL(inet_csk_prepare_forced_close);
+static int inet_ulp_can_listen(const struct sock *sk) +{ + const struct inet_connection_sock *icsk = inet_csk(sk); + + if (icsk->icsk_ulp_ops) + return -EINVAL; + + return 0; +} + int inet_csk_listen_start(struct sock *sk, int backlog) { struct inet_connection_sock *icsk = inet_csk(sk); struct inet_sock *inet = inet_sk(sk); - int err = -EADDRINUSE; + int err; + + err = inet_ulp_can_listen(sk); + if (unlikely(err)) + return err;
reqsk_queue_alloc(&icsk->icsk_accept_queue);
@@ -921,6 +935,7 @@ int inet_csk_listen_start(struct sock *sk, int backlog) * after validation is complete. */ inet_sk_state_store(sk, TCP_LISTEN); + err = -EADDRINUSE; if (!sk->sk_prot->get_port(sk, inet->inet_num)) { inet->inet_sport = htons(inet->inet_num);
diff --git a/net/ipv4/tcp_ulp.c b/net/ipv4/tcp_ulp.c index a5995bb2eaca..437987be68be 100644 --- a/net/ipv4/tcp_ulp.c +++ b/net/ipv4/tcp_ulp.c @@ -152,6 +152,11 @@ int tcp_set_ulp(struct sock *sk, const char *name) return -ENOENT; }
+ if (sk->sk_state == TCP_LISTEN) { + module_put(ulp_ops->owner); + return -EINVAL + } + err = ulp_ops->init(sk); if (err) { module_put(ulp_ops->owner);
On Fri, 3 Mar 2023 10:52:15 +0000 liujian (CE) wrote:
When I was working on CVE-2023-0461, I found the below backport commit in stable-4.19.x maybe something wrong?
755193f2523c ("net/ulp: prevent ULP without clone op from entering the LISTEN status")
- err = -EADDRINUSE in inet_csk_listen_start() was removed. But it is the error code when get_port() fails.
I think you're right, we should add setting the err back.
- The change in __tcp_set_ulp() should not be discarded?
That part should be fine, all ULPs in 4.19 (i.e. TLS) should fail the ->init() call if sk_state != ESTABLISHED.
linux-stable-mirror@lists.linaro.org