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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 21 Aug 2014 08:09:44 +0200 From: Steffen Klassert <steffen.klassert@...unet.com> To: Christophe Gouault <christophe.gouault@...nd.com> CC: "David S. Miller" <davem@...emloft.net>, <netdev@...r.kernel.org> Subject: Re: [PATCH net-next v2 2/2] xfrm: configure policy hash table thresholds by netlink On Fri, Aug 01, 2014 at 11:12:28AM +0200, Christophe Gouault wrote: > diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h > index 41902a8..9da7982 100644 > --- a/include/net/netns/xfrm.h > +++ b/include/net/netns/xfrm.h > @@ -19,6 +19,15 @@ struct xfrm_policy_hash { > u8 sbits6; > }; > > +struct xfrm_policy_hthresh { > + struct work_struct work; > + seqlock_t lock; This newly introduced lock is not initialized. It triggers an inconsistent lock state warning when acquired for the first time. > > +static void xfrm_hash_rebuild(struct work_struct *work) > +{ > + struct net *net = container_of(work, struct net, > + xfrm.policy_hthresh.work); > + unsigned int hmask; > + struct xfrm_policy *pol; > + struct xfrm_policy *policy; > + struct hlist_head *chain; > + struct hlist_head *odst; > + struct hlist_node *newpos; > + int i; > + int dir; > + unsigned seq; > + u8 lbits4, rbits4, lbits6, rbits6; > + > + mutex_lock(&hash_resize_mutex); > + > + /* read selector prefixlen thresholds */ > + do { > + seq = read_seqbegin(&net->xfrm.policy_hthresh.lock); > + > + lbits4 = net->xfrm.policy_hthresh.lbits4; > + rbits4 = net->xfrm.policy_hthresh.rbits4; > + lbits6 = net->xfrm.policy_hthresh.lbits6; > + rbits6 = net->xfrm.policy_hthresh.rbits6; > + } while (read_seqretry(&net->xfrm.policy_hthresh.lock, seq)); > + > + write_lock_bh(&net->xfrm.xfrm_policy_lock); > + > + pr_info("rebuilding SPD hash table: thresholds (%u,%u)(%u,%u)\n", > + lbits4, rbits4, lbits6, rbits6); Do we really need to print this? > + > + /* reset the bydst and inexact table in all directions */ > + for (dir = 0; dir < XFRM_POLICY_MAX * 2; dir++) { > + INIT_HLIST_HEAD(&net->xfrm.policy_inexact[dir]); > + hmask = net->xfrm.policy_bydst[dir].hmask; > + odst = net->xfrm.policy_bydst[dir].table; > + for (i = hmask; i >= 0; i--) > + INIT_HLIST_HEAD(odst + i); > + if ((dir & XFRM_POLICY_MASK) == XFRM_POLICY_OUT) { > + /* dir out => dst = remote, src = local */ > + net->xfrm.policy_bydst[dir].dbits4 = rbits4; > + net->xfrm.policy_bydst[dir].sbits4 = lbits4; > + net->xfrm.policy_bydst[dir].dbits6 = rbits6; > + net->xfrm.policy_bydst[dir].sbits6 = lbits6; > + } else { > + /* dir in/fwd => dst = local, src = remote */ > + net->xfrm.policy_bydst[dir].dbits4 = lbits4; > + net->xfrm.policy_bydst[dir].sbits4 = rbits4; > + net->xfrm.policy_bydst[dir].dbits6 = lbits6; > + net->xfrm.policy_bydst[dir].sbits6 = rbits6; > + } > + } > + > + /* re-insert all policies by order of creation */ > + list_for_each_entry_reverse(policy, &net->xfrm.policy_all, walk.all) { > + newpos = NULL; > + chain = policy_hash_bysel(net, &policy->selector, > + policy->family, > + xfrm_policy_id2dir(policy->index)); > + hlist_for_each_entry(pol, chain, bydst) { > + if (policy->priority >= pol->priority) > + newpos = &pol->bydst; > + else > + break; > + } > + if (newpos) > + hlist_add_after(newpos, &policy->bydst); hlist_add_after() does not exist any more, it was replaced by hlist_add_behind() recently. > > +static int xfrm_set_spdinfo(struct sk_buff *skb, struct nlmsghdr *nlh, > + struct nlattr **attrs) > +{ > + struct net *net = sock_net(skb->sk); > + struct sk_buff *r_skb; > + u32 *flags = nlmsg_data(nlh); > + u32 sportid = NETLINK_CB(skb).portid; > + u32 seq = nlh->nlmsg_seq; > + struct xfrmu_spdhthresh *thresh4 = NULL; > + struct xfrmu_spdhthresh *thresh6 = NULL; > + > + /* selector prefixlen thresholds to hash policies */ > + if (attrs[XFRMA_SPD_IPV4_HTHRESH]) { > + struct nlattr *rta = attrs[XFRMA_SPD_IPV4_HTHRESH]; > + > + if (nla_len(rta) < sizeof(*thresh4)) > + return -EINVAL; > + thresh4 = nla_data(rta); > + if (thresh4->lbits > 32 || thresh4->rbits > 32) > + return -EINVAL; > + } > + if (attrs[XFRMA_SPD_IPV6_HTHRESH]) { > + struct nlattr *rta = attrs[XFRMA_SPD_IPV6_HTHRESH]; > + > + if (nla_len(rta) < sizeof(*thresh6)) > + return -EINVAL; > + thresh6 = nla_data(rta); > + if (thresh6->lbits > 128 || thresh6->rbits > 128) > + return -EINVAL; > + } > + > + if (thresh4 || thresh6) { > + write_seqlock(&net->xfrm.policy_hthresh.lock); > + if (thresh4) { > + net->xfrm.policy_hthresh.lbits4 = thresh4->lbits; > + net->xfrm.policy_hthresh.rbits4 = thresh4->rbits; > + } > + if (thresh6) { > + net->xfrm.policy_hthresh.lbits6 = thresh6->lbits; > + net->xfrm.policy_hthresh.rbits6 = thresh6->rbits; > + } > + write_sequnlock(&net->xfrm.policy_hthresh.lock); > + > + xfrm_policy_hash_rebuild(net); > + } > + > + r_skb = nlmsg_new(xfrm_spdinfo_msgsize(), GFP_ATOMIC); > + if (r_skb == NULL) > + return -ENOMEM; > + > + if (build_spdinfo(r_skb, net, sportid, seq, *flags) < 0) > + BUG(); > + > + return nlmsg_unicast(net->xfrm.nlsk, r_skb, sportid); Why do you send these informations to userspace? This is a set operation, not get. The rest looks quite good, 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