[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] virtio-net: check the existence of peer before accesing its
From: |
Michael S. Tsirkin |
Subject: |
Re: [PATCH] virtio-net: check the existence of peer before accesing its config |
Date: |
Mon, 27 Jul 2020 07:41:30 -0400 |
On Mon, Jul 27, 2020 at 06:26:37PM +0800, Jason Wang wrote:
>
> On 2020/7/27 下午6:13, Michael S. Tsirkin wrote:
> > On Mon, Jul 27, 2020 at 11:53:22AM +0200, Cornelia Huck wrote:
> > > On Mon, 27 Jul 2020 05:41:17 -0400
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > >
> > > > On Mon, Jul 27, 2020 at 03:43:28PM +0800, Jason Wang wrote:
> > > > > We try to get config from peer unconditionally which may lead NULL
> > > > > pointer dereference. Add a check before trying to access the config.
> > > > >
> > > > > Fixes: 108a64818e69b ("vhost-vdpa: introduce vhost-vdpa backend")
> > > > > Cc: Cindy Lu <lulu@redhat.com>
> > > > > Tested-by: Cornelia Huck <cohuck@redhat.com>
> > > > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > > I am a bit lost here. Isn't this invoked
> > > > when guest attempts to read the config?
> > > > With no peer, what do we return to guest?
> > > Same as with a non-vdpa peer? It's the dereference that needs to be
> > > guarded.
> > So vdpa has a GET_CONFIG ioctl which to me hints that a peer needs to be
> > notified on get config calls.
> > If we return config from qemu memory here, then I guess we
> > need to call GET_CONFIG on connect and validate it -
> > does this make sense?
> >
> > Cindy, Jason?
>
>
> For "connect" you meant connecting virtio-net to its peer (vDPA)? AFAIK, if
> we start with no peer, there's no way to set a peer afterwards.
>
> Thanks
That would be a good sentence to add in a code comment:
/*
* Is this VDPA? No peer means not VDPA: there's no way to
* disconnect/reconnect a VDPA peer.
*/
>
> >
> > > > A code comment might be helpful here.
> > > >
> > > > > ---
> > > > > hw/net/virtio-net.c | 22 +++++++++++-----------
> > > > > 1 file changed, 11 insertions(+), 11 deletions(-)