| Message ID | 20231025145050.36114-1-przemyslaw.kitszel@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5af8d8ce643478d754ef72fc239466f6ad0e2562 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net/mlx5: fix uninit value use | expand |
On Wed, Oct 25, 2023 at 04:50:50PM +0200, Przemek Kitszel wrote: > Avoid use of uninitialized state variable. > > In case of mlx5e_tx_reporter_build_diagnose_output_sq_common() it's better > to still collect other data than bail out entirely. > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Link: https://lore.kernel.org/netdev/8bd30131-c9f2-4075-a575-7fa2793a1760@moroto.mountain > Fixes: d17f98bf7cc9 ("net/mlx5: devlink health: use retained error fmsg API") > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > --- > drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c | 6 +++++- > drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c | 8 ++++++-- > 2 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c > index fc5a9fdd06db..fea8c0a5fe89 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c > @@ -263,8 +263,12 @@ mlx5e_rx_reporter_build_diagnose_output_rq_common(struct mlx5e_rq *rq, > if (rq->icosq) { > struct mlx5e_icosq *icosq = rq->icosq; > u8 icosq_hw_state; > + int err; > + > + err = mlx5_core_query_sq_state(rq->mdev, icosq->sqn, &icosq_hw_state); > + if (err) > + return err; > > - mlx5_core_query_sq_state(rq->mdev, icosq->sqn, &icosq_hw_state); > mlx5e_reporter_icosq_diagnose(icosq, icosq_hw_state, fmsg); > } > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c > index ccff7c26d6ac..6b44ddce14e9 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c > @@ -221,12 +221,16 @@ mlx5e_tx_reporter_build_diagnose_output_sq_common(struct devlink_fmsg *fmsg, > bool stopped = netif_xmit_stopped(sq->txq); > struct mlx5e_priv *priv = sq->priv; > u8 state; > + int err; > > - mlx5_core_query_sq_state(priv->mdev, sq->sqn, &state); > devlink_fmsg_u32_pair_put(fmsg, "tc", tc); > devlink_fmsg_u32_pair_put(fmsg, "txq ix", sq->txq_ix); > devlink_fmsg_u32_pair_put(fmsg, "sqn", sq->sqn); > - devlink_fmsg_u8_pair_put(fmsg, "HW state", state); > + > + err = mlx5_core_query_sq_state(priv->mdev, sq->sqn, &state); > + if (!err) > + devlink_fmsg_u8_pair_put(fmsg, "HW state", state); > + > devlink_fmsg_bool_pair_put(fmsg, "stopped", stopped); > devlink_fmsg_u32_pair_put(fmsg, "cc", sq->cc); > devlink_fmsg_u32_pair_put(fmsg, "pc", sq->pc); > -- LGTM Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> > 2.38.1
On 25 Oct 16:50, Przemek Kitszel wrote: >Avoid use of uninitialized state variable. > >In case of mlx5e_tx_reporter_build_diagnose_output_sq_common() it's better >to still collect other data than bail out entirely. > >Reported-by: Dan Carpenter <dan.carpenter@linaro.org> >Link: https://lore.kernel.org/netdev/8bd30131-c9f2-4075-a575-7fa2793a1760@moroto.mountain >Fixes: d17f98bf7cc9 ("net/mlx5: devlink health: use retained error fmsg API") >Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> Applied to net-next-mlx5
On Thu, 26 Oct 2023 15:14:37 -0700 Saeed Mahameed wrote: > >Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > >Link: https://lore.kernel.org/netdev/8bd30131-c9f2-4075-a575-7fa2793a1760@moroto.mountain > >Fixes: d17f98bf7cc9 ("net/mlx5: devlink health: use retained error fmsg API") > >Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > > Applied to net-next-mlx5 Given the hold up with your PR I prefer to take this directly, thanks.
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Wed, 25 Oct 2023 16:50:50 +0200 you wrote: > Avoid use of uninitialized state variable. > > In case of mlx5e_tx_reporter_build_diagnose_output_sq_common() it's better > to still collect other data than bail out entirely. > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Link: https://lore.kernel.org/netdev/8bd30131-c9f2-4075-a575-7fa2793a1760@moroto.mountain > Fixes: d17f98bf7cc9 ("net/mlx5: devlink health: use retained error fmsg API") > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > > [...] Here is the summary with links: - [net-next] net/mlx5: fix uninit value use https://git.kernel.org/netdev/net-next/c/5af8d8ce6434 You are awesome, thank you!
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c index fc5a9fdd06db..fea8c0a5fe89 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c @@ -263,8 +263,12 @@ mlx5e_rx_reporter_build_diagnose_output_rq_common(struct mlx5e_rq *rq, if (rq->icosq) { struct mlx5e_icosq *icosq = rq->icosq; u8 icosq_hw_state; + int err; + + err = mlx5_core_query_sq_state(rq->mdev, icosq->sqn, &icosq_hw_state); + if (err) + return err; - mlx5_core_query_sq_state(rq->mdev, icosq->sqn, &icosq_hw_state); mlx5e_reporter_icosq_diagnose(icosq, icosq_hw_state, fmsg); } diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c index ccff7c26d6ac..6b44ddce14e9 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c @@ -221,12 +221,16 @@ mlx5e_tx_reporter_build_diagnose_output_sq_common(struct devlink_fmsg *fmsg, bool stopped = netif_xmit_stopped(sq->txq); struct mlx5e_priv *priv = sq->priv; u8 state; + int err; - mlx5_core_query_sq_state(priv->mdev, sq->sqn, &state); devlink_fmsg_u32_pair_put(fmsg, "tc", tc); devlink_fmsg_u32_pair_put(fmsg, "txq ix", sq->txq_ix); devlink_fmsg_u32_pair_put(fmsg, "sqn", sq->sqn); - devlink_fmsg_u8_pair_put(fmsg, "HW state", state); + + err = mlx5_core_query_sq_state(priv->mdev, sq->sqn, &state); + if (!err) + devlink_fmsg_u8_pair_put(fmsg, "HW state", state); + devlink_fmsg_bool_pair_put(fmsg, "stopped", stopped); devlink_fmsg_u32_pair_put(fmsg, "cc", sq->cc); devlink_fmsg_u32_pair_put(fmsg, "pc", sq->pc);
Avoid use of uninitialized state variable. In case of mlx5e_tx_reporter_build_diagnose_output_sq_common() it's better to still collect other data than bail out entirely. Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Link: https://lore.kernel.org/netdev/8bd30131-c9f2-4075-a575-7fa2793a1760@moroto.mountain Fixes: d17f98bf7cc9 ("net/mlx5: devlink health: use retained error fmsg API") Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> --- drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c | 6 +++++- drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c | 8 ++++++-- 2 files changed, 11 insertions(+), 3 deletions(-)