* [Qemu-devel] [PATCH] net: virtio-net: initialise local 'netcfg' variable
@ 2016-12-02 11:28 P J P
2016-12-05 7:48 ` Jason Wang
0 siblings, 1 reply; 5+ messages in thread
From: P J P @ 2016-12-02 11:28 UTC (permalink / raw)
To: Qemu Developers; +Cc: Jason Wang, Azureyang, Prasad J Pandit
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);
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net: virtio-net: initialise local 'netcfg' variable
2016-12-02 11:28 [Qemu-devel] [PATCH] net: virtio-net: initialise local 'netcfg' variable P J P
@ 2016-12-05 7:48 ` Jason Wang
2016-12-05 9:10 ` P J P
0 siblings, 1 reply; 5+ messages in thread
From: Jason Wang @ 2016-12-05 7:48 UTC (permalink / raw)
To: P J P, Qemu Developers; +Cc: Azureyang, Prasad J Pandit
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net: virtio-net: initialise local 'netcfg' variable
2016-12-05 7:48 ` Jason Wang
@ 2016-12-05 9:10 ` P J P
2016-12-05 9:43 ` Jason Wang
0 siblings, 1 reply; 5+ messages in thread
From: P J P @ 2016-12-05 9:10 UTC (permalink / raw)
To: Jason Wang; +Cc: Qemu Developers, Azureyang
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net: virtio-net: initialise local 'netcfg' variable
2016-12-05 9:10 ` P J P
@ 2016-12-05 9:43 ` Jason Wang
2016-12-05 10:30 ` P J P
0 siblings, 1 reply; 5+ messages in thread
From: Jason Wang @ 2016-12-05 9:43 UTC (permalink / raw)
To: P J P; +Cc: Azureyang, Qemu Developers
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] net: virtio-net: initialise local 'netcfg' variable
2016-12-05 9:43 ` Jason Wang
@ 2016-12-05 10:30 ` P J P
0 siblings, 0 replies; 5+ messages in thread
From: P J P @ 2016-12-05 10:30 UTC (permalink / raw)
To: Jason Wang; +Cc: Azureyang, Qemu Developers
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-05 10:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-02 11:28 [Qemu-devel] [PATCH] net: virtio-net: initialise local 'netcfg' variable P J P
2016-12-05 7:48 ` Jason Wang
2016-12-05 9:10 ` P J P
2016-12-05 9:43 ` Jason Wang
2016-12-05 10:30 ` P J P
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.