| 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: Mon, 24 Jun 2024 13:27:51 +0300
From: Leon Romanovsky <leon@...nel.org>
To: Steffen Klassert <steffen.klassert@...unet.com>
Cc: Jianbo Liu <jianbol@...dia.com>, Eric Dumazet <edumazet@...gle.com>,
netdev@...r.kernel.org
Subject: Re: [PATCH v2 ipsec] xfrm: Fix unregister netdevice hang on hardware
offload.
On Thu, Jun 20, 2024 at 08:47:24AM +0200, Steffen Klassert wrote:
> When offloading xfrm states to hardware, the offloading
> device is attached to the skbs secpath. If a skb is free
> is deferred, an unregister netdevice hangs because the
> netdevice is still refcounted.
>
> Fix this by removing the netdevice from the xfrm states
> when the netdevice is unregistered. To find all xfrm states
> that need to be cleared we add another list where skbs
> linked to that are unlinked from the lists (deleted)
> but not yet freed.
>
> Changes in v2:
>
> - Fix build with CONFIG_XFRM_OFFLOAD disabled.
> - Fix two typos in the commit message.
Changelog should be after "---" trailer marker.
Thanks
>
> Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API")
> Signed-off-by: Steffen Klassert <steffen.klassert@...unet.com>
> ---
> include/net/xfrm.h | 36 +++++++------------------
> net/xfrm/xfrm_state.c | 61 +++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 69 insertions(+), 28 deletions(-)
>
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index 77ebf5bcf0b9..7d4c2235252c 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -178,7 +178,10 @@ struct xfrm_state {
> struct hlist_node gclist;
> struct hlist_node bydst;
> };
> - struct hlist_node bysrc;
> + union {
> + struct hlist_node dev_gclist;
> + struct hlist_node bysrc;
> + };
> struct hlist_node byspi;
> struct hlist_node byseq;
>
> @@ -1588,7 +1591,7 @@ void xfrm_state_update_stats(struct net *net);
> static inline void xfrm_dev_state_update_stats(struct xfrm_state *x)
> {
> struct xfrm_dev_offload *xdo = &x->xso;
> - struct net_device *dev = xdo->dev;
> + struct net_device *dev = READ_ONCE(xdo->dev);
>
> if (dev && dev->xfrmdev_ops &&
> dev->xfrmdev_ops->xdo_dev_state_update_stats)
> @@ -1946,13 +1949,16 @@ int xfrm_dev_policy_add(struct net *net, struct xfrm_policy *xp,
> struct xfrm_user_offload *xuo, u8 dir,
> struct netlink_ext_ack *extack);
> bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x);
> +void xfrm_dev_state_delete(struct xfrm_state *x);
> +void xfrm_dev_state_free(struct xfrm_state *x);
>
> static inline void xfrm_dev_state_advance_esn(struct xfrm_state *x)
> {
> struct xfrm_dev_offload *xso = &x->xso;
> + struct net_device *dev = READ_ONCE(xso->dev);
>
> - if (xso->dev && xso->dev->xfrmdev_ops->xdo_dev_state_advance_esn)
> - xso->dev->xfrmdev_ops->xdo_dev_state_advance_esn(x);
> + if (dev && dev->xfrmdev_ops->xdo_dev_state_advance_esn)
> + dev->xfrmdev_ops->xdo_dev_state_advance_esn(x);
> }
>
> static inline bool xfrm_dst_offload_ok(struct dst_entry *dst)
> @@ -1973,28 +1979,6 @@ static inline bool xfrm_dst_offload_ok(struct dst_entry *dst)
> return false;
> }
>
> -static inline void xfrm_dev_state_delete(struct xfrm_state *x)
> -{
> - struct xfrm_dev_offload *xso = &x->xso;
> -
> - if (xso->dev)
> - xso->dev->xfrmdev_ops->xdo_dev_state_delete(x);
> -}
> -
> -static inline void xfrm_dev_state_free(struct xfrm_state *x)
> -{
> - struct xfrm_dev_offload *xso = &x->xso;
> - struct net_device *dev = xso->dev;
> -
> - if (dev && dev->xfrmdev_ops) {
> - if (dev->xfrmdev_ops->xdo_dev_state_free)
> - dev->xfrmdev_ops->xdo_dev_state_free(x);
> - xso->dev = NULL;
> - xso->type = XFRM_DEV_OFFLOAD_UNSPECIFIED;
> - netdev_put(dev, &xso->dev_tracker);
> - }
> -}
> -
> static inline void xfrm_dev_policy_delete(struct xfrm_policy *x)
> {
> struct xfrm_dev_offload *xdo = &x->xdo;
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index 649bb739df0d..d531d2a1fae2 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -49,6 +49,7 @@ static struct kmem_cache *xfrm_state_cache __ro_after_init;
>
> static DECLARE_WORK(xfrm_state_gc_work, xfrm_state_gc_task);
> static HLIST_HEAD(xfrm_state_gc_list);
> +static HLIST_HEAD(xfrm_state_dev_gc_list);
>
> static inline bool xfrm_state_hold_rcu(struct xfrm_state __rcu *x)
> {
> @@ -214,6 +215,7 @@ static DEFINE_SPINLOCK(xfrm_state_afinfo_lock);
> static struct xfrm_state_afinfo __rcu *xfrm_state_afinfo[NPROTO];
>
> static DEFINE_SPINLOCK(xfrm_state_gc_lock);
> +static DEFINE_SPINLOCK(xfrm_state_dev_gc_lock);
>
> int __xfrm_state_delete(struct xfrm_state *x);
>
> @@ -683,6 +685,40 @@ struct xfrm_state *xfrm_state_alloc(struct net *net)
> }
> EXPORT_SYMBOL(xfrm_state_alloc);
>
> +#ifdef CONFIG_XFRM_OFFLOAD
> +void xfrm_dev_state_delete(struct xfrm_state *x)
> +{
> + struct xfrm_dev_offload *xso = &x->xso;
> + struct net_device *dev = READ_ONCE(xso->dev);
> +
> + if (dev) {
> + dev->xfrmdev_ops->xdo_dev_state_delete(x);
> + spin_lock_bh(&xfrm_state_dev_gc_lock);
> + hlist_add_head(&x->dev_gclist, &xfrm_state_dev_gc_list);
> + spin_unlock_bh(&xfrm_state_dev_gc_lock);
> + }
> +}
> +
> +void xfrm_dev_state_free(struct xfrm_state *x)
> +{
> + struct xfrm_dev_offload *xso = &x->xso;
> + struct net_device *dev = READ_ONCE(xso->dev);
> +
> + if (dev && dev->xfrmdev_ops) {
> + spin_lock_bh(&xfrm_state_dev_gc_lock);
> + if (!hlist_unhashed(&x->dev_gclist))
> + hlist_del(&x->dev_gclist);
> + spin_unlock_bh(&xfrm_state_dev_gc_lock);
> +
> + if (dev->xfrmdev_ops->xdo_dev_state_free)
> + dev->xfrmdev_ops->xdo_dev_state_free(x);
> + WRITE_ONCE(xso->dev, NULL);
> + xso->type = XFRM_DEV_OFFLOAD_UNSPECIFIED;
> + netdev_put(dev, &xso->dev_tracker);
> + }
> +}
> +#endif
> +
> void __xfrm_state_destroy(struct xfrm_state *x, bool sync)
> {
> WARN_ON(x->km.state != XFRM_STATE_DEAD);
> @@ -848,6 +884,9 @@ EXPORT_SYMBOL(xfrm_state_flush);
>
> int xfrm_dev_state_flush(struct net *net, struct net_device *dev, bool task_valid)
> {
> + struct xfrm_state *x;
> + struct hlist_node *tmp;
> + struct xfrm_dev_offload *xso;
> int i, err = 0, cnt = 0;
>
> spin_lock_bh(&net->xfrm.xfrm_state_lock);
> @@ -857,8 +896,6 @@ int xfrm_dev_state_flush(struct net *net, struct net_device *dev, bool task_vali
>
> err = -ESRCH;
> for (i = 0; i <= net->xfrm.state_hmask; i++) {
> - struct xfrm_state *x;
> - struct xfrm_dev_offload *xso;
> restart:
> hlist_for_each_entry(x, net->xfrm.state_bydst+i, bydst) {
> xso = &x->xso;
> @@ -868,6 +905,8 @@ int xfrm_dev_state_flush(struct net *net, struct net_device *dev, bool task_vali
> spin_unlock_bh(&net->xfrm.xfrm_state_lock);
>
> err = xfrm_state_delete(x);
> + xfrm_dev_state_free(x);
> +
> xfrm_audit_state_delete(x, err ? 0 : 1,
> task_valid);
> xfrm_state_put(x);
> @@ -884,6 +923,24 @@ int xfrm_dev_state_flush(struct net *net, struct net_device *dev, bool task_vali
>
> out:
> spin_unlock_bh(&net->xfrm.xfrm_state_lock);
> +
> + spin_lock_bh(&xfrm_state_dev_gc_lock);
> +restart_gc:
> + hlist_for_each_entry_safe(x, tmp, &xfrm_state_dev_gc_list, dev_gclist) {
> + xso = &x->xso;
> +
> + if (xso->dev == dev) {
> + spin_unlock_bh(&xfrm_state_dev_gc_lock);
> + xfrm_dev_state_free(x);
> + spin_lock_bh(&xfrm_state_dev_gc_lock);
> + goto restart_gc;
> + }
> +
> + }
> + spin_unlock_bh(&xfrm_state_dev_gc_lock);
> +
> + xfrm_flush_gc();
> +
> return err;
> }
> EXPORT_SYMBOL(xfrm_dev_state_flush);
> --
> 2.34.1
>
>
Powered by blists - more mailing lists