Commit f6c7b422 authored by Jakub Kicinski's avatar Jakub Kicinski

Merge tag 'ipsec-2023-10-17' of git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec

Steffen Klassert says:

====================
pull request (net): ipsec 2023-10-17

1) Fix a slab-use-after-free in xfrm_policy_inexact_list_reinsert.
   From Dong Chenchen.

2) Fix data-races in the xfrm interfaces dev->stats fields.
   From Eric Dumazet.

3) Fix a data-race in xfrm_gen_index.
   From Eric Dumazet.

4) Fix an inet6_dev refcount underflow.
   From Zhang Changzhong.

5) Check the return value of pskb_trim in esp_remove_trailer
   for esp4 and esp6. From Ma Ke.

6) Fix a data-race in xfrm_lookup_with_ifid.
   From Eric Dumazet.

* tag 'ipsec-2023-10-17' of git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec:
  xfrm: fix a data-race in xfrm_lookup_with_ifid()
  net: ipv4: fix return value check in esp_remove_trailer
  net: ipv6: fix return value check in esp_remove_trailer
  xfrm6: fix inet6_dev refcount underflow problem
  xfrm: fix a data-race in xfrm_gen_index()
  xfrm: interface: use DEV_STATS_INC()
  net: xfrm: skip policies marked as dead while reinserting policies
====================

