[Qemu-devel] [PATCH 0/2] net: socket backend uses uninitialized variables

[Qemu-devel] [PATCH 0/2] net: socket backend uses uninitialized variables

[Markus Armbruster]

Compile-tested only, buyer beware.

Markus Armbruster (2):
  net: socket backend passes junk size to getsockname(), fix
  net: socket backend shows junk in "info network", fix

 net/socket.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

-- 
1.7.6.4

[Qemu-devel] [PATCH 1/2] net: socket backend passes junk size to getsockname(), fix

[Markus Armbruster]

net_socket_fd_init_dgram() passes an uninitialized address length to
getsockname().  I guess this happens to work as long as the junk value
is at least sizeof(sockaddr_in).

Spotted by Coverity.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 net/socket.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index e9ef128..eb3e0d6 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -260,6 +260,7 @@ static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan,
      */
 
     if (is_connected) {
+        saddr_len = sizeof(saddr);
 	if (getsockname(fd, (struct sockaddr *) &saddr, &saddr_len) == 0) {
 	    /* must be bound */
 	    if (saddr.sin_addr.s_addr==0) {
-- 
1.7.6.4

[Qemu-devel] [PATCH 2/2] net: socket backend shows junk in "info network", fix

[Markus Armbruster]

net_socket_fd_init_dgram() prints uninitialized saddr into
nc->info_str.

Spotted by Coverity.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 net/socket.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index eb3e0d6..42bbae4 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -288,10 +288,15 @@ static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan,
 
     nc = qemu_new_net_client(&net_dgram_socket_info, vlan, NULL, model, name);
 
-    snprintf(nc->info_str, sizeof(nc->info_str),
-	    "socket: fd=%d (%s mcast=%s:%d)",
-	    fd, is_connected ? "cloned" : "",
-	    inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
+    if (is_connected) {
+        snprintf(nc->info_str, sizeof(nc->info_str),
+                 "socket: fd=%d (cloned mcast=%s:%d)",
+                 fd, inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
+    } else {
+        snprintf(nc->info_str, sizeof(nc->info_str),
+                 "socket: fd=%d",
+                 fd);
+    }
 
     s = DO_UPCAST(NetSocketState, nc, nc);
 
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH 0/2] net: socket backend uses uninitialized variables

[Markus Armbruster]

Ping?

Markus Armbruster <armbru@redhat.com> writes:

> Compile-tested only, buyer beware.
>
> Markus Armbruster (2):
>   net: socket backend passes junk size to getsockname(), fix
>   net: socket backend shows junk in "info network", fix
>
>  net/socket.c |   14 ++++++++++----
>  1 files changed, 10 insertions(+), 4 deletions(-)

Re: [Qemu-devel] [PATCH 0/2] net: socket backend uses uninitialized variables

[Anthony Liguori]

On 11/11/2011 03:36 AM, Markus Armbruster wrote:
> Ping?

I assumed "Compile-tested only, buyer beware." was code for, I'll test the 
patches later and post to the ML ;-)

Regards,

Anthony Liguori

>
> Markus Armbruster<armbru@redhat.com>  writes:
>
>> Compile-tested only, buyer beware.
>>
>> Markus Armbruster (2):
>>    net: socket backend passes junk size to getsockname(), fix
>>    net: socket backend shows junk in "info network", fix
>>
>>   net/socket.c |   14 ++++++++++----
>>   1 files changed, 10 insertions(+), 4 deletions(-)
>
>

Re: [Qemu-devel] [PATCH 0/2] net: socket backend uses uninitialized variables

[Markus Armbruster]

Anthony Liguori <aliguori@us.ibm.com> writes:

> On 11/11/2011 03:36 AM, Markus Armbruster wrote:
>> Ping?
>
> I assumed "Compile-tested only, buyer beware." was code for, I'll test
> the patches later and post to the ML ;-)

Actually, it was "I've never used this network backend, and can't be
bothered to try it now, but maybe someone who is using it would like to
pick up the patches" ;-P

By the time I understood Coverity's reports, the fixes were obvious, so
I posted them.  I'm happy to shelve them until they get in the way of
Coverity scanning.  Which I don't expect to happen before another >500
reports have been taken care of.