August 2022 - Linux-stable-mirror

FAILED: patch "[PATCH] net/tls: Use RCU API to access tls_ctx->netdev" failed to apply to 5.15-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 5.15-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 94ce3b64c62d4b628cf85cd0d9a370aca8f7e43a Mon Sep 17 00:00:00 2001 From: Maxim Mikityanskiy <maximmi(a)nvidia.com> Date: Wed, 10 Aug 2022 11:16:02 +0300 Subject: [PATCH] net/tls: Use RCU API to access tls_ctx->netdev Currently, tls_device_down synchronizes with tls_device_resync_rx using RCU, however, the pointer to netdev is stored using WRITE_ONCE and loaded using READ_ONCE. Although such approach is technically correct (rcu_dereference is essentially a READ_ONCE, and rcu_assign_pointer uses WRITE_ONCE to store NULL), using special RCU helpers for pointers is more valid, as it includes additional checks and might change the implementation transparently to the callers. Mark the netdev pointer as __rcu and use the correct RCU helpers to access it. For non-concurrent access pass the right conditions that guarantee safe access (locks taken, refcount value). Also use the correct helper in mlx5e, where even READ_ONCE was missing. The transition to RCU exposes existing issues, fixed by this commit: 1. bond_tls_device_xmit could read netdev twice, and it could become NULL the second time, after the NULL check passed. 2. Drivers shouldn't stop processing the last packet if tls_device_down just set netdev to NULL, before tls_dev_del was called. This prevents a possible packet drop when transitioning to the fallback software mode. Fixes: 89df6a810470 ("net/bonding: Implement TLS TX device offload") Fixes: c55dcdd435aa ("net/tls: Fix use-after-free after the TLS device goes down and up") Signed-off-by: Maxim Mikityanskiy <maximmi(a)nvidia.com> Link: https://lore.kernel.org/r/20220810081602.1435800-1-maximmi@nvidia.com Signed-off-by: Jakub Kicinski <kuba(a)kernel.org> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index ab7fdbbc2530..50e60843020c 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -5338,8 +5338,14 @@ static struct net_device *bond_sk_get_lower_dev(struct net_device *dev, static netdev_tx_t bond_tls_device_xmit(struct bonding *bond, struct sk_buff *skb, struct net_device *dev) { - if (likely(bond_get_slave_by_dev(bond, tls_get_ctx(skb->sk)->netdev))) - return bond_dev_queue_xmit(bond, skb, tls_get_ctx(skb->sk)->netdev); + struct net_device *tls_netdev = rcu_dereference(tls_get_ctx(skb->sk)->netdev); + + /* tls_netdev might become NULL, even if tls_is_sk_tx_device_offloaded + * was true, if tls_device_down is running in parallel, but it's OK, + * because bond_get_slave_by_dev has a NULL check. + */ + if (likely(bond_get_slave_by_dev(bond, tls_netdev))) + return bond_dev_queue_xmit(bond, skb, tls_netdev); return bond_tx_drop(dev, skb); } #endif diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c index bfee0e4e54b1..da9973b711f4 100644 --- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c +++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c @@ -1932,6 +1932,7 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev) int data_len, qidx, ret = 0, mss; struct tls_record_info *record; struct chcr_ktls_info *tx_info; + struct net_device *tls_netdev; struct tls_context *tls_ctx; struct sge_eth_txq *q; struct adapter *adap; @@ -1945,7 +1946,12 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev) mss = skb_is_gso(skb) ? skb_shinfo(skb)->gso_size : data_len; tls_ctx = tls_get_ctx(skb->sk); - if (unlikely(tls_ctx->netdev != dev)) + tls_netdev = rcu_dereference_bh(tls_ctx->netdev); + /* Don't quit on NULL: if tls_device_down is running in parallel, + * netdev might become NULL, even if tls_is_sk_tx_device_offloaded was + * true. Rather continue processing this packet. + */ + if (unlikely(tls_netdev && tls_netdev != dev)) goto out; tx_ctx = chcr_get_ktls_tx_context(tls_ctx); diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c index 6b6c7044b64a..0aef69527226 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c @@ -808,6 +808,7 @@ bool mlx5e_ktls_handle_tx_skb(struct net_device *netdev, struct mlx5e_txqsq *sq, { struct mlx5e_ktls_offload_context_tx *priv_tx; struct mlx5e_sq_stats *stats = sq->stats; + struct net_device *tls_netdev; struct tls_context *tls_ctx; int datalen; u32 seq; @@ -819,7 +820,12 @@ bool mlx5e_ktls_handle_tx_skb(struct net_device *netdev, struct mlx5e_txqsq *sq, mlx5e_tx_mpwqe_ensure_complete(sq); tls_ctx = tls_get_ctx(skb->sk); - if (WARN_ON_ONCE(tls_ctx->netdev != netdev)) + tls_netdev = rcu_dereference_bh(tls_ctx->netdev); + /* Don't WARN on NULL: if tls_device_down is running in parallel, + * netdev might become NULL, even if tls_is_sk_tx_device_offloaded was + * true. Rather continue processing this packet. + */ + if (WARN_ON_ONCE(tls_netdev && tls_netdev != netdev)) goto err_out; priv_tx = mlx5e_get_ktls_tx_priv_ctx(tls_ctx); diff --git a/include/net/tls.h b/include/net/tls.h index b75b5727abdb..cb205f9d9473 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -237,7 +237,7 @@ struct tls_context { void *priv_ctx_tx; void *priv_ctx_rx; - struct net_device *netdev; + struct net_device __rcu *netdev; /* rw cache line */ struct cipher_context tx; diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c index 6ed41474bdf8..0f983e5f7dde 100644 --- a/net/tls/tls_device.c +++ b/net/tls/tls_device.c @@ -71,7 +71,13 @@ static void tls_device_tx_del_task(struct work_struct *work) struct tls_offload_context_tx *offload_ctx = container_of(work, struct tls_offload_context_tx, destruct_work); struct tls_context *ctx = offload_ctx->ctx; - struct net_device *netdev = ctx->netdev; + struct net_device *netdev; + + /* Safe, because this is the destroy flow, refcount is 0, so + * tls_device_down can't store this field in parallel. + */ + netdev = rcu_dereference_protected(ctx->netdev, + !refcount_read(&ctx->refcount)); netdev->tlsdev_ops->tls_dev_del(netdev, ctx, TLS_OFFLOAD_CTX_DIR_TX); dev_put(netdev); @@ -81,6 +87,7 @@ static void tls_device_tx_del_task(struct work_struct *work) static void tls_device_queue_ctx_destruction(struct tls_context *ctx) { + struct net_device *netdev; unsigned long flags; bool async_cleanup; @@ -91,7 +98,14 @@ static void tls_device_queue_ctx_destruction(struct tls_context *ctx) } list_del(&ctx->list); /* Remove from tls_device_list / tls_device_down_list */ - async_cleanup = ctx->netdev && ctx->tx_conf == TLS_HW; + + /* Safe, because this is the destroy flow, refcount is 0, so + * tls_device_down can't store this field in parallel. + */ + netdev = rcu_dereference_protected(ctx->netdev, + !refcount_read(&ctx->refcount)); + + async_cleanup = netdev && ctx->tx_conf == TLS_HW; if (async_cleanup) { struct tls_offload_context_tx *offload_ctx = tls_offload_ctx_tx(ctx); @@ -229,7 +243,8 @@ static void tls_device_resync_tx(struct sock *sk, struct tls_context *tls_ctx, trace_tls_device_tx_resync_send(sk, seq, rcd_sn); down_read(&device_offload_lock); - netdev = tls_ctx->netdev; + netdev = rcu_dereference_protected(tls_ctx->netdev, + lockdep_is_held(&device_offload_lock)); if (netdev) err = netdev->tlsdev_ops->tls_dev_resync(netdev, sk, seq, rcd_sn, @@ -710,7 +725,7 @@ static void tls_device_resync_rx(struct tls_context *tls_ctx, trace_tls_device_rx_resync_send(sk, seq, rcd_sn, rx_ctx->resync_type); rcu_read_lock(); - netdev = READ_ONCE(tls_ctx->netdev); + netdev = rcu_dereference(tls_ctx->netdev); if (netdev) netdev->tlsdev_ops->tls_dev_resync(netdev, sk, seq, rcd_sn, TLS_OFFLOAD_CTX_DIR_RX); @@ -1035,7 +1050,7 @@ static void tls_device_attach(struct tls_context *ctx, struct sock *sk, if (sk->sk_destruct != tls_device_sk_destruct) { refcount_set(&ctx->refcount, 1); dev_hold(netdev); - ctx->netdev = netdev; + RCU_INIT_POINTER(ctx->netdev, netdev); spin_lock_irq(&tls_device_lock); list_add_tail(&ctx->list, &tls_device_list); spin_unlock_irq(&tls_device_lock); @@ -1306,7 +1321,8 @@ void tls_device_offload_cleanup_rx(struct sock *sk) struct net_device *netdev; down_read(&device_offload_lock); - netdev = tls_ctx->netdev; + netdev = rcu_dereference_protected(tls_ctx->netdev, + lockdep_is_held(&device_offload_lock)); if (!netdev) goto out; @@ -1315,7 +1331,7 @@ void tls_device_offload_cleanup_rx(struct sock *sk) if (tls_ctx->tx_conf != TLS_HW) { dev_put(netdev); - tls_ctx->netdev = NULL; + rcu_assign_pointer(tls_ctx->netdev, NULL); } else { set_bit(TLS_RX_DEV_CLOSED, &tls_ctx->flags); } @@ -1335,7 +1351,11 @@ static int tls_device_down(struct net_device *netdev) spin_lock_irqsave(&tls_device_lock, flags); list_for_each_entry_safe(ctx, tmp, &tls_device_list, list) { - if (ctx->netdev != netdev || + struct net_device *ctx_netdev = + rcu_dereference_protected(ctx->netdev, + lockdep_is_held(&device_offload_lock)); + + if (ctx_netdev != netdev || !refcount_inc_not_zero(&ctx->refcount)) continue; @@ -1352,7 +1372,7 @@ static int tls_device_down(struct net_device *netdev) /* Stop the RX and TX resync. * tls_dev_resync must not be called after tls_dev_del. */ - WRITE_ONCE(ctx->netdev, NULL); + rcu_assign_pointer(ctx->netdev, NULL); /* Start skipping the RX resync logic completely. */ set_bit(TLS_RX_DEV_DEGRADED, &ctx->flags); diff --git a/net/tls/tls_device_fallback.c b/net/tls/tls_device_fallback.c index 618cee704217..7dfc8023e0f1 100644 --- a/net/tls/tls_device_fallback.c +++ b/net/tls/tls_device_fallback.c @@ -426,7 +426,8 @@ struct sk_buff *tls_validate_xmit_skb(struct sock *sk, struct net_device *dev, struct sk_buff *skb) { - if (dev == tls_get_ctx(sk)->netdev || netif_is_bond_master(dev)) + if (dev == rcu_dereference_bh(tls_get_ctx(sk)->netdev) || + netif_is_bond_master(dev)) return skb; return tls_sw_fallback(sk, skb);

