| Message ID | 1480678088-21464-1-git-send-email-ppandit@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 2016年12月02日 19:28, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > Local 'netcfg' variable in 'virtio_net_get_config' routine was > not initialised. It could leak uninitialised 'netcfg.mtu' field > memory. Initialise 'netcfg' to avoid it. > > Reported-by: Azureyang <azureyang@tencent.com> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > hw/net/virtio-net.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 01f1351..cb5b3dc 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -72,7 +72,7 @@ static int vq2q(int queue_index) > static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config) > { > VirtIONet *n = VIRTIO_NET(vdev); > - struct virtio_net_config netcfg; > + struct virtio_net_config netcfg = {}; > > virtio_stw_p(vdev, &netcfg.status, n->status); > virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues); Good catch but since mtu patch wasn't accepted so mtu were in fact not exposed to guest. (FYI, you can have a look at Maxime patch, he did a stw_p here()). Thanks
Hello Jason,
+-- On Mon, 5 Dec 2016, Jason Wang wrote --+
| > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
| > index 01f1351..cb5b3dc 100644
| > --- a/hw/net/virtio-net.c
| > +++ b/hw/net/virtio-net.c
| > @@ -72,7 +72,7 @@ static int vq2q(int queue_index)
| > static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
| > {
| > VirtIONet *n = VIRTIO_NET(vdev);
| > - struct virtio_net_config netcfg;
| > + struct virtio_net_config netcfg = {};
| >
| > virtio_stw_p(vdev, &netcfg.status, n->status);
| > virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);
|
| Good catch but since mtu patch wasn't accepted so mtu were in fact not exposed
| to guest.
'mtu' appears to have been added by commit 'dbdfea9226c9d0bdd', could you
pleae confirm?
| (FYI, you can have a look at Maxime patch, he did a stw_p here()).
+ virtio_stw_p(vdev, &netcfg.mtu, n->mtu);
+
Yes, but this isn't accepted yet, is it?
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
On 2016年12月05日 17:10, P J P wrote: > Hello Jason, > > +-- On Mon, 5 Dec 2016, Jason Wang wrote --+ > | > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > | > index 01f1351..cb5b3dc 100644 > | > --- a/hw/net/virtio-net.c > | > +++ b/hw/net/virtio-net.c > | > @@ -72,7 +72,7 @@ static int vq2q(int queue_index) > | > static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config) > | > { > | > VirtIONet *n = VIRTIO_NET(vdev); > | > - struct virtio_net_config netcfg; > | > + struct virtio_net_config netcfg = {}; > | > > | > virtio_stw_p(vdev, &netcfg.status, n->status); > | > virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues); > | > | Good catch but since mtu patch wasn't accepted so mtu were in fact not exposed > | to guest. > > 'mtu' appears to have been added by commit 'dbdfea9226c9d0bdd', could you > pleae confirm? Yes. > > | (FYI, you can have a look at Maxime patch, he did a stw_p here()). > > + virtio_stw_p(vdev, &netcfg.mtu, n->mtu); > + > > Yes, but this isn't accepted yet, is it? > > > Thank you. > -- > Prasad J Pandit / Red Hat Product Security Team > 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F > See feature_sizes[] in virtio-net.c, we won't expose mtu to guest until MTU feature were negotiated. Thanks
Hello Jason,
+-- On Mon, 5 Dec 2016, Jason Wang wrote --+
| See feature_sizes[] in virtio-net.c, we won't expose mtu to guest until MTU
| feature were negotiated.
Oh, it updates config size without the 'mtu' field during realise, okay.
Still IMO having an initialiser({}) is better. But upto you.
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 01f1351..cb5b3dc 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -72,7 +72,7 @@ static int vq2q(int queue_index) static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config) { VirtIONet *n = VIRTIO_NET(vdev); - struct virtio_net_config netcfg; + struct virtio_net_config netcfg = {}; virtio_stw_p(vdev, &netcfg.status, n->status); virtio_stw_p(vdev, &netcfg.max_virtqueue_pairs, n->max_queues);