Re: [PATCH -next] vdpa/mlx5: fix check wrong pointer in mlx5_vdpa_add_mac_vlan_rules()

Re: [PATCH -next] vdpa/mlx5: fix check wrong pointer in mlx5_vdpa_add_mac_vlan_rules()

[Michael S. Tsirkin]

On Wed, Jan 04, 2023 at 11:41:31AM +0000, Eli Cohen wrote:
> I see these patches were not sent yet.
> Michael/Yang Yinglliang, isn't it better to send a fixed patch instead of sending the original and fix on top of it?

depends on the severity of the fix. in this case it seems to leak
uninitialized stack data to userspace, so I'd say it's best to
squash in the fix.
keep signed-off by of the fixup contributor so they get attribution.

-- 
MST

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH -next] vdpa/mlx5: fix check wrong pointer in mlx5_vdpa_add_mac_vlan_rules()

[Jason Wang]

On Wed, Jan 4, 2023 at 3:42 PM Yang Yingliang <yangyingliang@huawei.com> wrote:
>
> The local variable 'rule' is not used anymore, fix return value
> check after calling mlx5_add_flow_rules().
>
> Fixes: 8b14d56c6128 ("vdpa/mlx5: Add RX counters to debugfs")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

> ---
>  drivers/vdpa/mlx5/net/mlx5_vnet.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 6632651b1e54..75f800685677 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -1454,7 +1454,6 @@ static int mlx5_vdpa_add_mac_vlan_rules(struct mlx5_vdpa_net *ndev, u8 *mac,
>  {
>         struct mlx5_flow_destination dests[NUM_DESTS] = {};
>         struct mlx5_flow_act flow_act = {};
> -       struct mlx5_flow_handle *rule;
>         struct mlx5_flow_spec *spec;
>         void *headers_c;
>         void *headers_v;
> @@ -1494,8 +1493,8 @@ static int mlx5_vdpa_add_mac_vlan_rules(struct mlx5_vdpa_net *ndev, u8 *mac,
>         dests[1].counter_id = mlx5_fc_id(node->ucast_counter.counter);
>  #endif
>         node->ucast_rule = mlx5_add_flow_rules(ndev->rxft, spec, &flow_act, dests, NUM_DESTS);
> -       if (IS_ERR(rule)) {
> -               err = PTR_ERR(rule);
> +       if (IS_ERR(node->ucast_rule)) {
> +               err = PTR_ERR(node->ucast_rule);
>                 goto err_ucast;
>         }
>
> @@ -1508,8 +1507,8 @@ static int mlx5_vdpa_add_mac_vlan_rules(struct mlx5_vdpa_net *ndev, u8 *mac,
>         dmac_c[0] = 1;
>         dmac_v[0] = 1;
>         node->mcast_rule = mlx5_add_flow_rules(ndev->rxft, spec, &flow_act, dests, NUM_DESTS);
> -       if (IS_ERR(rule)) {
> -               err = PTR_ERR(rule);
> +       if (IS_ERR(node->mcast_rule)) {
> +               err = PTR_ERR(node->mcast_rule);
>                 goto err_mcast;
>         }
>         kvfree(spec);
> --
> 2.25.1
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH -next] vdpa/mlx5: fix check wrong pointer in mlx5_vdpa_add_mac_vlan_rules()

[Michael S. Tsirkin]

Were you going to do this?

On Wed, Jan 04, 2023 at 02:31:52PM +0000, Eli Cohen wrote:
> Sure, thanks!
> 
> > -----Original Message-----
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, 4 January 2023 15:58
> > To: Eli Cohen <elic@nvidia.com>
> > Cc: Yang Yingliang <yangyingliang@huawei.com>; virtualization@lists.linux-
> > foundation.org; jasowang@redhat.com
> > Subject: Re: [PATCH -next] vdpa/mlx5: fix check wrong pointer in
> > mlx5_vdpa_add_mac_vlan_rules()
> > 
> > On Wed, Jan 04, 2023 at 11:41:31AM +0000, Eli Cohen wrote:
> > > I see these patches were not sent yet.
> > > Michael/Yang Yinglliang, isn't it better to send a fixed patch instead of
> > sending the original and fix on top of it?
> > 
> > depends on the severity of the fix. in this case it seems to leak
> > uninitialized stack data to userspace, so I'd say it's best to
> > squash in the fix.
> > keep signed-off by of the fixup contributor so they get attribution.
> > 
> > --
> > MST
> 

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization