* [bug report] vdpa/mlx5: Support different address spaces for control and data
@ 2022-08-11 10:39 Dan Carpenter
[not found] ` <DM8PR12MB54007865AC6BC6FCEC9911FCAB649@DM8PR12MB5400.namprd12.prod.outlook.com>
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2022-08-11 10:39 UTC (permalink / raw)
To: elic; +Cc: virtualization
Hello Eli Cohen,
The patch d5358cd0e369: "vdpa/mlx5: Support different address spaces
for control and data" from Jul 14, 2022, leads to the following
Smatch static checker warning:
drivers/vdpa/mlx5/net/mlx5_vnet.c:2676 mlx5_vdpa_set_map()
error: uninitialized symbol 'err'.
drivers/vdpa/mlx5/net/mlx5_vnet.c
2657 static int mlx5_vdpa_set_map(struct vdpa_device *vdev, unsigned int asid,
2658 struct vhost_iotlb *iotlb)
2659 {
2660 struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
2661 struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
2662 int err;
2663
2664 down_write(&ndev->reslock);
2665 if (mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] == asid) {
2666 err = set_map_data(mvdev, iotlb);
2667 if (err)
2668 goto out;
2669 }
2670
2671 if (mvdev->group2asid[MLX5_VDPA_CVQ_GROUP] == asid)
2672 err = set_map_control(mvdev, iotlb);
err not initialized on else path. My guess is that one or both of these
conditions has to be true and this is a false positive but I don't know
the code well enough to be sure.
2673
2674 out:
2675 up_write(&ndev->reslock);
--> 2676 return err;
2677 }
regards,
dan carpenter
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] vdpa/mlx5: Support different address spaces for control and data
[not found] ` <DM8PR12MB54007865AC6BC6FCEC9911FCAB649@DM8PR12MB5400.namprd12.prod.outlook.com>
@ 2022-08-11 13:05 ` Michael S. Tsirkin
2022-08-11 13:09 ` Michael S. Tsirkin
1 sibling, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2022-08-11 13:05 UTC (permalink / raw)
To: Eli Cohen
Cc: Eugenio Perez Martin, Dan Carpenter,
virtualization@lists.linux-foundation.org
On Thu, Aug 11, 2022 at 12:40:09PM +0000, Eli Cohen wrote:
> > From: Dan Carpenter <dan.carpenter@oracle.com>
> > Sent: Thursday, August 11, 2022 1:40 PM
> > To: Eli Cohen <elic@nvidia.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Subject: [bug report] vdpa/mlx5: Support different address spaces for control and data
> >
> > Hello Eli Cohen,
> >
> > The patch d5358cd0e369: "vdpa/mlx5: Support different address spaces
> > for control and data" from Jul 14, 2022, leads to the following
> > Smatch static checker warning:
> >
> > drivers/vdpa/mlx5/net/mlx5_vnet.c:2676 mlx5_vdpa_set_map()
> > error: uninitialized symbol 'err'.
> >
> > drivers/vdpa/mlx5/net/mlx5_vnet.c
> > 2657 static int mlx5_vdpa_set_map(struct vdpa_device *vdev, unsigned int asid,
> > 2658 struct vhost_iotlb *iotlb)
> > 2659 {
> > 2660 struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
> > 2661 struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
> > 2662 int err;
> > 2663
> > 2664 down_write(&ndev->reslock);
> > 2665 if (mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] == asid) {
> > 2666 err = set_map_data(mvdev, iotlb);
> > 2667 if (err)
> > 2668 goto out;
> > 2669 }
> > 2670
> > 2671 if (mvdev->group2asid[MLX5_VDPA_CVQ_GROUP] == asid)
> > 2672 err = set_map_control(mvdev, iotlb);
> >
> > err not initialized on else path. My guess is that one or both of these
> > conditions has to be true and this is a false positive but I don't know
> > the code well enough to be sure.
>
> Thanks for reporting this.
> I think it would be better to return an error if the provided asid is not recognized.
>
> Therefore I am thinking about adding something like this:
>
> if (asid != mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] &&
> asid != mvdev->group2asid[MLX5_VDPA_CVQ_GROUP]) {
> err = -EINVAL;
> goto out;
> }
Patch on top pls? Time's running short.
> >
> > 2673
> > 2674 out:
> > 2675 up_write(&ndev->reslock);
> > --> 2676 return err;
> > 2677 }
> >
> > regards,
> > dan carpenter
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] vdpa/mlx5: Support different address spaces for control and data
[not found] ` <DM8PR12MB54007865AC6BC6FCEC9911FCAB649@DM8PR12MB5400.namprd12.prod.outlook.com>
2022-08-11 13:05 ` Michael S. Tsirkin
@ 2022-08-11 13:09 ` Michael S. Tsirkin
[not found] ` <DM8PR12MB54007F39684505DC60ACB92CAB649@DM8PR12MB5400.namprd12.prod.outlook.com>
1 sibling, 1 reply; 4+ messages in thread
From: Michael S. Tsirkin @ 2022-08-11 13:09 UTC (permalink / raw)
To: Eli Cohen
Cc: Eugenio Perez Martin, Dan Carpenter,
virtualization@lists.linux-foundation.org
On Thu, Aug 11, 2022 at 12:40:09PM +0000, Eli Cohen wrote:
> > From: Dan Carpenter <dan.carpenter@oracle.com>
> > Sent: Thursday, August 11, 2022 1:40 PM
> > To: Eli Cohen <elic@nvidia.com>
> > Cc: virtualization@lists.linux-foundation.org
> > Subject: [bug report] vdpa/mlx5: Support different address spaces for control and data
> >
> > Hello Eli Cohen,
> >
> > The patch d5358cd0e369: "vdpa/mlx5: Support different address spaces
> > for control and data" from Jul 14, 2022, leads to the following
> > Smatch static checker warning:
> >
> > drivers/vdpa/mlx5/net/mlx5_vnet.c:2676 mlx5_vdpa_set_map()
> > error: uninitialized symbol 'err'.
> >
> > drivers/vdpa/mlx5/net/mlx5_vnet.c
> > 2657 static int mlx5_vdpa_set_map(struct vdpa_device *vdev, unsigned int asid,
> > 2658 struct vhost_iotlb *iotlb)
> > 2659 {
> > 2660 struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
> > 2661 struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
> > 2662 int err;
> > 2663
> > 2664 down_write(&ndev->reslock);
> > 2665 if (mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] == asid) {
> > 2666 err = set_map_data(mvdev, iotlb);
> > 2667 if (err)
> > 2668 goto out;
> > 2669 }
> > 2670
> > 2671 if (mvdev->group2asid[MLX5_VDPA_CVQ_GROUP] == asid)
> > 2672 err = set_map_control(mvdev, iotlb);
> >
> > err not initialized on else path. My guess is that one or both of these
> > conditions has to be true and this is a false positive but I don't know
> > the code well enough to be sure.
>
> Thanks for reporting this.
> I think it would be better to return an error if the provided asid is not recognized.
>
> Therefore I am thinking about adding something like this:
>
> if (asid != mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] &&
> asid != mvdev->group2asid[MLX5_VDPA_CVQ_GROUP]) {
> err = -EINVAL;
> goto out;
> }
I would probably chain the conditions:
if (mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] == asid) {
} else if (mvdev->group2asid[MLX5_VDPA_CVQ_GROUP] == asid) {
} else {
err = -EINVAL;
goto out;
}
or alternatively initialize err with -EINVAL and be done with it.
> >
> > 2673
> > 2674 out:
> > 2675 up_write(&ndev->reslock);
> > --> 2676 return err;
> > 2677 }
> >
> > regards,
> > dan carpenter
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] vdpa/mlx5: Support different address spaces for control and data
[not found] ` <DM8PR12MB54007F39684505DC60ACB92CAB649@DM8PR12MB5400.namprd12.prod.outlook.com>
@ 2022-08-11 13:21 ` Michael S. Tsirkin
0 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2022-08-11 13:21 UTC (permalink / raw)
To: Eli Cohen
Cc: Eugenio Perez Martin, Dan Carpenter,
virtualization@lists.linux-foundation.org
On Thu, Aug 11, 2022 at 01:11:11PM +0000, Eli Cohen wrote:
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, August 11, 2022 4:09 PM
> > To: Eli Cohen <elic@nvidia.com>
> > Cc: Dan Carpenter <dan.carpenter@oracle.com>; Jason Wang <jasowang@redhat.com>; Eugenio Perez Martin
> > <eperezma@redhat.com>; virtualization@lists.linux-foundation.org
> > Subject: Re: [bug report] vdpa/mlx5: Support different address spaces for control and data
> >
> > On Thu, Aug 11, 2022 at 12:40:09PM +0000, Eli Cohen wrote:
> > > > From: Dan Carpenter <dan.carpenter@oracle.com>
> > > > Sent: Thursday, August 11, 2022 1:40 PM
> > > > To: Eli Cohen <elic@nvidia.com>
> > > > Cc: virtualization@lists.linux-foundation.org
> > > > Subject: [bug report] vdpa/mlx5: Support different address spaces for control and data
> > > >
> > > > Hello Eli Cohen,
> > > >
> > > > The patch d5358cd0e369: "vdpa/mlx5: Support different address spaces
> > > > for control and data" from Jul 14, 2022, leads to the following
> > > > Smatch static checker warning:
> > > >
> > > > drivers/vdpa/mlx5/net/mlx5_vnet.c:2676 mlx5_vdpa_set_map()
> > > > error: uninitialized symbol 'err'.
> > > >
> > > > drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > 2657 static int mlx5_vdpa_set_map(struct vdpa_device *vdev, unsigned int asid,
> > > > 2658 struct vhost_iotlb *iotlb)
> > > > 2659 {
> > > > 2660 struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
> > > > 2661 struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
> > > > 2662 int err;
> > > > 2663
> > > > 2664 down_write(&ndev->reslock);
> > > > 2665 if (mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] == asid) {
> > > > 2666 err = set_map_data(mvdev, iotlb);
> > > > 2667 if (err)
> > > > 2668 goto out;
> > > > 2669 }
> > > > 2670
> > > > 2671 if (mvdev->group2asid[MLX5_VDPA_CVQ_GROUP] == asid)
> > > > 2672 err = set_map_control(mvdev, iotlb);
> > > >
> > > > err not initialized on else path. My guess is that one or both of these
> > > > conditions has to be true and this is a false positive but I don't know
> > > > the code well enough to be sure.
> > >
> > > Thanks for reporting this.
> > > I think it would be better to return an error if the provided asid is not recognized.
> > >
> > > Therefore I am thinking about adding something like this:
> > >
> > > if (asid != mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] &&
> > > asid != mvdev->group2asid[MLX5_VDPA_CVQ_GROUP]) {
> > > err = -EINVAL;
> > > goto out;
> > > }
> >
> > I would probably chain the conditions:
> >
> > if (mvdev->group2asid[MLX5_VDPA_DATAVQ_GROUP] == asid) {
> > } else if (mvdev->group2asid[MLX5_VDPA_CVQ_GROUP] == asid) {
> > } else {
> > err = -EINVAL;
> > goto out;
> > }
> >
> >
> > or alternatively initialize err with -EINVAL and be done with it.
> >
> This makes more sense.
> Will send a patch in an hour.
Just making sure, the only result without this patch is that
and iotlb might be created without a mapping.
But nothing terrible bad will happen things just won't work
but this userspace is already buggy.
Right?
> >
> > > >
> > > > 2673
> > > > 2674 out:
> > > > 2675 up_write(&ndev->reslock);
> > > > --> 2676 return err;
> > > > 2677 }
> > > >
> > > > regards,
> > > > dan carpenter
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-11 13:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-11 10:39 [bug report] vdpa/mlx5: Support different address spaces for control and data Dan Carpenter
[not found] ` <DM8PR12MB54007865AC6BC6FCEC9911FCAB649@DM8PR12MB5400.namprd12.prod.outlook.com>
2022-08-11 13:05 ` Michael S. Tsirkin
2022-08-11 13:09 ` Michael S. Tsirkin
[not found] ` <DM8PR12MB54007F39684505DC60ACB92CAB649@DM8PR12MB5400.namprd12.prod.outlook.com>
2022-08-11 13:21 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).