| 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: Thu, 7 Jan 2016 15:23:24 -0200
From: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
To: Vlad Yasevich <vyasevich@...il.com>
Cc: Xin Long <lucien.xin@...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 Wed, Jan 06, 2016 at 04:19:50PM -0200, Marcelo Ricardo Leitner wrote:
> 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?
I did.. Xin kindly explained the issue to me offline. Yup, this is an
issue, we will work on it. Thanks
--
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