2 years, 5 months

1
0
0 0

FAILED: patch "[PATCH] net/tls: Use RCU API to access tls_ctx->netdev" failed to apply to 5.19-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 5.19-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 94ce3b64c62d4b628cf85cd0d9a370aca8f7e43a Mon Sep 17 00:00:00 2001 From: Maxim Mikityanskiy <maximmi(a)nvidia.com> Date: Wed, 10 Aug 2022 11:16:02 +0300 Subject: [PATCH] net/tls: Use RCU API to access tls_ctx->netdev Currently, tls_device_down synchronizes with tls_device_resync_rx using RCU, however, the pointer to netdev is stored using WRITE_ONCE and loaded using READ_ONCE. Although such approach is technically correct (rcu_dereference is essentially a READ_ONCE, and rcu_assign_pointer uses WRITE_ONCE to store NULL), using special RCU helpers for pointers is more valid, as it includes additional checks and might change the implementation transparently to the callers. Mark the netdev pointer as __rcu and use the correct RCU helpers to access it. For non-concurrent access pass the right conditions that guarantee safe access (locks taken, refcount value). Also use the correct helper in mlx5e, where even READ_ONCE was missing. The transition to RCU exposes existing issues, fixed by this commit: 1. bond_tls_device_xmit could read netdev twice, and it could become NULL the second time, after the NULL check passed. 2. Drivers shouldn't stop processing the last packet if tls_device_down just set netdev to NULL, before tls_dev_del was called. This prevents a possible packet drop when transitioning to the fallback software mode. Fixes: 89df6a810470 ("net/bonding: Implement TLS TX device offload") Fixes: c55dcdd435aa ("net/tls: Fix use-after-free after the TLS device goes down and up") Signed-off-by: Maxim Mikityanskiy <maximmi(a)nvidia.com> Link: https://lore.kernel.org/r/20220810081602.1435800-1-maximmi@nvidia.com Signed-off-by: Jakub Kicinski <kuba(a)kernel.org> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index ab7fdbbc2530..50e60843020c 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -5338,8 +5338,14 @@ static struct net_device *bond_sk_get_lower_dev(struct net_device *dev, static netdev_tx_t bond_tls_device_xmit(struct bonding *bond, struct sk_buff *skb, struct net_device *dev) { - if (likely(bond_get_slave_by_dev(bond, tls_get_ctx(skb->sk)->netdev))) - return bond_dev_queue_xmit(bond, skb, tls_get_ctx(skb->sk)->netdev); + struct net_device *tls_netdev = rcu_dereference(tls_get_ctx(skb->sk)->netdev); + + /* tls_netdev might become NULL, even if tls_is_sk_tx_device_offloaded + * was true, if tls_device_down is running in parallel, but it's OK, + * because bond_get_slave_by_dev has a NULL check. + */ + if (likely(bond_get_slave_by_dev(bond, tls_netdev))) + return bond_dev_queue_xmit(bond, skb, tls_netdev); return bond_tx_drop(dev, skb); } #endif diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c index bfee0e4e54b1..da9973b711f4 100644 --- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c +++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c @@ -1932,6 +1932,7 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev) int data_len, qidx, ret = 0, mss; struct tls_record_info *record; struct chcr_ktls_info *tx_info; + struct net_device *tls_netdev; struct tls_context *tls_ctx; struct sge_eth_txq *q; struct adapter *adap; @@ -1945,7 +1946,12 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev) mss = skb_is_gso(skb) ? skb_shinfo(skb)->gso_size : data_len; tls_ctx = tls_get_ctx(skb->sk); - if (unlikely(tls_ctx->netdev != dev)) + tls_netdev = rcu_dereference_bh(tls_ctx->netdev); + /* Don't quit on NULL: if tls_device_down is running in parallel, + * netdev might become NULL, even if tls_is_sk_tx_device_offloaded was + * true. Rather continue processing this packet. + */ + if (unlikely(tls_netdev && tls_netdev != dev)) goto out; tx_ctx = chcr_get_ktls_tx_context(tls_ctx); diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c index 6b6c7044b64a..0aef69527226 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c @@ -808,6 +808,7 @@ bool mlx5e_ktls_handle_tx_skb(struct net_device *netdev, struct mlx5e_txqsq *sq, { struct mlx5e_ktls_offload_context_tx *priv_tx; struct mlx5e_sq_stats *stats = sq->stats; + struct net_device *tls_netdev; struct tls_context *tls_ctx; int datalen; u32 seq; @@ -819,7 +820,12 @@ bool mlx5e_ktls_handle_tx_skb(struct net_device *netdev, struct mlx5e_txqsq *sq, mlx5e_tx_mpwqe_ensure_complete(sq); tls_ctx = tls_get_ctx(skb->sk); - if (WARN_ON_ONCE(tls_ctx->netdev != netdev)) + tls_netdev = rcu_dereference_bh(tls_ctx->netdev); + /* Don't WARN on NULL: if tls_device_down is running in parallel, + * netdev might become NULL, even if tls_is_sk_tx_device_offloaded was + * true. Rather continue processing this packet. + */ + if (WARN_ON_ONCE(tls_netdev && tls_netdev != netdev)) goto err_out; priv_tx = mlx5e_get_ktls_tx_priv_ctx(tls_ctx); diff --git a/include/net/tls.h b/include/net/tls.h index b75b5727abdb..cb205f9d9473 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -237,7 +237,7 @@ struct tls_context { void *priv_ctx_tx; void *priv_ctx_rx; - struct net_device *netdev; + struct net_device __rcu *netdev; /* rw cache line */ struct cipher_context tx; diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c index 6ed41474bdf8..0f983e5f7dde 100644 --- a/net/tls/tls_device.c +++ b/net/tls/tls_device.c @@ -71,7 +71,13 @@ static void tls_device_tx_del_task(struct work_struct *work) struct tls_offload_context_tx *offload_ctx = container_of(work, struct tls_offload_context_tx, destruct_work); struct tls_context *ctx = offload_ctx->ctx; - struct net_device *netdev = ctx->netdev; + struct net_device *netdev; + + /* Safe, because this is the destroy flow, refcount is 0, so + * tls_device_down can't store this field in parallel. + */ + netdev = rcu_dereference_protected(ctx->netdev, + !refcount_read(&ctx->refcount)); netdev->tlsdev_ops->tls_dev_del(netdev, ctx, TLS_OFFLOAD_CTX_DIR_TX); dev_put(netdev); @@ -81,6 +87,7 @@ static void tls_device_tx_del_task(struct work_struct *work) static void tls_device_queue_ctx_destruction(struct tls_context *ctx) { + struct net_device *netdev; unsigned long flags; bool async_cleanup; @@ -91,7 +98,14 @@ static void tls_device_queue_ctx_destruction(struct tls_context *ctx) } list_del(&ctx->list); /* Remove from tls_device_list / tls_device_down_list */ - async_cleanup = ctx->netdev && ctx->tx_conf == TLS_HW; + + /* Safe, because this is the destroy flow, refcount is 0, so + * tls_device_down can't store this field in parallel. + */ + netdev = rcu_dereference_protected(ctx->netdev, + !refcount_read(&ctx->refcount)); + + async_cleanup = netdev && ctx->tx_conf == TLS_HW; if (async_cleanup) { struct tls_offload_context_tx *offload_ctx = tls_offload_ctx_tx(ctx); @@ -229,7 +243,8 @@ static void tls_device_resync_tx(struct sock *sk, struct tls_context *tls_ctx, trace_tls_device_tx_resync_send(sk, seq, rcd_sn); down_read(&device_offload_lock); - netdev = tls_ctx->netdev; + netdev = rcu_dereference_protected(tls_ctx->netdev, + lockdep_is_held(&device_offload_lock)); if (netdev) err = netdev->tlsdev_ops->tls_dev_resync(netdev, sk, seq, rcd_sn, @@ -710,7 +725,7 @@ static void tls_device_resync_rx(struct tls_context *tls_ctx, trace_tls_device_rx_resync_send(sk, seq, rcd_sn, rx_ctx->resync_type); rcu_read_lock(); - netdev = READ_ONCE(tls_ctx->netdev); + netdev = rcu_dereference(tls_ctx->netdev); if (netdev) netdev->tlsdev_ops->tls_dev_resync(netdev, sk, seq, rcd_sn, TLS_OFFLOAD_CTX_DIR_RX); @@ -1035,7 +1050,7 @@ static void tls_device_attach(struct tls_context *ctx, struct sock *sk, if (sk->sk_destruct != tls_device_sk_destruct) { refcount_set(&ctx->refcount, 1); dev_hold(netdev); - ctx->netdev = netdev; + RCU_INIT_POINTER(ctx->netdev, netdev); spin_lock_irq(&tls_device_lock); list_add_tail(&ctx->list, &tls_device_list); spin_unlock_irq(&tls_device_lock); @@ -1306,7 +1321,8 @@ void tls_device_offload_cleanup_rx(struct sock *sk) struct net_device *netdev; down_read(&device_offload_lock); - netdev = tls_ctx->netdev; + netdev = rcu_dereference_protected(tls_ctx->netdev, + lockdep_is_held(&device_offload_lock)); if (!netdev) goto out; @@ -1315,7 +1331,7 @@ void tls_device_offload_cleanup_rx(struct sock *sk) if (tls_ctx->tx_conf != TLS_HW) { dev_put(netdev); - tls_ctx->netdev = NULL; + rcu_assign_pointer(tls_ctx->netdev, NULL); } else { set_bit(TLS_RX_DEV_CLOSED, &tls_ctx->flags); } @@ -1335,7 +1351,11 @@ static int tls_device_down(struct net_device *netdev) spin_lock_irqsave(&tls_device_lock, flags); list_for_each_entry_safe(ctx, tmp, &tls_device_list, list) { - if (ctx->netdev != netdev || + struct net_device *ctx_netdev = + rcu_dereference_protected(ctx->netdev, + lockdep_is_held(&device_offload_lock)); + + if (ctx_netdev != netdev || !refcount_inc_not_zero(&ctx->refcount)) continue; @@ -1352,7 +1372,7 @@ static int tls_device_down(struct net_device *netdev) /* Stop the RX and TX resync. * tls_dev_resync must not be called after tls_dev_del. */ - WRITE_ONCE(ctx->netdev, NULL); + rcu_assign_pointer(ctx->netdev, NULL); /* Start skipping the RX resync logic completely. */ set_bit(TLS_RX_DEV_DEGRADED, &ctx->flags); diff --git a/net/tls/tls_device_fallback.c b/net/tls/tls_device_fallback.c index 618cee704217..7dfc8023e0f1 100644 --- a/net/tls/tls_device_fallback.c +++ b/net/tls/tls_device_fallback.c @@ -426,7 +426,8 @@ struct sk_buff *tls_validate_xmit_skb(struct sock *sk, struct net_device *dev, struct sk_buff *skb) { - if (dev == tls_get_ctx(sk)->netdev || netif_is_bond_master(dev)) + if (dev == rcu_dereference_bh(tls_get_ctx(sk)->netdev) || + netif_is_bond_master(dev)) return skb; return tls_sw_fallback(sk, skb);

