| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 16 Jan 2013 13:30:53 -0800
From: Tom Herbert <therbert@...gle.com>
To: Vijay Subramanian <subramanian.vijay@...il.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net,
netdev@...kandruth.co.uk, eric.dumazet@...il.com
Subject: Re: [PATCH 2/5] soreuseport: TCP/IPv4 implementation
I'll send updated patches today fixing the issue raised so far. You
might want to put these on top of those?
On Wed, Jan 16, 2013 at 1:09 PM, Vijay Subramanian
<subramanian.vijay@...il.com> wrote:
>
>
>> * Unlike other sk lookup places we do not check
>> @@ -73,8 +75,11 @@ int inet_csk_bind_conflict(const struct sock *sk,
>> (!sk->sk_bound_dev_if ||
>> !sk2->sk_bound_dev_if ||
>> sk->sk_bound_dev_if == sk2->sk_bound_dev_if)) {
>> - if (!reuse || !sk2->sk_reuse ||
>> - sk2->sk_state == TCP_LISTEN) {
>> + if ((!reuse || !sk2->sk_reuse ||
>> + sk2->sk_state == TCP_LISTEN) &&
>> + (!reuseport || !sk2->sk_reuseport ||
>> + (sk2->sk_state != TCP_TIME_WAIT &&
>> + uid != sock_i_uid(sk2)))) {
>> const __be32 sk2_rcv_saddr =
>> sk_rcv_saddr(sk2);
>> if (!sk2_rcv_saddr || !sk_rcv_saddr(sk) ||
>> sk2_rcv_saddr == sk_rcv_saddr(sk))
>
>
>
> How about introducing some helper functions to make inet_csk_bid_conflict()
> and inet6_csk_bind_conflict() more readable as in patch below? We can add
> another test for reuseport() for soreuseport patches.
>
> udp.c already has ipv4_rcv_saddr_equal() but it seems to call
> ipv6_only_sock() and not inet_v6_ipv6only() which is needed in
> inet{6}_csk_bid_conflict().So I added sk_rcv_saddr_equal().
>
> Also the bind_conflict functions can return bool instead of int (not
> implemented in patch below).
>
>
> If patch idea below is ok, I will send it officially.
>
> Thanks,
> Vijay
>
>
> diff --git a/include/net/inet_connection_sock.h
> b/include/net/inet_connection_sock.h
> index 1832927..c15d2eb 100644
> --- a/include/net/inet_connection_sock.h
> +++ b/include/net/inet_connection_sock.h
> @@ -22,6 +22,8 @@
>
> #include <net/inet_sock.h>
> #include <net/request_sock.h>
> +#include <net/tcp_states.h>
> +#include <net/inet_timewait_sock.h>
>
> #define INET_CSK_DEBUG 1
>
> @@ -205,6 +207,37 @@ static inline void inet_csk_clear_xmit_timer(struct
> sock *sk, const int what)
> #endif
> }
>
> +/* The port cannot be reused if the older socket is in LISTEN state or if
> + * either the old or new one does not allow reuse
> + */
> +static inline bool sk_reuse_equal(int reuse, const struct sock *sk2)
> +{
> + return !reuse || !sk2->sk_reuse || sk2->sk_state == TCP_LISTEN;
> +}
> +
> +/* The port cannot be reused if both sockets are bound to the same device
> or
> + * if either one is not bound
> + */
> +static inline bool sk_bound_dev_equal(const struct sock *sk,
> + const struct sock *sk2)
> +{
> + return !sk->sk_bound_dev_if || !sk2->sk_bound_dev_if ||
> + sk->sk_bound_dev_if == sk2->sk_bound_dev_if;
> +}
> +
> +/* The port cannot be reused if both sockets have the same rcv_saddr
> + * or if either rcv_saddr is NULL
> + */
> +static inline bool sk_rcv_saddr_equal(const struct sock *sk1,
> + const struct sock *sk2)
> +{
> + __be32 sk1_rcv_saddr = sk_rcv_saddr(sk1);
> + __be32 sk2_rcv_saddr = sk_rcv_saddr(sk2);
> +
> + return !sk2_rcv_saddr || !sk1_rcv_saddr ||
> + sk2_rcv_saddr == sk1_rcv_saddr;
> +}
> +
> /*
> * Reset the retransmission timer
> */
> diff --git a/net/ipv4/inet_connection_sock.c
> b/net/ipv4/inet_connection_sock.c
> index d0670f0..375cca3 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -68,26 +68,15 @@ int inet_csk_bind_conflict(const struct sock *sk,
> */
>
> sk_for_each_bound(sk2, node, &tb->owners) {
> - if (sk != sk2 &&
> - !inet_v6_ipv6only(sk2) &&
> - (!sk->sk_bound_dev_if ||
> - !sk2->sk_bound_dev_if ||
>
> - sk->sk_bound_dev_if == sk2->sk_bound_dev_if)) {
> - if (!reuse || !sk2->sk_reuse ||
> - sk2->sk_state == TCP_LISTEN) {
> - const __be32 sk2_rcv_saddr =
> sk_rcv_saddr(sk2);
> - if (!sk2_rcv_saddr || !sk_rcv_saddr(sk) ||
> - sk2_rcv_saddr == sk_rcv_saddr(sk))
> - break;
> - }
> - if (!relax && reuse && sk2->sk_reuse &&
> - sk2->sk_state != TCP_LISTEN) {
> - const __be32 sk2_rcv_saddr =
> sk_rcv_saddr(sk2);
> + if (sk != sk2 && !inet_v6_ipv6only(sk2) &&
> + sk_bound_dev_equal(sk, sk2)) {
> + if (sk_reuse_equal(reuse, sk2) &&
> + sk_rcv_saddr_equal(sk, sk2))
> + break;
>
> - if (!sk2_rcv_saddr || !sk_rcv_saddr(sk) ||
> - sk2_rcv_saddr == sk_rcv_saddr(sk))
> - break;
> - }
> + if (!relax && sk_reuse_equal(reuse, sk2) &&
> + sk_rcv_saddr_equal(sk, sk2))
> + break;
> }
> }
> return node != NULL;
> diff --git a/net/ipv6/inet6_connection_sock.c
> b/net/ipv6/inet6_connection_sock.c
> index 3064785..8ebe20d 100644
> --- a/net/ipv6/inet6_connection_sock.c
> +++ b/net/ipv6/inet6_connection_sock.c
> @@ -39,13 +39,9 @@ int inet6_csk_bind_conflict(const struct sock *sk,
> * vs net namespaces issues.
> */
> sk_for_each_bound(sk2, node, &tb->owners) {
> - if (sk != sk2 &&
> - (!sk->sk_bound_dev_if ||
> - !sk2->sk_bound_dev_if ||
> - sk->sk_bound_dev_if == sk2->sk_bound_dev_if) &&
> - (!sk->sk_reuse || !sk2->sk_reuse ||
> - sk2->sk_state == TCP_LISTEN) &&
> - ipv6_rcv_saddr_equal(sk, sk2))
> + if (sk != sk2 && sk_bound_dev_equal(sk, sk2) &&
> + sk_reuse_equal(sk->sk_reuse, sk2) &&
> + ipv6_rcv_saddr_equal(sk, sk2))
> break;
> }
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists