[Qemu-devel] Bug in net/socket.c: info_str is overwritten

[Qemu-devel] Bug in net/socket.c: info_str is overwritten

[Miguel Di Ciurcio Filho]

Hi there,

I'm working on the conversion of the 'info network' command do QMP,
and I think I've found some problems.

Running qemu like this: qemu -net socket,listen=:3333

In net/socket.c, the function net_socket_listen_init() [1] is called
and a listening socket is created. There is nothing registering this,
so when using 'info network' as is, no information about this
listening socket is shown.

When a connection is accepted, the handler net_socket_accept() calls
net_socket_fd_init(). If the socket is
UDP/multicast net_socket_fd_init() calls net_socket_fd_init_dgram, if
the socket is TCP it calls net_socket_fd_init_dgram_stream. [2]

In both cases the info_str string is written inside
net_socket_fd_init_(stream|dgram) [3], and after that, it is
overwritten on a subsequent
snprintf() in net_socket_accept() [4].

net_socket_fd_init_(stream|dgram) always puts into the info_str the fd
number, and this information is overwritten latter. So, is the fd
number
relevant to be transmitted over QMP? Right now this information is
being lost, IMHO.

Same thing happens when qemu is run as a client:
net_socket_connect_init() calls net_socket_fd_init_(dgram|stream) and
latter overwrites info_str.

[1] http://git.qemu.org/qemu.git/tree/net/socket.c#n375
[2] http://git.qemu.org/qemu.git/tree/net/socket.c#n336
[3] http://git.qemu.org/qemu.git/tree/net/socket.c#n310
[4] http://git.qemu.org/qemu.git/tree/net/socket.c#n369

[Qemu-devel] Re: Bug in net/socket.c: info_str is overwritten

[Luiz Capitulino]

On Thu, 6 May 2010 10:42:10 -0300
Miguel Di Ciurcio Filho <miguel.filho@gmail.com> wrote:

> Hi there,
> 
> I'm working on the conversion of the 'info network' command do QMP,
> and I think I've found some problems.
> 
> Running qemu like this: qemu -net socket,listen=:3333
> 
> In net/socket.c, the function net_socket_listen_init() [1] is called
> and a listening socket is created. There is nothing registering this,
> so when using 'info network' as is, no information about this
> listening socket is shown.

 Right, not sure how important this info is but I'd expect it to
be shown.

> When a connection is accepted, the handler net_socket_accept() calls
> net_socket_fd_init(). If the socket is
> UDP/multicast net_socket_fd_init() calls net_socket_fd_init_dgram, if
> the socket is TCP it calls net_socket_fd_init_dgram_stream. [2]
> 
> In both cases the info_str string is written inside
> net_socket_fd_init_(stream|dgram) [3], and after that, it is
> overwritten on a subsequent
> snprintf() in net_socket_accept() [4].

 Yes, those snprintf() in net_socket_fd_init_{stream,dgram} seems dead
code to me.

 Another problem is that info_str is not cleaned when the connection
is closed, which makes 'info network' keep saying that the connection is there.

> net_socket_fd_init_(stream|dgram) always puts into the info_str the fd
> number, and this information is overwritten latter. So, is the fd
> number
> relevant to be transmitted over QMP? Right now this information is
> being lost, IMHO.

 Only client writers can answer, I guess. But it's not a problem if we
don't include it now, as it's easy to extend the output later if needed.

> 
> Same thing happens when qemu is run as a client:
> net_socket_connect_init() calls net_socket_fd_init_(dgram|stream) and
> latter overwrites info_str.
> 
> [1] http://git.qemu.org/qemu.git/tree/net/socket.c#n375
> [2] http://git.qemu.org/qemu.git/tree/net/socket.c#n336
> [3] http://git.qemu.org/qemu.git/tree/net/socket.c#n310
> [4] http://git.qemu.org/qemu.git/tree/net/socket.c#n369
>