2 years, 5 months

1
0
0 0

FAILED: patch "[PATCH] virtio_net: fix memory leak inside XPD_TX with mergeable" failed to apply to 4.19-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 4.19-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 7a542bee27c6a57e45c33cbbdc963325fd6493af Mon Sep 17 00:00:00 2001 From: Xuan Zhuo <xuanzhuo(a)linux.alibaba.com> Date: Thu, 4 Aug 2022 14:32:48 +0800 Subject: [PATCH] virtio_net: fix memory leak inside XPD_TX with mergeable When we call xdp_convert_buff_to_frame() to get xdpf, if it returns NULL, we should check if xdp_page was allocated by xdp_linearize_page(). If it is newly allocated, it should be freed here alone. Just like any other "goto err_xdp". Fixes: 44fa2dbd4759 ("xdp: transition into using xdp_frame for ndo_xdp_xmit") Signed-off-by: Xuan Zhuo <xuanzhuo(a)linux.alibaba.com> Acked-by: Jason Wang <jasowang(a)redhat.com> Signed-off-by: David S. Miller <davem(a)davemloft.net> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index ec8e1b3108c3..3b3eebad3977 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1057,8 +1057,11 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, case XDP_TX: stats->xdp_tx++; xdpf = xdp_convert_buff_to_frame(&xdp); - if (unlikely(!xdpf)) + if (unlikely(!xdpf)) { + if (unlikely(xdp_page != page)) + put_page(xdp_page); goto err_xdp; + } err = virtnet_xdp_xmit(dev, 1, &xdpf, 0); if (unlikely(!err)) { xdp_return_frame_rx_napi(xdpf);

2 years, 5 months

1
0
0 0

FAILED: patch "[PATCH] virtio_net: fix memory leak inside XPD_TX with mergeable" failed to apply to 5.4-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 5.4-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 7a542bee27c6a57e45c33cbbdc963325fd6493af Mon Sep 17 00:00:00 2001 From: Xuan Zhuo <xuanzhuo(a)linux.alibaba.com> Date: Thu, 4 Aug 2022 14:32:48 +0800 Subject: [PATCH] virtio_net: fix memory leak inside XPD_TX with mergeable When we call xdp_convert_buff_to_frame() to get xdpf, if it returns NULL, we should check if xdp_page was allocated by xdp_linearize_page(). If it is newly allocated, it should be freed here alone. Just like any other "goto err_xdp". Fixes: 44fa2dbd4759 ("xdp: transition into using xdp_frame for ndo_xdp_xmit") Signed-off-by: Xuan Zhuo <xuanzhuo(a)linux.alibaba.com> Acked-by: Jason Wang <jasowang(a)redhat.com> Signed-off-by: David S. Miller <davem(a)davemloft.net> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index ec8e1b3108c3..3b3eebad3977 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1057,8 +1057,11 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, case XDP_TX: stats->xdp_tx++; xdpf = xdp_convert_buff_to_frame(&xdp); - if (unlikely(!xdpf)) + if (unlikely(!xdpf)) { + if (unlikely(xdp_page != page)) + put_page(xdp_page); goto err_xdp; + } err = virtnet_xdp_xmit(dev, 1, &xdpf, 0); if (unlikely(!err)) { xdp_return_frame_rx_napi(xdpf);

2 years, 5 months

1
0
0 0

[PATCH v2 00/10] x86: make pat and mtrr independent from each other

by Juergen Gross

Today PAT can't be used without MTRR being available, unless MTRR is at least configured via CONFIG_MTRR and the system is running as Xen PV guest. In this case PAT is automatically available via the hypervisor, but the PAT MSR can't be modified by the kernel and MTRR is disabled. The same applies to a kernel built with no MTRR support: it won't allow to use the PAT MSR, even if there is no technical reason for that, other than setting up PAT on all cpus the same way (which is a requirement of the processor's cache management) is relying on some MTRR specific code. Fix all of that by: - moving the function needed by PAT from MTRR specific code one level up - reworking the init sequences of MTRR and PAT to be more similar to each other without calling PAT from MTRR code - removing the dependency of PAT on MTRR While working on that I discovered two minor bugs in MTRR code, which are fixed, too. Changes in V2: - complete rework of the patches based on comments by Boris Petkov - added several patches to the series Juergen Gross (10): x86/mtrr: fix MTRR fixup on APs x86/mtrr: remove unused cyrix_set_all() function x86/mtrr: replace use_intel() with a local flag x86: move some code out of arch/x86/kernel/cpu/mtrr x86/mtrr: split generic_set_all() x86/mtrr: remove set_all callback from struct mtrr_ops x86/mtrr: simplify mtrr_bp_init() x86/mtrr: let cache_aps_delayed_init replace mtrr_aps_delayed_init x86/mtrr: add a stop_machine() handler calling only cache_cpu_init() x86: decouple pat and mtrr handling arch/x86/include/asm/cacheinfo.h | 14 +++ arch/x86/include/asm/memtype.h | 5 +- arch/x86/include/asm/mtrr.h | 12 +-- arch/x86/kernel/cpu/cacheinfo.c | 159 +++++++++++++++++++++++++++++ arch/x86/kernel/cpu/common.c | 3 +- arch/x86/kernel/cpu/mtrr/cyrix.c | 34 ------ arch/x86/kernel/cpu/mtrr/generic.c | 120 ++++------------------ arch/x86/kernel/cpu/mtrr/mtrr.c | 158 +++++----------------------- arch/x86/kernel/cpu/mtrr/mtrr.h | 5 - arch/x86/kernel/setup.c | 14 +-- arch/x86/kernel/smpboot.c | 9 +- arch/x86/mm/pat/memtype.c | 127 +++++++---------------- arch/x86/power/cpu.c | 3 +- 13 files changed, 274 insertions(+), 389 deletions(-) -- 2.35.3

2 years, 5 months

3
7
0 0

FAILED: patch "[PATCH] net: atlantic: fix aq_vec index out of range error" failed to apply to 4.14-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 4.14-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 2ba5e47fb75fbb8fab45f5c1bc8d5c33d8834bd3 Mon Sep 17 00:00:00 2001 From: "Chia-Lin Kao (AceLan)" <acelan.kao(a)canonical.com> Date: Mon, 8 Aug 2022 16:18:45 +0800 Subject: [PATCH] net: atlantic: fix aq_vec index out of range error The final update statement of the for loop exceeds the array range, the dereference of self->aq_vec[i] is not checked and then leads to the index out of range error. Also fixed this kind of coding style in other for loop. [ 97.937604] UBSAN: array-index-out-of-bounds in drivers/net/ethernet/aquantia/atlantic/aq_nic.c:1404:48 [ 97.937607] index 8 is out of range for type 'aq_vec_s *[8]' [ 97.937608] CPU: 38 PID: 3767 Comm: kworker/u256:18 Not tainted 5.19.0+ #2 [ 97.937610] Hardware name: Dell Inc. Precision 7865 Tower/, BIOS 1.0.0 06/12/2022 [ 97.937611] Workqueue: events_unbound async_run_entry_fn [ 97.937616] Call Trace: [ 97.937617] <TASK> [ 97.937619] dump_stack_lvl+0x49/0x63 [ 97.937624] dump_stack+0x10/0x16 [ 97.937626] ubsan_epilogue+0x9/0x3f [ 97.937627] __ubsan_handle_out_of_bounds.cold+0x44/0x49 [ 97.937629] ? __scm_send+0x348/0x440 [ 97.937632] ? aq_vec_stop+0x72/0x80 [atlantic] [ 97.937639] aq_nic_stop+0x1b6/0x1c0 [atlantic] [ 97.937644] aq_suspend_common+0x88/0x90 [atlantic] [ 97.937648] aq_pm_suspend_poweroff+0xe/0x20 [atlantic] [ 97.937653] pci_pm_suspend+0x7e/0x1a0 [ 97.937655] ? pci_pm_suspend_noirq+0x2b0/0x2b0 [ 97.937657] dpm_run_callback+0x54/0x190 [ 97.937660] __device_suspend+0x14c/0x4d0 [ 97.937661] async_suspend+0x23/0x70 [ 97.937663] async_run_entry_fn+0x33/0x120 [ 97.937664] process_one_work+0x21f/0x3f0 [ 97.937666] worker_thread+0x4a/0x3c0 [ 97.937668] ? process_one_work+0x3f0/0x3f0 [ 97.937669] kthread+0xf0/0x120 [ 97.937671] ? kthread_complete_and_exit+0x20/0x20 [ 97.937672] ret_from_fork+0x22/0x30 [ 97.937676] </TASK> v2. fixed "warning: variable 'aq_vec' set but not used" v3. simplified a for loop Fixes: 97bde5c4f909 ("net: ethernet: aquantia: Support for NIC-specific code") Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao(a)canonical.com> Acked-by: Sudarsana Reddy Kalluru <skalluru(a)marvell.com> Link: https://lore.kernel.org/r/20220808081845.42005-1-acelan.kao@canonical.com Signed-off-by: Jakub Kicinski <kuba(a)kernel.org> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c index e11cc29d3264..06508eebb585 100644 --- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c +++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c @@ -265,12 +265,10 @@ static void aq_nic_service_timer_cb(struct timer_list *t) static void aq_nic_polling_timer_cb(struct timer_list *t) { struct aq_nic_s *self = from_timer(self, t, polling_timer); - struct aq_vec_s *aq_vec = NULL; unsigned int i = 0U; - for (i = 0U, aq_vec = self->aq_vec[0]; - self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i]) - aq_vec_isr(i, (void *)aq_vec); + for (i = 0U; self->aq_vecs > i; ++i) + aq_vec_isr(i, (void *)self->aq_vec[i]); mod_timer(&self->polling_timer, jiffies + AQ_CFG_POLLING_TIMER_INTERVAL); @@ -1014,7 +1012,6 @@ int aq_nic_get_regs_count(struct aq_nic_s *self) u64 *aq_nic_get_stats(struct aq_nic_s *self, u64 *data) { - struct aq_vec_s *aq_vec = NULL; struct aq_stats_s *stats; unsigned int count = 0U; unsigned int i = 0U; @@ -1064,11 +1061,11 @@ u64 *aq_nic_get_stats(struct aq_nic_s *self, u64 *data) data += i; for (tc = 0U; tc < self->aq_nic_cfg.tcs; tc++) { - for (i = 0U, aq_vec = self->aq_vec[0]; - aq_vec && self->aq_vecs > i; - ++i, aq_vec = self->aq_vec[i]) { + for (i = 0U; self->aq_vecs > i; ++i) { + if (!self->aq_vec[i]) + break; data += count; - count = aq_vec_get_sw_stats(aq_vec, tc, data); + count = aq_vec_get_sw_stats(self->aq_vec[i], tc, data); } } @@ -1382,7 +1379,6 @@ int aq_nic_set_loopback(struct aq_nic_s *self) int aq_nic_stop(struct aq_nic_s *self) { - struct aq_vec_s *aq_vec = NULL; unsigned int i = 0U; netif_tx_disable(self->ndev); @@ -1400,9 +1396,8 @@ int aq_nic_stop(struct aq_nic_s *self) aq_ptp_irq_free(self); - for (i = 0U, aq_vec = self->aq_vec[0]; - self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i]) - aq_vec_stop(aq_vec); + for (i = 0U; self->aq_vecs > i; ++i) + aq_vec_stop(self->aq_vec[i]); aq_ptp_ring_stop(self);

2 years, 5 months

1
0
0 0

FAILED: patch "[PATCH] net: atlantic: fix aq_vec index out of range error" failed to apply to 4.19-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 4.19-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 2ba5e47fb75fbb8fab45f5c1bc8d5c33d8834bd3 Mon Sep 17 00:00:00 2001 From: "Chia-Lin Kao (AceLan)" <acelan.kao(a)canonical.com> Date: Mon, 8 Aug 2022 16:18:45 +0800 Subject: [PATCH] net: atlantic: fix aq_vec index out of range error The final update statement of the for loop exceeds the array range, the dereference of self->aq_vec[i] is not checked and then leads to the index out of range error. Also fixed this kind of coding style in other for loop. [ 97.937604] UBSAN: array-index-out-of-bounds in drivers/net/ethernet/aquantia/atlantic/aq_nic.c:1404:48 [ 97.937607] index 8 is out of range for type 'aq_vec_s *[8]' [ 97.937608] CPU: 38 PID: 3767 Comm: kworker/u256:18 Not tainted 5.19.0+ #2 [ 97.937610] Hardware name: Dell Inc. Precision 7865 Tower/, BIOS 1.0.0 06/12/2022 [ 97.937611] Workqueue: events_unbound async_run_entry_fn [ 97.937616] Call Trace: [ 97.937617] <TASK> [ 97.937619] dump_stack_lvl+0x49/0x63 [ 97.937624] dump_stack+0x10/0x16 [ 97.937626] ubsan_epilogue+0x9/0x3f [ 97.937627] __ubsan_handle_out_of_bounds.cold+0x44/0x49 [ 97.937629] ? __scm_send+0x348/0x440 [ 97.937632] ? aq_vec_stop+0x72/0x80 [atlantic] [ 97.937639] aq_nic_stop+0x1b6/0x1c0 [atlantic] [ 97.937644] aq_suspend_common+0x88/0x90 [atlantic] [ 97.937648] aq_pm_suspend_poweroff+0xe/0x20 [atlantic] [ 97.937653] pci_pm_suspend+0x7e/0x1a0 [ 97.937655] ? pci_pm_suspend_noirq+0x2b0/0x2b0 [ 97.937657] dpm_run_callback+0x54/0x190 [ 97.937660] __device_suspend+0x14c/0x4d0 [ 97.937661] async_suspend+0x23/0x70 [ 97.937663] async_run_entry_fn+0x33/0x120 [ 97.937664] process_one_work+0x21f/0x3f0 [ 97.937666] worker_thread+0x4a/0x3c0 [ 97.937668] ? process_one_work+0x3f0/0x3f0 [ 97.937669] kthread+0xf0/0x120 [ 97.937671] ? kthread_complete_and_exit+0x20/0x20 [ 97.937672] ret_from_fork+0x22/0x30 [ 97.937676] </TASK> v2. fixed "warning: variable 'aq_vec' set but not used" v3. simplified a for loop Fixes: 97bde5c4f909 ("net: ethernet: aquantia: Support for NIC-specific code") Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao(a)canonical.com> Acked-by: Sudarsana Reddy Kalluru <skalluru(a)marvell.com> Link: https://lore.kernel.org/r/20220808081845.42005-1-acelan.kao@canonical.com Signed-off-by: Jakub Kicinski <kuba(a)kernel.org> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c index e11cc29d3264..06508eebb585 100644 --- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c +++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c @@ -265,12 +265,10 @@ static void aq_nic_service_timer_cb(struct timer_list *t) static void aq_nic_polling_timer_cb(struct timer_list *t) { struct aq_nic_s *self = from_timer(self, t, polling_timer); - struct aq_vec_s *aq_vec = NULL; unsigned int i = 0U; - for (i = 0U, aq_vec = self->aq_vec[0]; - self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i]) - aq_vec_isr(i, (void *)aq_vec); + for (i = 0U; self->aq_vecs > i; ++i) + aq_vec_isr(i, (void *)self->aq_vec[i]); mod_timer(&self->polling_timer, jiffies + AQ_CFG_POLLING_TIMER_INTERVAL); @@ -1014,7 +1012,6 @@ int aq_nic_get_regs_count(struct aq_nic_s *self) u64 *aq_nic_get_stats(struct aq_nic_s *self, u64 *data) { - struct aq_vec_s *aq_vec = NULL; struct aq_stats_s *stats; unsigned int count = 0U; unsigned int i = 0U; @@ -1064,11 +1061,11 @@ u64 *aq_nic_get_stats(struct aq_nic_s *self, u64 *data) data += i; for (tc = 0U; tc < self->aq_nic_cfg.tcs; tc++) { - for (i = 0U, aq_vec = self->aq_vec[0]; - aq_vec && self->aq_vecs > i; - ++i, aq_vec = self->aq_vec[i]) { + for (i = 0U; self->aq_vecs > i; ++i) { + if (!self->aq_vec[i]) + break; data += count; - count = aq_vec_get_sw_stats(aq_vec, tc, data); + count = aq_vec_get_sw_stats(self->aq_vec[i], tc, data); } } @@ -1382,7 +1379,6 @@ int aq_nic_set_loopback(struct aq_nic_s *self) int aq_nic_stop(struct aq_nic_s *self) { - struct aq_vec_s *aq_vec = NULL; unsigned int i = 0U; netif_tx_disable(self->ndev); @@ -1400,9 +1396,8 @@ int aq_nic_stop(struct aq_nic_s *self) aq_ptp_irq_free(self); - for (i = 0U, aq_vec = self->aq_vec[0]; - self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i]) - aq_vec_stop(aq_vec); + for (i = 0U; self->aq_vecs > i; ++i) + aq_vec_stop(self->aq_vec[i]); aq_ptp_ring_stop(self);

2 years, 5 months

1
0
0 0

FAILED: patch "[PATCH] net: atlantic: fix aq_vec index out of range error" failed to apply to 5.4-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 5.4-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From 2ba5e47fb75fbb8fab45f5c1bc8d5c33d8834bd3 Mon Sep 17 00:00:00 2001 From: "Chia-Lin Kao (AceLan)" <acelan.kao(a)canonical.com> Date: Mon, 8 Aug 2022 16:18:45 +0800 Subject: [PATCH] net: atlantic: fix aq_vec index out of range error The final update statement of the for loop exceeds the array range, the dereference of self->aq_vec[i] is not checked and then leads to the index out of range error. Also fixed this kind of coding style in other for loop. [ 97.937604] UBSAN: array-index-out-of-bounds in drivers/net/ethernet/aquantia/atlantic/aq_nic.c:1404:48 [ 97.937607] index 8 is out of range for type 'aq_vec_s *[8]' [ 97.937608] CPU: 38 PID: 3767 Comm: kworker/u256:18 Not tainted 5.19.0+ #2 [ 97.937610] Hardware name: Dell Inc. Precision 7865 Tower/, BIOS 1.0.0 06/12/2022 [ 97.937611] Workqueue: events_unbound async_run_entry_fn [ 97.937616] Call Trace: [ 97.937617] <TASK> [ 97.937619] dump_stack_lvl+0x49/0x63 [ 97.937624] dump_stack+0x10/0x16 [ 97.937626] ubsan_epilogue+0x9/0x3f [ 97.937627] __ubsan_handle_out_of_bounds.cold+0x44/0x49 [ 97.937629] ? __scm_send+0x348/0x440 [ 97.937632] ? aq_vec_stop+0x72/0x80 [atlantic] [ 97.937639] aq_nic_stop+0x1b6/0x1c0 [atlantic] [ 97.937644] aq_suspend_common+0x88/0x90 [atlantic] [ 97.937648] aq_pm_suspend_poweroff+0xe/0x20 [atlantic] [ 97.937653] pci_pm_suspend+0x7e/0x1a0 [ 97.937655] ? pci_pm_suspend_noirq+0x2b0/0x2b0 [ 97.937657] dpm_run_callback+0x54/0x190 [ 97.937660] __device_suspend+0x14c/0x4d0 [ 97.937661] async_suspend+0x23/0x70 [ 97.937663] async_run_entry_fn+0x33/0x120 [ 97.937664] process_one_work+0x21f/0x3f0 [ 97.937666] worker_thread+0x4a/0x3c0 [ 97.937668] ? process_one_work+0x3f0/0x3f0 [ 97.937669] kthread+0xf0/0x120 [ 97.937671] ? kthread_complete_and_exit+0x20/0x20 [ 97.937672] ret_from_fork+0x22/0x30 [ 97.937676] </TASK> v2. fixed "warning: variable 'aq_vec' set but not used" v3. simplified a for loop Fixes: 97bde5c4f909 ("net: ethernet: aquantia: Support for NIC-specific code") Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao(a)canonical.com> Acked-by: Sudarsana Reddy Kalluru <skalluru(a)marvell.com> Link: https://lore.kernel.org/r/20220808081845.42005-1-acelan.kao@canonical.com Signed-off-by: Jakub Kicinski <kuba(a)kernel.org> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c index e11cc29d3264..06508eebb585 100644 --- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c +++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c @@ -265,12 +265,10 @@ static void aq_nic_service_timer_cb(struct timer_list *t) static void aq_nic_polling_timer_cb(struct timer_list *t) { struct aq_nic_s *self = from_timer(self, t, polling_timer); - struct aq_vec_s *aq_vec = NULL; unsigned int i = 0U; - for (i = 0U, aq_vec = self->aq_vec[0]; - self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i]) - aq_vec_isr(i, (void *)aq_vec); + for (i = 0U; self->aq_vecs > i; ++i) + aq_vec_isr(i, (void *)self->aq_vec[i]); mod_timer(&self->polling_timer, jiffies + AQ_CFG_POLLING_TIMER_INTERVAL); @@ -1014,7 +1012,6 @@ int aq_nic_get_regs_count(struct aq_nic_s *self) u64 *aq_nic_get_stats(struct aq_nic_s *self, u64 *data) { - struct aq_vec_s *aq_vec = NULL; struct aq_stats_s *stats; unsigned int count = 0U; unsigned int i = 0U; @@ -1064,11 +1061,11 @@ u64 *aq_nic_get_stats(struct aq_nic_s *self, u64 *data) data += i; for (tc = 0U; tc < self->aq_nic_cfg.tcs; tc++) { - for (i = 0U, aq_vec = self->aq_vec[0]; - aq_vec && self->aq_vecs > i; - ++i, aq_vec = self->aq_vec[i]) { + for (i = 0U; self->aq_vecs > i; ++i) { + if (!self->aq_vec[i]) + break; data += count; - count = aq_vec_get_sw_stats(aq_vec, tc, data); + count = aq_vec_get_sw_stats(self->aq_vec[i], tc, data); } } @@ -1382,7 +1379,6 @@ int aq_nic_set_loopback(struct aq_nic_s *self) int aq_nic_stop(struct aq_nic_s *self) { - struct aq_vec_s *aq_vec = NULL; unsigned int i = 0U; netif_tx_disable(self->ndev); @@ -1400,9 +1396,8 @@ int aq_nic_stop(struct aq_nic_s *self) aq_ptp_irq_free(self); - for (i = 0U, aq_vec = self->aq_vec[0]; - self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i]) - aq_vec_stop(aq_vec); + for (i = 0U; self->aq_vecs > i; ++i) + aq_vec_stop(self->aq_vec[i]); aq_ptp_ring_stop(self);

2 years, 5 months

1
0
0 0

FAILED: patch "[PATCH] can: mcp251x: Fix race condition on receive interrupt" failed to apply to 4.9-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 4.9-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From d80d60b0db6ff3dd2e29247cc2a5166d7e9ae37e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20W=C3=BCrl?= <sebastian.wuerl(a)ororatech.com> Date: Thu, 4 Aug 2022 10:14:11 +0200 Subject: [PATCH] can: mcp251x: Fix race condition on receive interrupt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mcp251x driver uses both receiving mailboxes of the CAN controller chips. For retrieving the CAN frames from the controller via SPI, it checks once per interrupt which mailboxes have been filled and will retrieve the messages accordingly. This introduces a race condition, as another CAN frame can enter mailbox 1 while mailbox 0 is emptied. If now another CAN frame enters mailbox 0 until the interrupt handler is called next, mailbox 0 is emptied before mailbox 1, leading to out-of-order CAN frames in the network device. This is fixed by checking the interrupt flags once again after freeing mailbox 0, to correctly also empty mailbox 1 before leaving the handler. For reproducing the bug I created the following setup: - Two CAN devices, one Raspberry Pi with MCP2515, the other can be any. - Setup CAN to 1 MHz - Spam bursts of 5 CAN-messages with increasing CAN-ids - Continue sending the bursts while sleeping a second between the bursts - Check on the RPi whether the received messages have increasing CAN-ids - Without this patch, every burst of messages will contain a flipped pair v3: https://lore.kernel.org/all/20220804075914.67569-1-sebastian.wuerl@ororatec… v2: https://lore.kernel.org/all/20220804064803.63157-1-sebastian.wuerl@ororatec… v1: https://lore.kernel.org/all/20220803153300.58732-1-sebastian.wuerl@ororatec… Fixes: bf66f3736a94 ("can: mcp251x: Move to threaded interrupts instead of workqueues.") Signed-off-by: Sebastian Würl <sebastian.wuerl(a)ororatech.com> Link: https://lore.kernel.org/all/20220804081411.68567-1-sebastian.wuerl@ororatec… [mkl: reduce scope of intf1, eflag1] Signed-off-by: Marc Kleine-Budde <mkl(a)pengutronix.de> diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c index e750d13c8841..c320de474f40 100644 --- a/drivers/net/can/spi/mcp251x.c +++ b/drivers/net/can/spi/mcp251x.c @@ -1070,9 +1070,6 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id) mcp251x_read_2regs(spi, CANINTF, &intf, &eflag); - /* mask out flags we don't care about */ - intf &= CANINTF_RX | CANINTF_TX | CANINTF_ERR; - /* receive buffer 0 */ if (intf & CANINTF_RX0IF) { mcp251x_hw_rx(spi, 0); @@ -1082,6 +1079,18 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id) if (mcp251x_is_2510(spi)) mcp251x_write_bits(spi, CANINTF, CANINTF_RX0IF, 0x00); + + /* check if buffer 1 is already known to be full, no need to re-read */ + if (!(intf & CANINTF_RX1IF)) { + u8 intf1, eflag1; + + /* intf needs to be read again to avoid a race condition */ + mcp251x_read_2regs(spi, CANINTF, &intf1, &eflag1); + + /* combine flags from both operations for error handling */ + intf |= intf1; + eflag |= eflag1; + } } /* receive buffer 1 */ @@ -1092,6 +1101,9 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id) clear_intf |= CANINTF_RX1IF; } + /* mask out flags we don't care about */ + intf &= CANINTF_RX | CANINTF_TX | CANINTF_ERR; + /* any error or tx interrupt we need to clear? */ if (intf & (CANINTF_ERR | CANINTF_TX)) clear_intf |= intf & (CANINTF_ERR | CANINTF_TX);

2 years, 5 months

1
0
0 0

FAILED: patch "[PATCH] can: mcp251x: Fix race condition on receive interrupt" failed to apply to 4.14-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 4.14-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ From d80d60b0db6ff3dd2e29247cc2a5166d7e9ae37e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20W=C3=BCrl?= <sebastian.wuerl(a)ororatech.com> Date: Thu, 4 Aug 2022 10:14:11 +0200 Subject: [PATCH] can: mcp251x: Fix race condition on receive interrupt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mcp251x driver uses both receiving mailboxes of the CAN controller chips. For retrieving the CAN frames from the controller via SPI, it checks once per interrupt which mailboxes have been filled and will retrieve the messages accordingly. This introduces a race condition, as another CAN frame can enter mailbox 1 while mailbox 0 is emptied. If now another CAN frame enters mailbox 0 until the interrupt handler is called next, mailbox 0 is emptied before mailbox 1, leading to out-of-order CAN frames in the network device. This is fixed by checking the interrupt flags once again after freeing mailbox 0, to correctly also empty mailbox 1 before leaving the handler. For reproducing the bug I created the following setup: - Two CAN devices, one Raspberry Pi with MCP2515, the other can be any. - Setup CAN to 1 MHz - Spam bursts of 5 CAN-messages with increasing CAN-ids - Continue sending the bursts while sleeping a second between the bursts - Check on the RPi whether the received messages have increasing CAN-ids - Without this patch, every burst of messages will contain a flipped pair v3: https://lore.kernel.org/all/20220804075914.67569-1-sebastian.wuerl@ororatec… v2: https://lore.kernel.org/all/20220804064803.63157-1-sebastian.wuerl@ororatec… v1: https://lore.kernel.org/all/20220803153300.58732-1-sebastian.wuerl@ororatec… Fixes: bf66f3736a94 ("can: mcp251x: Move to threaded interrupts instead of workqueues.") Signed-off-by: Sebastian Würl <sebastian.wuerl(a)ororatech.com> Link: https://lore.kernel.org/all/20220804081411.68567-1-sebastian.wuerl@ororatec… [mkl: reduce scope of intf1, eflag1] Signed-off-by: Marc Kleine-Budde <mkl(a)pengutronix.de> diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c index e750d13c8841..c320de474f40 100644 --- a/drivers/net/can/spi/mcp251x.c +++ b/drivers/net/can/spi/mcp251x.c @@ -1070,9 +1070,6 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id) mcp251x_read_2regs(spi, CANINTF, &intf, &eflag); - /* mask out flags we don't care about */ - intf &= CANINTF_RX | CANINTF_TX | CANINTF_ERR; - /* receive buffer 0 */ if (intf & CANINTF_RX0IF) { mcp251x_hw_rx(spi, 0); @@ -1082,6 +1079,18 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id) if (mcp251x_is_2510(spi)) mcp251x_write_bits(spi, CANINTF, CANINTF_RX0IF, 0x00); + + /* check if buffer 1 is already known to be full, no need to re-read */ + if (!(intf & CANINTF_RX1IF)) { + u8 intf1, eflag1; + + /* intf needs to be read again to avoid a race condition */ + mcp251x_read_2regs(spi, CANINTF, &intf1, &eflag1); + + /* combine flags from both operations for error handling */ + intf |= intf1; + eflag |= eflag1; + } } /* receive buffer 1 */ @@ -1092,6 +1101,9 @@ static irqreturn_t mcp251x_can_ist(int irq, void *dev_id) clear_intf |= CANINTF_RX1IF; } + /* mask out flags we don't care about */ + intf &= CANINTF_RX | CANINTF_TX | CANINTF_ERR; + /* any error or tx interrupt we need to clear? */ if (intf & (CANINTF_ERR | CANINTF_TX)) clear_intf |= intf & (CANINTF_ERR | CANINTF_TX);

2 years, 5 months

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror August 2022