| 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
| ||
|
Message-ID: <20160106181950.GE6061@mrl.redhat.com>
Date: Wed, 6 Jan 2016 16:19:50 -0200
From: Marcelo Ricardo Leitner <mleitner@...hat.com>
To: Xin Long <lucien.xin@...il.com>
Cc: Vlad Yasevich <vyasevich@...il.com>,
network dev <netdev@...r.kernel.org>,
linux-sctp@...r.kernel.org, Vlad Yasevich <vyasevic@...hat.com>,
daniel@...earbox.net, davem <davem@...emloft.net>
Subject: Re: [PATCH net-next 1/5] sctp: add the rhashtable apis for sctp
global transport hashtable
On Thu, Jan 07, 2016 at 01:01:11AM +0800, Xin Long wrote:
> On Wed, Jan 6, 2016 at 2:38 AM, Vlad Yasevich <vyasevich@...il.com> wrote:
> > On 12/30/2015 10:50 AM, Xin Long wrote:
> >> tranport hashtbale will replace the association hashtable to do the
> >> lookup for transport, and then get association by t->assoc, rhashtable
> >> apis will be used because of it's resizable, scalable and using rcu.
> >>
> >> lport + rport + paddr will be the base hashkey to locate the chain,
> >> with net to protect one netns from another, then plus the laddr to
> >> compare to get the target.
> >>
> >> this patch will provider the lookup functions:
> >> - sctp_epaddr_lookup_transport
> >> - sctp_addrs_lookup_transport
> >>
> >> hash/unhash functions:
> >> - sctp_hash_transport
> >> - sctp_unhash_transport
> >>
> >> init/destroy functions:
> >> - sctp_transport_hashtable_init
> >> - sctp_transport_hashtable_destroy
> >>
> >> Signed-off-by: Xin Long <lucien.xin@...il.com>
> >> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
> >> ---
> >> include/net/sctp/sctp.h | 11 ++++
> >> include/net/sctp/structs.h | 5 ++
> >> net/sctp/input.c | 131 +++++++++++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 147 insertions(+)
> >>
> >> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> >> index ce13cf2..7bbdfba 100644
> >> --- a/include/net/sctp/sctp.h
> >> +++ b/include/net/sctp/sctp.h
> >> @@ -143,6 +143,17 @@ void sctp_icmp_proto_unreachable(struct sock *sk,
> >> struct sctp_transport *t);
> >> void sctp_backlog_migrate(struct sctp_association *assoc,
> >> struct sock *oldsk, struct sock *newsk);
> >> +int sctp_transport_hashtable_init(void);
> >> +void sctp_transport_hashtable_destroy(void);
> >> +void sctp_hash_transport(struct sctp_transport *t);
> >> +void sctp_unhash_transport(struct sctp_transport *t);
> >> +struct sctp_transport *sctp_addrs_lookup_transport(
> >> + struct net *net,
> >> + const union sctp_addr *laddr,
> >> + const union sctp_addr *paddr);
> >> +struct sctp_transport *sctp_epaddr_lookup_transport(
> >> + const struct sctp_endpoint *ep,
> >> + const union sctp_addr *paddr);
> >>
> >> /*
> >> * sctp/proc.c
> >> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> >> index eea9bde..4ab87d0 100644
> >> --- a/include/net/sctp/structs.h
> >> +++ b/include/net/sctp/structs.h
> >> @@ -48,6 +48,7 @@
> >> #define __sctp_structs_h__
> >>
> >> #include <linux/ktime.h>
> >> +#include <linux/rhashtable.h>
> >> #include <linux/socket.h> /* linux/in.h needs this!! */
> >> #include <linux/in.h> /* We get struct sockaddr_in. */
> >> #include <linux/in6.h> /* We get struct in6_addr */
> >> @@ -123,6 +124,8 @@ extern struct sctp_globals {
> >> struct sctp_hashbucket *assoc_hashtable;
> >> /* This is the sctp port control hash. */
> >> struct sctp_bind_hashbucket *port_hashtable;
> >> + /* This is the hash of all transports. */
> >> + struct rhashtable transport_hashtable;
> >>
> >> /* Sizes of above hashtables. */
> >> int ep_hashsize;
> >> @@ -147,6 +150,7 @@ extern struct sctp_globals {
> >> #define sctp_assoc_hashtable (sctp_globals.assoc_hashtable)
> >> #define sctp_port_hashsize (sctp_globals.port_hashsize)
> >> #define sctp_port_hashtable (sctp_globals.port_hashtable)
> >> +#define sctp_transport_hashtable (sctp_globals.transport_hashtable)
> >> #define sctp_checksum_disable (sctp_globals.checksum_disable)
> >>
> >> /* SCTP Socket type: UDP or TCP style. */
> >> @@ -753,6 +757,7 @@ static inline int sctp_packet_empty(struct sctp_packet *packet)
> >> struct sctp_transport {
> >> /* A list of transports. */
> >> struct list_head transports;
> >> + struct rhash_head node;
> >>
> >> /* Reference counting. */
> >> atomic_t refcnt;
> >> diff --git a/net/sctp/input.c b/net/sctp/input.c
> >> index b6493b3..bac8278 100644
> >> --- a/net/sctp/input.c
> >> +++ b/net/sctp/input.c
> >> @@ -782,6 +782,137 @@ hit:
> >> return ep;
> >> }
> >>
> >> +/* rhashtable for transport */
> >> +struct sctp_hash_cmp_arg {
> >> + const union sctp_addr *laddr;
> >> + const union sctp_addr *paddr;
> >> + const struct net *net;
> >> +};
> >> +
> >> +static inline int sctp_hash_cmp(struct rhashtable_compare_arg *arg,
> >> + const void *ptr)
> >> +{
> >> + const struct sctp_hash_cmp_arg *x = arg->key;
> >> + const struct sctp_transport *t = ptr;
> >> + struct sctp_association *asoc = t->asoc;
> >> + const struct net *net = x->net;
> >> +
> >> + if (x->laddr->v4.sin_port != htons(asoc->base.bind_addr.port))
> >> + return 1;
> >> + if (!sctp_cmp_addr_exact(&t->ipaddr, x->paddr))
> >> + return 1;
> >> + if (!net_eq(sock_net(asoc->base.sk), net))
> >> + return 1;
> >> + if (!sctp_bind_addr_match(&asoc->base.bind_addr,
> >> + x->laddr, sctp_sk(asoc->base.sk)))
> >> + return 1;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static inline u32 sctp_hash_obj(const void *data, u32 len, u32 seed)
> >> +{
> >> + const struct sctp_transport *t = data;
> >> + const union sctp_addr *paddr = &t->ipaddr;
> >> + const struct net *net = sock_net(t->asoc->base.sk);
> >> + u16 lport = htons(t->asoc->base.bind_addr.port);
> >> + u32 addr;
> >> +
> >> + if (paddr->sa.sa_family == AF_INET6)
> >> + addr = jhash(&paddr->v6.sin6_addr, 16, seed);
> >> + else
> >> + addr = paddr->v4.sin_addr.s_addr;
> >> +
> >> + return jhash_3words(addr, ((__u32)paddr->v4.sin_port) << 16 |
> >> + (__force __u32)lport, net_hash_mix(net), seed);
> >> +}
> >> +
> >> +static inline u32 sctp_hash_key(const void *data, u32 len, u32 seed)
> >> +{
> >> + const struct sctp_hash_cmp_arg *x = data;
> >> + const union sctp_addr *paddr = x->paddr;
> >> + const struct net *net = x->net;
> >> + u16 lport = x->laddr->v4.sin_port;
> >> + u32 addr;
> >> +
> >> + if (paddr->sa.sa_family == AF_INET6)
> >> + addr = jhash(&paddr->v6.sin6_addr, 16, seed);
> >> + else
> >> + addr = paddr->v4.sin_addr.s_addr;
> >> +
> >> + return jhash_3words(addr, ((__u32)paddr->v4.sin_port) << 16 |
> >> + (__force __u32)lport, net_hash_mix(net), seed);
> >> +}
> >> +
> >> +static const struct rhashtable_params sctp_hash_params = {
> >> + .head_offset = offsetof(struct sctp_transport, node),
> >> + .hashfn = sctp_hash_key,
> >> + .obj_hashfn = sctp_hash_obj,
> >> + .obj_cmpfn = sctp_hash_cmp,
> >> + .automatic_shrinking = true,
> >> +};
> >> +
> >> +int sctp_transport_hashtable_init(void)
> >> +{
> >> + return rhashtable_init(&sctp_transport_hashtable, &sctp_hash_params);
> >> +}
> >> +
> >> +void sctp_transport_hashtable_destroy(void)
> >> +{
> >> + rhashtable_destroy(&sctp_transport_hashtable);
> >> +}
> >> +
> >> +void sctp_hash_transport(struct sctp_transport *t)
> >> +{
> >> + struct sctp_sockaddr_entry *addr;
> >> + struct sctp_hash_cmp_arg arg;
> >> +
> >> + addr = list_entry(t->asoc->base.bind_addr.address_list.next,
> >> + struct sctp_sockaddr_entry, list);
> >> + arg.laddr = &addr->a;
> >> + arg.paddr = &t->ipaddr;
> >> + arg.net = sock_net(t->asoc->base.sk);
> >> +
> >> +reinsert:
> >> + if (rhashtable_lookup_insert_key(&sctp_transport_hashtable, &arg,
> >> + &t->node, sctp_hash_params) == -EBUSY)
> >> + goto reinsert;
> >> +}
> >> +
> >> +void sctp_unhash_transport(struct sctp_transport *t)
> >> +{
> >> + rhashtable_remove_fast(&sctp_transport_hashtable, &t->node,
> >> + sctp_hash_params);
> >> +}
> >> +
> >> +struct sctp_transport *sctp_addrs_lookup_transport(
> >> + struct net *net,
> >> + const union sctp_addr *laddr,
> >> + const union sctp_addr *paddr)
> >> +{
> >> + struct sctp_hash_cmp_arg arg = {
> >> + .laddr = laddr,
> >> + .paddr = paddr,
> >> + .net = net,
> >> + };
> >> +
> >> + return rhashtable_lookup_fast(&sctp_transport_hashtable, &arg,
> >> + sctp_hash_params);
> >> +}
> >> +
> >> +struct sctp_transport *sctp_epaddr_lookup_transport(
> >> + const struct sctp_endpoint *ep,
> >> + const union sctp_addr *paddr)
> >> +{
> >> + struct sctp_sockaddr_entry *addr;
> >> + struct net *net = sock_net(ep->base.sk);
> >> +
> >> + addr = list_entry(ep->base.bind_addr.address_list.next,
> >> + struct sctp_sockaddr_entry, list);
> >> +
> >> + return sctp_addrs_lookup_transport(net, &addr->a, paddr);
> >> +}
> >> +
> >
> > I don't think that this right, mainly because not all endpoint
> > addresses will be copied to association bind_addr list. As a result,
> > you may actually have an association on this endpoint to a given
> > peer, but may not be using the first address from the endpoint..
That shouldn't be a problem, as laddr is not considered in the hash
function and later on sctp_hash_cmp() will use sctp_bind_addr_match() as
below, which will traverse asoc->base.bind_addr in this case, meaning it
would still see all transport/asoc possibilities.
Or did I miss something?
Marcelo
> > What might work is to pick an endpoint address that would be usable within
> > the scope of the peer address.
> it's not that easy, does it make sense to you if I change
>
> if (!sctp_bind_addr_match(&asoc->base.bind_addr,
> x->laddr, sctp_sk(asoc->base.sk)))
>
> to
> if (!sctp_bind_addr_match(&asoc->ep->base.bind_addr,
> x->laddr, sctp_sk(asoc->base.sk)))
>
> in sctp_hash_cmp() ?
>
--
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