Hi Leon,
On Wed, Feb 5, 2025 at 11:50 PM Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>
> From: Leon Romanovsky <leonro@xxxxxxxxxx>
>
> XFRM offload path is probed even if offload isn't needed at all. Let's
> make sure that x->type_offload pointer stays NULL for such path to
> reduce ambiguity.
>
> Fixes: 9d389d7f84bb ("xfrm: Add a xfrm type offload.")
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxx>
> ---
> include/net/xfrm.h | 12 +++++++++++-
> net/xfrm/xfrm_device.c | 14 +++++++++-----
> net/xfrm/xfrm_state.c | 22 +++++++++-------------
> net/xfrm/xfrm_user.c | 2 +-
> 4 files changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index ed4b83696c77..28355a5be5b9 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -464,6 +464,16 @@ struct xfrm_type_offload {
>
> int xfrm_register_type_offload(const struct xfrm_type_offload *type, unsigned short family);
> void xfrm_unregister_type_offload(const struct xfrm_type_offload *type, unsigned short family);
> +const struct xfrm_type_offload *xfrm_get_type_offload(u8 proto,
> + unsigned short family);
> +static inline void xfrm_put_type_offload(const struct xfrm_type_offload *type)
> +{
> + if (!type)
> + return;
> +
> + module_put(type->owner);
> +}
> +
>
> /**
> * struct xfrm_mode_cbs - XFRM mode callbacks
> @@ -1760,7 +1770,7 @@ void xfrm_spd_getinfo(struct net *net, struct xfrmk_spdinfo *si);
> u32 xfrm_replay_seqhi(struct xfrm_state *x, __be32 net_seq);
> int xfrm_init_replay(struct xfrm_state *x, struct netlink_ext_ack *extack);
> u32 xfrm_state_mtu(struct xfrm_state *x, int mtu);
> -int __xfrm_init_state(struct xfrm_state *x, bool init_replay, bool offload,
> +int __xfrm_init_state(struct xfrm_state *x, bool init_replay,
> struct netlink_ext_ack *extack);
> int xfrm_init_state(struct xfrm_state *x);
> int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type);
> diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
> index d1fa94e52cea..e01a7f5a4c75 100644
> --- a/net/xfrm/xfrm_device.c
> +++ b/net/xfrm/xfrm_device.c
> @@ -244,11 +244,6 @@ int xfrm_dev_state_add(struct net *net, struct xfrm_state *x,
> xfrm_address_t *daddr;
> bool is_packet_offload;
>
> - if (!x->type_offload) {
> - NL_SET_ERR_MSG(extack, "Type doesn't support offload");
> - return -EINVAL;
> - }
> -
> if (xuo->flags &
> ~(XFRM_OFFLOAD_IPV6 | XFRM_OFFLOAD_INBOUND | XFRM_OFFLOAD_PACKET)) {
> NL_SET_ERR_MSG(extack, "Unrecognized flags in offload request");
> @@ -310,6 +305,13 @@ int xfrm_dev_state_add(struct net *net, struct xfrm_state *x,
> return -EINVAL;
> }
>
> + x->type_offload = xfrm_get_type_offload(x->id.proto, x->props.family);
> + if (!x->type_offload) {
> + NL_SET_ERR_MSG(extack, "Type doesn't support offload");
> + dev_put(dev);
> + return -EINVAL;
> + }
> +
> xso->dev = dev;
> netdev_tracker_alloc(dev, &xso->dev_tracker, GFP_ATOMIC);
> xso->real_dev = dev;
> @@ -332,6 +334,8 @@ int xfrm_dev_state_add(struct net *net, struct xfrm_state *x,
> netdev_put(dev, &xso->dev_tracker);
> xso->type = XFRM_DEV_OFFLOAD_UNSPECIFIED;
>
> + xfrm_put_type_offload(x->type_offload);
> + x->type_offload = NULL;
We always set type_offload to NULL. Can we move type_offload = NULL in
xfrm_put_type_offload() ?
Thanks
-Bharat
> /* User explicitly requested packet offload mode and configured
> * policy in addition to the XFRM state. So be civil to users,
> * and return an error instead of taking fallback path.
> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index ad2202fa82f3..568fe8df7741 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -424,11 +424,12 @@ void xfrm_unregister_type_offload(const struct xfrm_type_offload *type,
> }
> EXPORT_SYMBOL(xfrm_unregister_type_offload);
>
> -static const struct xfrm_type_offload *
> -xfrm_get_type_offload(u8 proto, unsigned short family, bool try_load)
> +const struct xfrm_type_offload *xfrm_get_type_offload(u8 proto,
> + unsigned short family)
> {
> const struct xfrm_type_offload *type = NULL;
> struct xfrm_state_afinfo *afinfo;
> + bool try_load = true;
>
> retry:
> afinfo = xfrm_state_get_afinfo(family);
> @@ -456,11 +457,7 @@ xfrm_get_type_offload(u8 proto, unsigned short family, bool try_load)
>
> return type;
> }
> -
> -static void xfrm_put_type_offload(const struct xfrm_type_offload *type)
> -{
> - module_put(type->owner);
> -}
> +EXPORT_SYMBOL(xfrm_get_type_offload);
>
> static const struct xfrm_mode xfrm4_mode_map[XFRM_MODE_MAX] = {
> [XFRM_MODE_BEET] = {
> @@ -609,8 +606,6 @@ static void ___xfrm_state_destroy(struct xfrm_state *x)
> kfree(x->coaddr);
> kfree(x->replay_esn);
> kfree(x->preplay_esn);
> - if (x->type_offload)
> - xfrm_put_type_offload(x->type_offload);
> if (x->type) {
> x->type->destructor(x);
> xfrm_put_type(x->type);
> @@ -784,6 +779,9 @@ void xfrm_dev_state_free(struct xfrm_state *x)
> struct xfrm_dev_offload *xso = &x->xso;
> struct net_device *dev = READ_ONCE(xso->dev);
>
> + xfrm_put_type_offload(x->type_offload);
> + x->type_offload = NULL;
> +
> if (dev && dev->xfrmdev_ops) {
> spin_lock_bh(&xfrm_state_dev_gc_lock);
> if (!hlist_unhashed(&x->dev_gclist))
> @@ -3122,7 +3120,7 @@ u32 xfrm_state_mtu(struct xfrm_state *x, int mtu)
> }
> EXPORT_SYMBOL_GPL(xfrm_state_mtu);
>
> -int __xfrm_init_state(struct xfrm_state *x, bool init_replay, bool offload,
> +int __xfrm_init_state(struct xfrm_state *x, bool init_replay,
> struct netlink_ext_ack *extack)
> {
> const struct xfrm_mode *inner_mode;
> @@ -3178,8 +3176,6 @@ int __xfrm_init_state(struct xfrm_state *x, bool init_replay, bool offload,
> goto error;
> }
>
> - x->type_offload = xfrm_get_type_offload(x->id.proto, family, offload);
> -
> err = x->type->init_state(x, extack);
> if (err)
> goto error;
> @@ -3229,7 +3225,7 @@ int xfrm_init_state(struct xfrm_state *x)
> {
> int err;
>
> - err = __xfrm_init_state(x, true, false, NULL);
> + err = __xfrm_init_state(x, true, NULL);
> if (!err)
> x->km.state = XFRM_STATE_VALID;
>
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index 08c6d6f0179f..82a768500999 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -907,7 +907,7 @@ static struct xfrm_state *xfrm_state_construct(struct net *net,
> goto error;
> }
>
> - err = __xfrm_init_state(x, false, attrs[XFRMA_OFFLOAD_DEV], extack);
> + err = __xfrm_init_state(x, false, extack);
> if (err)
> goto error;
>
> --
> 2.48.1
>
>
