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, 21 Dec 2016 10:16:54 -0500 From: Josef Bacik <jbacik@...com> To: Hannes Frederic Sowa <hannes@...essinduktion.org> CC: <davem@...emloft.net>, <kraigatgoog@...il.com>, <eric.dumazet@...il.com>, <tom@...bertland.com>, <netdev@...r.kernel.org>, <kernel-team@...com> Subject: Re: [PATCH 1/5 net-next] inet: replace ->bind_conflict with ->rcv_saddr_equal On Wed, Dec 21, 2016 at 10:06 AM, Hannes Frederic Sowa <hannes@...essinduktion.org> wrote: > On Tue, 2016-12-20 at 15:07 -0500, Josef Bacik wrote: >> The only difference between inet6_csk_bind_conflict and >> inet_csk_bind_conflict >> is how they check the rcv_saddr. Since we want to be able to check >> the saddr in >> other places just drop the protocol specific ->bind_conflict and >> replace it with >> ->rcv_saddr_equal, then make inet_csk_bind_conflict the one true >> bind conflict >> function. >> >> Signed-off-by: Josef Bacik <jbacik@...com> >> > > > >> --- >> include/net/inet6_connection_sock.h | 5 ----- >> include/net/inet_connection_sock.h | 9 +++------ >> net/dccp/ipv4.c | 3 ++- >> net/dccp/ipv6.c | 2 +- >> net/ipv4/inet_connection_sock.c | 22 +++++++------------- >> net/ipv4/tcp_ipv4.c | 3 ++- >> net/ipv4/udp.c | 1 + >> net/ipv6/inet6_connection_sock.c | 40 >> ------------------------------------- >> net/ipv6/tcp_ipv6.c | 4 ++-- >> 9 files changed, 18 insertions(+), 71 deletions(-) >> >> diff --git a/include/net/inet6_connection_sock.h >> b/include/net/inet6_connection_sock.h >> index 3212b39..8ec87b6 100644 >> --- a/include/net/inet6_connection_sock.h >> +++ b/include/net/inet6_connection_sock.h >> @@ -15,16 +15,11 @@ >> >> #include <linux/types.h> >> >> -struct inet_bind_bucket; >> struct request_sock; >> struct sk_buff; >> struct sock; >> struct sockaddr; >> >> -int inet6_csk_bind_conflict(const struct sock *sk, >> - const struct inet_bind_bucket *tb, bool relax, >> - bool soreuseport_ok); >> - >> struct dst_entry *inet6_csk_route_req(const struct sock *sk, >> struct flowi6 *fl6, >> const struct request_sock *req, u8 proto); >> >> diff --git a/include/net/inet_connection_sock.h >> b/include/net/inet_connection_sock.h >> index ec0479a..9cd43c5 100644 >> --- a/include/net/inet_connection_sock.h >> +++ b/include/net/inet_connection_sock.h >> @@ -62,9 +62,9 @@ struct inet_connection_sock_af_ops { >> char __user *optval, int __user *optlen); >> #endif >> void (*addr2sockaddr)(struct sock *sk, struct sockaddr *); >> - int (*bind_conflict)(const struct sock *sk, >> - const struct inet_bind_bucket *tb, >> - bool relax, bool soreuseport_ok); >> + int (*rcv_saddr_equal)(const struct sock *sk1, >> + const struct sock *sk2, >> + bool match_wildcard); >> void (*mtu_reduced)(struct sock *sk); >> }; >> >> > > The patch looks as a nice code cleanup already! > > Have you looked if we can simply have one rcv_saddr_equal for both > ipv4 > and ipv6 that e.g. uses sk->sk_family instead of function pointers? > This could give us even more possibilities to remove some indirect > functions calls and thus might relieve some cycles? I was going to do that but I'm not familiar enough with how sockets work to be comfortable. My main concern is we have the ipv6_only() check on a socket, which seems to indicate to me that you can have a socket that can do both ipv4/ipv6, so what if we're specifically going through the ipv6 code, but we aren't ipv6_only() and we end up doing the ipv4 address compare when we really need to do the ipv6 address compare? If this can't happen (and honestly as I type it out it sounds crazier than it did in my head) then yeah I'll totally do that as well and we can just have a global function without the protocol specific callbacks, but I need you or somebody to tell me I'm crazy and that it would be ok to have it all in one function. Thanks, Josef
Powered by blists - more mailing lists