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.
Shannon Nelson
2023-May-12 23:55 UTC
[PATCH] virtio-vdpa: Fix unchecked call to NULL set_vq_affinity
On 5/12/23 6:30 AM, Michael S. Tsirkin wrote:> > 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.Are these queued into a repo that is accessible? I haven't seen activity in the vhost.git where I would have expected it. After stumbling over and debugging this same problem, I was happy to see it fixed, and I'd like to pull from a repo that has the current updates. Thanks, sln