* [bug report] net/mlx5e: Add mlx5e HV VHCA stats agent
@ 2019-08-26 12:56 Dan Carpenter
2019-08-28 7:41 ` Eran Ben Elisha
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2019-08-26 12:56 UTC (permalink / raw)
To: eranbe; +Cc: linux-rdma
Hello Eran Ben Elisha,
The patch cef35af34d6d: "net/mlx5e: Add mlx5e HV VHCA stats agent"
from Aug 22, 2019, leads to the following static checker warning:
drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c:41 mlx5e_hv_vhca_fill_stats()
warn: potential pointer math issue ('buf' is a u64 pointer)
drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c
33 static void mlx5e_hv_vhca_fill_stats(struct mlx5e_priv *priv, u64 *data,
^^^^^^^^^
data is a u64 pointer.
34 int buf_len)
35 {
36 int ch, i = 0;
37
38 for (ch = 0; ch < priv->max_nch; ch++) {
39 u64 *buf = data + i;
^^^^^^^^
40
41 if (WARN_ON_ONCE(buf +
42 sizeof(struct mlx5e_hv_vhca_per_ring_stats) >
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This pointer math doesn't work. I'm surprised the warning doesn't
trigger.
43 data + buf_len))
44 return;
45
46 mlx5e_hv_vhca_fill_ring_stats(priv, ch,
47 (struct mlx5e_hv_vhca_per_ring_stats *)buf);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
If we made both "data" and "buf" void pointers then this would be
much easier and the casting could be removed.
48 i += sizeof(struct mlx5e_hv_vhca_per_ring_stats) / sizeof(u64);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This math works, but it's so complicated and you have to verify that
mlx5e_hv_vhca_per_ring_stats % sizeof(u64) is zero.
49 }
50 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] net/mlx5e: Add mlx5e HV VHCA stats agent
2019-08-26 12:56 [bug report] net/mlx5e: Add mlx5e HV VHCA stats agent Dan Carpenter
@ 2019-08-28 7:41 ` Eran Ben Elisha
2019-08-28 8:46 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Eran Ben Elisha @ 2019-08-28 7:41 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-rdma@vger.kernel.org, Saeed Mahameed
On 8/26/2019 3:56 PM, Dan Carpenter wrote:
> Hello Eran Ben Elisha,
>
> The patch cef35af34d6d: "net/mlx5e: Add mlx5e HV VHCA stats agent"
> from Aug 22, 2019, leads to the following static checker warning:
>
> drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c:41 mlx5e_hv_vhca_fill_stats()
> warn: potential pointer math issue ('buf' is a u64 pointer)
>
> drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c
> 33 static void mlx5e_hv_vhca_fill_stats(struct mlx5e_priv *priv, u64 *data,
> ^^^^^^^^^
> data is a u64 pointer.
>
> 34 int buf_len)
> 35 {
> 36 int ch, i = 0;
> 37
> 38 for (ch = 0; ch < priv->max_nch; ch++) {
> 39 u64 *buf = data + i;
> ^^^^^^^^
>
> 40
> 41 if (WARN_ON_ONCE(buf +
> 42 sizeof(struct mlx5e_hv_vhca_per_ring_stats) >
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This pointer math doesn't work. I'm surprised the warning doesn't
> trigger.
It it not triggered as both 'data' and 'buf' are u64*,
and sizeof(struct mlx5e_hv_vhca_per_ring_stats) < buf_len as expected.
This checker does the work, but over wrong range.
>
> 43 data + buf_len))
> 44 return;
> 45
> 46 mlx5e_hv_vhca_fill_ring_stats(priv, ch,
> 47 (struct mlx5e_hv_vhca_per_ring_stats *)buf);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> If we made both "data" and "buf" void pointers then this would be
> much easier and the casting could be removed.
I agree I can make it simpler, I will post a fix.
>
>
> 48 i += sizeof(struct mlx5e_hv_vhca_per_ring_stats) / sizeof(u64);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This math works, but it's so complicated and you have to verify that
> mlx5e_hv_vhca_per_ring_stats % sizeof(u64) is zero.
It shouldn't be zero. Basically, it equals to a static value of 4...
the code is written in that way to support extension of
mlx5e_hv_vhca_per_ring_stats without touching here.
>
> 49 }
> 50 }
>
> regards,
> dan carpenter
Thanks you for the report,
Eran
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] net/mlx5e: Add mlx5e HV VHCA stats agent
2019-08-28 7:41 ` Eran Ben Elisha
@ 2019-08-28 8:46 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2019-08-28 8:46 UTC (permalink / raw)
To: Eran Ben Elisha; +Cc: linux-rdma@vger.kernel.org, Saeed Mahameed
On Wed, Aug 28, 2019 at 07:41:14AM +0000, Eran Ben Elisha wrote:
>
>
> On 8/26/2019 3:56 PM, Dan Carpenter wrote:
> > Hello Eran Ben Elisha,
> >
> > The patch cef35af34d6d: "net/mlx5e: Add mlx5e HV VHCA stats agent"
> > from Aug 22, 2019, leads to the following static checker warning:
> >
> > drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c:41 mlx5e_hv_vhca_fill_stats()
> > warn: potential pointer math issue ('buf' is a u64 pointer)
> >
> > drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c
> > 33 static void mlx5e_hv_vhca_fill_stats(struct mlx5e_priv *priv, u64 *data,
> > ^^^^^^^^^
> > data is a u64 pointer.
> >
> > 34 int buf_len)
> > 35 {
> > 36 int ch, i = 0;
> > 37
> > 38 for (ch = 0; ch < priv->max_nch; ch++) {
> > 39 u64 *buf = data + i;
> > ^^^^^^^^
> >
> > 40
> > 41 if (WARN_ON_ONCE(buf +
> > 42 sizeof(struct mlx5e_hv_vhca_per_ring_stats) >
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > This pointer math doesn't work. I'm surprised the warning doesn't
> > trigger.
>
> It it not triggered as both 'data' and 'buf' are u64*,
> and sizeof(struct mlx5e_hv_vhca_per_ring_stats) < buf_len as expected.
> This checker does the work, but over wrong range.
Ah. Of course. Thanks!
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-08-28 8:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-26 12:56 [bug report] net/mlx5e: Add mlx5e HV VHCA stats agent Dan Carpenter
2019-08-28 7:41 ` Eran Ben Elisha
2019-08-28 8:46 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox