Dragos Tatulea
2023-May-12 12:51 UTC
[PATCH] virtio-vdpa: Fix unchecked call to NULL set_vq_affinity
On Thu, 2023-05-04 at 14:51 -0400, Michael S. Tsirkin wrote:> On Thu, May 04, 2023 at 01:08:54PM -0400, Feng Liu wrote: > > > > > > On 2023-05-04 a.m.9:50, Dragos Tatulea wrote: > > > External email: Use caution opening links or attachments > > > > > > > > > The referenced patch calls set_vq_affinity without checking if the op is > > > valid. This patch adds the check. > > > > > > Fixes: 3dad56823b53 ("virtio-vdpa: Support interrupt affinity spreading > > > mechanism") > > > Reviewed-by: Gal Pressman <gal at nvidia.com> > > > Signed-off-by: Dragos Tatulea <dtatulea at nvidia.com> > > > --- > > > ? drivers/virtio/virtio_vdpa.c | 4 +++- > > > ? 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c > > > index eb6aee8c06b2..989e2d7184ce 100644 > > > --- a/drivers/virtio/virtio_vdpa.c > > > +++ b/drivers/virtio/virtio_vdpa.c > > > @@ -385,7 +385,9 @@ static int virtio_vdpa_find_vqs(struct virtio_device > > > *vdev, unsigned int nvqs, > > > ???????????????????????? err = PTR_ERR(vqs[i]); > > > ???????????????????????? goto err_setup_vq; > > > ???????????????? } > > > -?????????????? ops->set_vq_affinity(vdpa, i, &masks[i]); > > > + > > > +?????????????? if (ops->set_vq_affinity) > > > +?????????????????????? ops->set_vq_affinity(vdpa, i, &masks[i]); > > if ops->set_vq_affinity is NULL, should give an error code to err, and > > return err > > Given we ignore return code, hardly seems like a critical thing to do. > Is it really important? affinity is an optimization isn't it? > > > >set_vq_affinity is optional so it's not an error if the op is not implemented. Is there anything else that needs to be done for this fix? Thanks, Dragos
Michael S. Tsirkin
2023-May-12 13:30 UTC
[PATCH] virtio-vdpa: Fix unchecked call to NULL set_vq_affinity
On Fri, May 12, 2023 at 12:51:21PM +0000, Dragos Tatulea wrote:> On Thu, 2023-05-04 at 14:51 -0400, Michael S. Tsirkin wrote: > > On Thu, May 04, 2023 at 01:08:54PM -0400, Feng Liu wrote: > > > > > > > > > On 2023-05-04 a.m.9:50, Dragos Tatulea wrote: > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > The referenced patch calls set_vq_affinity without checking if the op is > > > > valid. This patch adds the check. > > > > > > > > Fixes: 3dad56823b53 ("virtio-vdpa: Support interrupt affinity spreading > > > > mechanism") > > > > Reviewed-by: Gal Pressman <gal at nvidia.com> > > > > Signed-off-by: Dragos Tatulea <dtatulea at nvidia.com> > > > > --- > > > > ? drivers/virtio/virtio_vdpa.c | 4 +++- > > > > ? 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c > > > > index eb6aee8c06b2..989e2d7184ce 100644 > > > > --- a/drivers/virtio/virtio_vdpa.c > > > > +++ b/drivers/virtio/virtio_vdpa.c > > > > @@ -385,7 +385,9 @@ static int virtio_vdpa_find_vqs(struct virtio_device > > > > *vdev, unsigned int nvqs, > > > > ???????????????????????? err = PTR_ERR(vqs[i]); > > > > ???????????????????????? goto err_setup_vq; > > > > ???????????????? } > > > > -?????????????? ops->set_vq_affinity(vdpa, i, &masks[i]); > > > > + > > > > +?????????????? if (ops->set_vq_affinity) > > > > +?????????????????????? ops->set_vq_affinity(vdpa, i, &masks[i]); > > > if ops->set_vq_affinity is NULL, should give an error code to err, and > > > return err > > > > Given we ignore return code, hardly seems like a critical thing to do. > > Is it really important? affinity is an optimization isn't it? > > > > > > > set_vq_affinity is optional so it's not an error if the op is not implemented. > > Is there anything else that needs to be done for this fix? > > Thanks, > Dragos >no, it's queued already.