Link: https://lore.kernel.org/r/20231017083723.1364940-1-steffen.klassert@secunet.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents c53647a5 de5724ca
...@@ -50,6 +50,7 @@ struct netns_xfrm { ...@@ -50,6 +50,7 @@ struct netns_xfrm {
struct list_head policy_all; struct list_head policy_all;
struct hlist_head *policy_byidx; struct hlist_head *policy_byidx;
unsigned int policy_idx_hmask; unsigned int policy_idx_hmask;
unsigned int idx_generator;
struct hlist_head policy_inexact[XFRM_POLICY_MAX]; struct hlist_head policy_inexact[XFRM_POLICY_MAX];
struct xfrm_policy_hash policy_bydst[XFRM_POLICY_MAX]; struct xfrm_policy_hash policy_bydst[XFRM_POLICY_MAX];
unsigned int policy_count[XFRM_POLICY_MAX * 2]; unsigned int policy_count[XFRM_POLICY_MAX * 2];
......
...@@ -732,7 +732,9 @@ static inline int esp_remove_trailer(struct sk_buff *skb) ...@@ -732,7 +732,9 @@ static inline int esp_remove_trailer(struct sk_buff *skb)
skb->csum = csum_block_sub(skb->csum, csumdiff, skb->csum = csum_block_sub(skb->csum, csumdiff,
skb->len - trimlen); skb->len - trimlen);
} }
pskb_trim(skb, skb->len - trimlen); ret = pskb_trim(skb, skb->len - trimlen);
if (unlikely(ret))
return ret;
ret = nexthdr[1]; ret = nexthdr[1];
......
...@@ -770,7 +770,9 @@ static inline int esp_remove_trailer(struct sk_buff *skb) ...@@ -770,7 +770,9 @@ static inline int esp_remove_trailer(struct sk_buff *skb)
skb->csum = csum_block_sub(skb->csum, csumdiff, skb->csum = csum_block_sub(skb->csum, csumdiff,
skb->len - trimlen); skb->len - trimlen);
} }
pskb_trim(skb, skb->len - trimlen); ret = pskb_trim(skb, skb->len - trimlen);
if (unlikely(ret))
return ret;
ret = nexthdr[1]; ret = nexthdr[1];
......
...@@ -117,10 +117,10 @@ static void xfrm6_dst_destroy(struct dst_entry *dst) ...@@ -117,10 +117,10 @@ static void xfrm6_dst_destroy(struct dst_entry *dst)
{ {
struct xfrm_dst *xdst = (struct xfrm_dst *)dst; struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
if (likely(xdst->u.rt6.rt6i_idev))
in6_dev_put(xdst->u.rt6.rt6i_idev);
dst_destroy_metrics_generic(dst); dst_destroy_metrics_generic(dst);
rt6_uncached_list_del(&xdst->u.rt6); rt6_uncached_list_del(&xdst->u.rt6);
if (likely(xdst->u.rt6.rt6i_idev))
in6_dev_put(xdst->u.rt6.rt6i_idev);
xfrm_dst_destroy(xdst); xfrm_dst_destroy(xdst);
} }
......
...@@ -380,8 +380,8 @@ static int xfrmi_rcv_cb(struct sk_buff *skb, int err) ...@@ -380,8 +380,8 @@ static int xfrmi_rcv_cb(struct sk_buff *skb, int err)
skb->dev = dev; skb->dev = dev;
if (err) { if (err) {
dev->stats.rx_errors++; DEV_STATS_INC(dev, rx_errors);
dev->stats.rx_dropped++; DEV_STATS_INC(dev, rx_dropped);
return 0; return 0;
} }
...@@ -426,7 +426,6 @@ static int ...@@ -426,7 +426,6 @@ static int
xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl) xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
{ {
struct xfrm_if *xi = netdev_priv(dev); struct xfrm_if *xi = netdev_priv(dev);
struct net_device_stats *stats = &xi->dev->stats;
struct dst_entry *dst = skb_dst(skb); struct dst_entry *dst = skb_dst(skb);
unsigned int length = skb->len; unsigned int length = skb->len;
struct net_device *tdev; struct net_device *tdev;
...@@ -473,7 +472,7 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl) ...@@ -473,7 +472,7 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
tdev = dst->dev; tdev = dst->dev;
if (tdev == dev) { if (tdev == dev) {
stats->collisions++; DEV_STATS_INC(dev, collisions);
net_warn_ratelimited("%s: Local routing loop detected!\n", net_warn_ratelimited("%s: Local routing loop detected!\n",
dev->name); dev->name);
goto tx_err_dst_release; goto tx_err_dst_release;
...@@ -512,13 +511,13 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl) ...@@ -512,13 +511,13 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
if (net_xmit_eval(err) == 0) { if (net_xmit_eval(err) == 0) {
dev_sw_netstats_tx_add(dev, 1, length); dev_sw_netstats_tx_add(dev, 1, length);
} else { } else {
stats->tx_errors++; DEV_STATS_INC(dev, tx_errors);
stats->tx_aborted_errors++; DEV_STATS_INC(dev, tx_aborted_errors);
} }
return 0; return 0;
tx_err_link_failure: tx_err_link_failure:
stats->tx_carrier_errors++; DEV_STATS_INC(dev, tx_carrier_errors);
dst_link_failure(skb); dst_link_failure(skb);
tx_err_dst_release: tx_err_dst_release:
dst_release(dst); dst_release(dst);
...@@ -528,7 +527,6 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl) ...@@ -528,7 +527,6 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev) static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev)
{ {
struct xfrm_if *xi = netdev_priv(dev); struct xfrm_if *xi = netdev_priv(dev);
struct net_device_stats *stats = &xi->dev->stats;
struct dst_entry *dst = skb_dst(skb); struct dst_entry *dst = skb_dst(skb);
struct flowi fl; struct flowi fl;
int ret; int ret;
...@@ -545,7 +543,7 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev) ...@@ -545,7 +543,7 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev)
dst = ip6_route_output(dev_net(dev), NULL, &fl.u.ip6); dst = ip6_route_output(dev_net(dev), NULL, &fl.u.ip6);
if (dst->error) { if (dst->error) {
dst_release(dst); dst_release(dst);
stats->tx_carrier_errors++; DEV_STATS_INC(dev, tx_carrier_errors);
goto tx_err; goto tx_err;
} }
skb_dst_set(skb, dst); skb_dst_set(skb, dst);
...@@ -561,7 +559,7 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev) ...@@ -561,7 +559,7 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev)
fl.u.ip4.flowi4_flags |= FLOWI_FLAG_ANYSRC; fl.u.ip4.flowi4_flags |= FLOWI_FLAG_ANYSRC;
rt = __ip_route_output_key(dev_net(dev), &fl.u.ip4); rt = __ip_route_output_key(dev_net(dev), &fl.u.ip4);
if (IS_ERR(rt)) { if (IS_ERR(rt)) {
stats->tx_carrier_errors++; DEV_STATS_INC(dev, tx_carrier_errors);
goto tx_err; goto tx_err;
} }
skb_dst_set(skb, &rt->dst); skb_dst_set(skb, &rt->dst);
...@@ -580,8 +578,8 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev) ...@@ -580,8 +578,8 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev)
return NETDEV_TX_OK; return NETDEV_TX_OK;
tx_err: tx_err:
stats->tx_errors++; DEV_STATS_INC(dev, tx_errors);
stats->tx_dropped++; DEV_STATS_INC(dev, tx_dropped);
kfree_skb(skb); kfree_skb(skb);
return NETDEV_TX_OK; return NETDEV_TX_OK;
} }
......
...@@ -851,7 +851,7 @@ static void xfrm_policy_inexact_list_reinsert(struct net *net, ...@@ -851,7 +851,7 @@ static void xfrm_policy_inexact_list_reinsert(struct net *net,
struct hlist_node *newpos = NULL; struct hlist_node *newpos = NULL;
bool matches_s, matches_d; bool matches_s, matches_d;
if (!policy->bydst_reinsert) if (policy->walk.dead || !policy->bydst_reinsert)
continue; continue;
WARN_ON_ONCE(policy->family != family); WARN_ON_ONCE(policy->family != family);
...@@ -1256,8 +1256,11 @@ static void xfrm_hash_rebuild(struct work_struct *work) ...@@ -1256,8 +1256,11 @@ static void xfrm_hash_rebuild(struct work_struct *work)
struct xfrm_pol_inexact_bin *bin; struct xfrm_pol_inexact_bin *bin;
u8 dbits, sbits; u8 dbits, sbits;
if (policy->walk.dead)
continue;
dir = xfrm_policy_id2dir(policy->index); dir = xfrm_policy_id2dir(policy->index);
if (policy->walk.dead || dir >= XFRM_POLICY_MAX) if (dir >= XFRM_POLICY_MAX)
continue; continue;
if ((dir & XFRM_POLICY_MASK) == XFRM_POLICY_OUT) { if ((dir & XFRM_POLICY_MASK) == XFRM_POLICY_OUT) {
...@@ -1372,8 +1375,6 @@ EXPORT_SYMBOL(xfrm_policy_hash_rebuild); ...@@ -1372,8 +1375,6 @@ EXPORT_SYMBOL(xfrm_policy_hash_rebuild);
* of an absolute inpredictability of ordering of rules. This will not pass. */ * of an absolute inpredictability of ordering of rules. This will not pass. */
static u32 xfrm_gen_index(struct net *net, int dir, u32 index) static u32 xfrm_gen_index(struct net *net, int dir, u32 index)
{ {
static u32 idx_generator;
for (;;) { for (;;) {
struct hlist_head *list; struct hlist_head *list;
struct xfrm_policy *p; struct xfrm_policy *p;
...@@ -1381,8 +1382,8 @@ static u32 xfrm_gen_index(struct net *net, int dir, u32 index) ...@@ -1381,8 +1382,8 @@ static u32 xfrm_gen_index(struct net *net, int dir, u32 index)
int found; int found;
if (!index) { if (!index) {
idx = (idx_generator | dir); idx = (net->xfrm.idx_generator | dir);
idx_generator += 8; net->xfrm.idx_generator += 8;
} else { } else {
idx = index; idx = index;
index = 0; index = 0;
...@@ -1823,9 +1824,11 @@ int xfrm_policy_flush(struct net *net, u8 type, bool task_valid) ...@@ -1823,9 +1824,11 @@ int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)
again: again:
list_for_each_entry(pol, &net->xfrm.policy_all, walk.all) { list_for_each_entry(pol, &net->xfrm.policy_all, walk.all) {
if (pol->walk.dead)
continue;
dir = xfrm_policy_id2dir(pol->index); dir = xfrm_policy_id2dir(pol->index);
if (pol->walk.dead || if (dir >= XFRM_POLICY_MAX ||
dir >= XFRM_POLICY_MAX ||
pol->type != type) pol->type != type)
continue; continue;
...@@ -1862,9 +1865,11 @@ int xfrm_dev_policy_flush(struct net *net, struct net_device *dev, ...@@ -1862,9 +1865,11 @@ int xfrm_dev_policy_flush(struct net *net, struct net_device *dev,
again: again:
list_for_each_entry(pol, &net->xfrm.policy_all, walk.all) { list_for_each_entry(pol, &net->xfrm.policy_all, walk.all) {
if (pol->walk.dead)
continue;
dir = xfrm_policy_id2dir(pol->index); dir = xfrm_policy_id2dir(pol->index);
if (pol->walk.dead || if (dir >= XFRM_POLICY_MAX ||
dir >= XFRM_POLICY_MAX ||
pol->xdo.dev != dev) pol->xdo.dev != dev)
continue; continue;
...@@ -3215,7 +3220,7 @@ struct dst_entry *xfrm_lookup_with_ifid(struct net *net, ...@@ -3215,7 +3220,7 @@ struct dst_entry *xfrm_lookup_with_ifid(struct net *net,
} }
for (i = 0; i < num_pols; i++) for (i = 0; i < num_pols; i++)
pols[i]->curlft.use_time = ktime_get_real_seconds(); WRITE_ONCE(pols[i]->curlft.use_time, ktime_get_real_seconds());
if (num_xfrms < 0) { if (num_xfrms < 0) {
/* Prohibit the flow */ /* Prohibit the flow */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment